unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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).