* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
@ 2021-09-22 6:52 Stefan Kangas
2021-09-22 7:37 ` Eli Zaretskii
2021-09-22 20:35 ` Lars Ingebrigtsen
0 siblings, 2 replies; 10+ messages in thread
From: Stefan Kangas @ 2021-09-22 6:52 UTC (permalink / raw)
To: 50731
Severity: minor
The docstring of `progress-reporter-update' contains the following text:
If REPORTER is a numerical progress reporter---i.e. if it was
made using non-nil MIN-VALUE and MAX-VALUE arguments to
`make-progress-reporter'---then VALUE should be a number between
MIN-VALUE and MAX-VALUE.
If I place point on the space after "i.e." on the first row, and type
C-M-b (backward-sexp), point ends up before "reporter". (This happens
in `emacs-lisp-mode', but not in `message-mode'; `message-mode' seems to
have no concept of an abbreviation and lands on the "e".)
This in turn leads to checkdoc flagging this as a mistake, and asks you
to put two spaces after dot.
I'm not sure which of these options is true:
A) We should support the above convention of using three dashes in
`backward-sexp'. (This is the rough ASCII equivalent of an em dash.)
B) We should just not use the above convention in
`progress-reporter-update'.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-22 6:52 bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction Stefan Kangas
@ 2021-09-22 7:37 ` Eli Zaretskii
2021-09-22 22:12 ` Stefan Kangas
2021-09-22 20:35 ` Lars Ingebrigtsen
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-09-22 7:37 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 50731
> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 21 Sep 2021 23:52:58 -0700
>
> If REPORTER is a numerical progress reporter---i.e. if it was
> made using non-nil MIN-VALUE and MAX-VALUE arguments to
> `make-progress-reporter'---then VALUE should be a number between
> MIN-VALUE and MAX-VALUE.
>
> If I place point on the space after "i.e." on the first row, and type
> C-M-b (backward-sexp), point ends up before "reporter". (This happens
> in `emacs-lisp-mode', but not in `message-mode'; `message-mode' seems to
> have no concept of an abbreviation and lands on the "e".)
>
> This in turn leads to checkdoc flagging this as a mistake, and asks you
> to put two spaces after dot.
Why does it do that? because it doesn't recognize "i.e." as something
that doesn't end a sentence? And if so, how does the "---" thing come
into play here?
> A) We should support the above convention of using three dashes in
> `backward-sexp'. (This is the rough ASCII equivalent of an em dash.)
>
> B) We should just not use the above convention in
> `progress-reporter-update'.
None of the above? I guess I don't yet understand the root cause for
the problem.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-22 6:52 bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction Stefan Kangas
2021-09-22 7:37 ` Eli Zaretskii
@ 2021-09-22 20:35 ` Lars Ingebrigtsen
2021-09-22 22:12 ` Stefan Kangas
1 sibling, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-22 20:35 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 50731
Stefan Kangas <stefan@marxist.se> writes:
> The docstring of `progress-reporter-update' contains the following text:
>
> If REPORTER is a numerical progress reporter---i.e. if it was
> made using non-nil MIN-VALUE and MAX-VALUE arguments to
> `make-progress-reporter'---then VALUE should be a number between
> MIN-VALUE and MAX-VALUE.
I think the ---s here should just be made into parentheses.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-22 20:35 ` Lars Ingebrigtsen
@ 2021-09-22 22:12 ` Stefan Kangas
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Kangas @ 2021-09-22 22:12 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 50731
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Stefan Kangas <stefan@marxist.se> writes:
>
>> The docstring of `progress-reporter-update' contains the following text:
>>
>> If REPORTER is a numerical progress reporter---i.e. if it was
>> made using non-nil MIN-VALUE and MAX-VALUE arguments to
>> `make-progress-reporter'---then VALUE should be a number between
>> MIN-VALUE and MAX-VALUE.
>
> I think the ---s here should just be made into parentheses.
That would be the easiest option by far. Maybe that's all we need to do
here, really.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-22 7:37 ` Eli Zaretskii
@ 2021-09-22 22:12 ` Stefan Kangas
2021-09-23 6:16 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-09-22 22:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50731
Eli Zaretskii <eliz@gnu.org> writes:
>> A) We should support the above convention of using three dashes in
>> `backward-sexp'. (This is the rough ASCII equivalent of an em dash.)
>>
>> B) We should just not use the above convention in
>> `progress-reporter-update'.
>
> None of the above? I guess I don't yet understand the root cause for
> the problem.
I guess I have mixed up two different things above.
From my point of view, the first thing to do is to decide is if this
`backward-sexp' behavior is a bug or not. So this comes down to if we
think that "---" should be treated as a delimiter, just as " - " is, in
the context of ELisp docstrings.
I don't have a strong opinion either way, but I did find the current
behavior somewhat surprising when I saw it.
On the other hand, even if this is a bug, it might very well not be
worth fixing as the "---" delimiter is not very commonly used.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-22 22:12 ` Stefan Kangas
@ 2021-09-23 6:16 ` Eli Zaretskii
2021-09-23 7:12 ` Stefan Kangas
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-09-23 6:16 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 50731
> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 22 Sep 2021 15:12:51 -0700
> Cc: 50731@debbugs.gnu.org
>
> >From my point of view, the first thing to do is to decide is if this
> `backward-sexp' behavior is a bug or not.
How do we decide? What is TRT for backward-sexp in context that has
no sexps? If we change anything here, it could potentially affect a
lot of unrelated code out there.
Once again, why does checkdoc want you to have 2 spaces there? what
rule is it that ends up requiring that?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-23 6:16 ` Eli Zaretskii
@ 2021-09-23 7:12 ` Stefan Kangas
2021-09-23 7:47 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-09-23 7:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50731
Eli Zaretskii <eliz@gnu.org> writes:
> How do we decide? What is TRT for backward-sexp in context that has
> no sexps? If we change anything here, it could potentially affect a
> lot of unrelated code out there.
That's true.
> Once again, why does checkdoc want you to have 2 spaces there? what
> rule is it that ends up requiring that?
Sorry, I didn't understand you were asking about checkdoc. The code
there is really simple: find the next dot (that is also at a word
boundary), run `backward-sexp', use a regexp to see if this is an
abbreviation.
So it lands on the "r" in "reporter---i.e." and says, no, this is not an
abbreviation I know about, so there has to be two spaces after the dot.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-23 7:12 ` Stefan Kangas
@ 2021-09-23 7:47 ` Eli Zaretskii
2021-09-23 9:04 ` Stefan Kangas
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2021-09-23 7:47 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 50731
> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 23 Sep 2021 00:12:08 -0700
> Cc: 50731@debbugs.gnu.org
>
> > Once again, why does checkdoc want you to have 2 spaces there? what
> > rule is it that ends up requiring that?
>
> Sorry, I didn't understand you were asking about checkdoc.
Because you said:
> This in turn leads to checkdoc flagging this as a mistake, and asks you
> to put two spaces after dot.
I read this as the issue you'd like to solve: to prevent checkdoc from
flagging this as a mistake. Was I misinterpreting your report?
> The code
> there is really simple: find the next dot (that is also at a word
> boundary), run `backward-sexp', use a regexp to see if this is an
> abbreviation.
>
> So it lands on the "r" in "reporter---i.e." and says, no, this is not an
> abbreviation I know about
Isn't _that_ a bug, right there? Why does checkdoc use this strange
strategy for identifying abbreviations? A punctuation character that
precedes an abbreviation shouldn't interfere with recognizing the
abbreviation, IMO. And using sexp movements in text that is not
really a sexp is perhaps "the original sin" here?
IOW, I see no reason whatsoever to believe backward-sexp is the
problem here. The problem is in checkdoc, and IMO should be solved
there.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-23 7:47 ` Eli Zaretskii
@ 2021-09-23 9:04 ` Stefan Kangas
2021-09-23 20:40 ` Stefan Kangas
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Kangas @ 2021-09-23 9:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50731
Eli Zaretskii <eliz@gnu.org> writes:
> I read this as the issue you'd like to solve: to prevent checkdoc from
> flagging this as a mistake. Was I misinterpreting your report?
No, I guess this bug report touched on several different things is all.
It seems productive to focus the discussion on checkdoc as you suggest.
> IOW, I see no reason whatsoever to believe backward-sexp is the
> problem here. The problem is in checkdoc, and IMO should be solved
> there.
True. I guess it makes sense to just use 'backward-word' instead.
Unless anyone has any better suggestions, I will try this approach and
see how well it works in practice.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction
2021-09-23 9:04 ` Stefan Kangas
@ 2021-09-23 20:40 ` Stefan Kangas
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Kangas @ 2021-09-23 20:40 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50731
tags 50731 fixed
close 50731 28.1
thanks
Stefan Kangas <stefan@marxist.se> writes:
> True. I guess it makes sense to just use 'backward-word' instead.
> Unless anyone has any better suggestions, I will try this approach and
> see how well it works in practice.
This has now been fixed by using 'forward-word' instead of
'forward-sexp' in checkdoc (commit 55083d90a3). I also added three new
test cases.
I'm therefore closing this bug report.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-23 20:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-22 6:52 bug#50731: `progress-reporter-update' docstring and `backward-sexp' interaction Stefan Kangas
2021-09-22 7:37 ` Eli Zaretskii
2021-09-22 22:12 ` Stefan Kangas
2021-09-23 6:16 ` Eli Zaretskii
2021-09-23 7:12 ` Stefan Kangas
2021-09-23 7:47 ` Eli Zaretskii
2021-09-23 9:04 ` Stefan Kangas
2021-09-23 20:40 ` Stefan Kangas
2021-09-22 20:35 ` Lars Ingebrigtsen
2021-09-22 22:12 ` Stefan Kangas
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).