bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Drew Adams]

> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Friday, November 24, 2017 2:51 AM
> To: Drew Adams <drew.adams@oracle.com>
> Cc: 29373-done@debbugs.gnu.org
> Subject: Re: bug#29373: 24.5; doc string of `self-insert-uses-region-
> functions'
> 
> > Date: Mon, 20 Nov 2017 18:56:15 -0800 (PST)
> > From: Drew Adams <drew.adams@oracle.com>
> >
> > Two lines of the doc string are too long.
> 
> Longer than 79 characters?  I didn't find any such lines.

Where do you see 79 as the recommended max length?
Traditionaly it has been 70, IIRC.

From (elisp) `Documentation Tips':

 Format the documentation string so that it fits in an Emacs window
 on an 80-column screen.  It is a good idea for most lines to be no
 wider than 60 characters.  The first line should not be wider than
 67 characters or it will look bad in the output of 'apropos'.

https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html

1 out of the 5 lines is less than 61 chars - not most.
But if you're happy with the lengths as is, fine.

> > The doc string is close to incomprehensible.
> 
> Such derogatory remarks are best kept out of bug reports.  The facts
> are grave enough to tell we should fix this.

Nothing derogatory intended.  That's a summary of the
problem - lack of clarity overall.  For me, at least,
the wording of this doc is not very understandable.

I have no idea who wrote it, nor do I care.  It's not
about blaming or attacking anyone.  It's about calling
your attention to what I, at least, think is a doc
string that needs some love (improvement).  And the
overall problem is not the technical content; it is
the lack of clarity - comprehensibility.  To me, at
least.

Anyway, I'm sure you made an improvement, whatever
change you made.  Thanks for that.

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Eli Zaretskii]

> Date: Fri, 24 Nov 2017 08:43:05 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 29373-done@debbugs.gnu.org
> 
> > > Two lines of the doc string are too long.
> > 
> > Longer than 79 characters?  I didn't find any such lines.
> 
> Where do you see 79 as the recommended max length?

Sometimes it's impossible to limit a line to 70 characters without
incurring worse problems.  This is one such case.

> > > The doc string is close to incomprehensible.
> > 
> > Such derogatory remarks are best kept out of bug reports.  The facts
> > are grave enough to tell we should fix this.
> 
> Nothing derogatory intended.

That sentence could simply be omitted without any effect on the
report.  May I suggest that you try to refrain from such unneeded
remarks?

> That's a summary of the problem - lack of clarity overall.

"Lack of clarity" would be fine (although it, too, adds nothing).
"Incomprehensible" is not a summary.

> I have no idea who wrote it, nor do I care.  It's not
> about blaming or attacking anyone.  It's about calling
> your attention to what I, at least, think is a doc
> string that needs some love (improvement).

What you said in the rest of the report is enough to drawing
attention.

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Drew Adams]

> Sometimes it's impossible to limit a line to 70 characters without
> incurring worse problems.  This is one such case.

I don't see any such "worse problems" in this case.
Without any change to the wording (which does need
to be changed), this shows no "worse problems":

 Special hook to tell if `self-insert-command' will use the region.
 It must be called via `run-hook-with-args-until-success' with no
 arguments.

 Any `post-self-insert-command' which consumes the region should
 register a function on this hook so that things like
 `delete-selection-mode' can refrain from consuming the region.

> > > > The doc string is close to incomprehensible.
> > >
> > > Such derogatory remarks are best kept out of bug reports.
> > > The facts are grave enough to tell we should fix this.
> >
> > Nothing derogatory intended.
> 
> That sentence could simply be omitted without any effect on the
> report.  May I suggest that you try to refrain from such unneeded
> remarks?
> 
> > That's a summary of the problem - lack of clarity overall.
> 
> "Lack of clarity" would be fine (although it, too, adds nothing).
> "Incomprehensible" is not a summary.

"Incomprehensible" was not the summary I used.
"Close to incomprehensible" is the summary I used.

I stand by that characterization, as one user
reporting a doc problem: to me, the doc string was
close to incomprehensible.  You might not find it so.

It's understandable that a new doc string might not
have had the attention it deserves, and so is not
as clear as it could be.

To me, this one was not really understandable.
Perhaps you prefer the statement, "I don't
understand this doc string, and I think others
may also have trouble making sense of it."

And no, I don't think that such a statement adds
nothing, even if it is better if accompanied by
guesses about the meaning, suggestions for
improvement, or pointing out particular places
that are especially problematic for the reporter.

If a reporter has no clue what is meant and no
clue what parts are particularly unclear, the
simple report that a doc string is unclear to
that reporter can help.  We can disagree about
this, of course.

But my guess is that if you agree with the
reporter after reading such an incomplete "Huh?"
report then the aim is reached.  And if you
disagree with the reporter - the doc is clear
to you, even if the reporter doesn't understand
it at all - it is still good that the perceived
problem was reported, and no harm was done.
We can disagree about this too, of course.

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Noam Postavsky]

On Fri, Nov 24, 2017 at 12:33 PM, Drew Adams <drew.adams@oracle.com> wrote:

> "Incomprehensible" was not the summary I used.
> "Close to incomprehensible" is the summary I used.

I don't think it's especially useful to focus too much on this
particular choice of phrase, but overall, I find your bug reports tend
towards a more rant-like and haranguing tone.

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Drew Adams]

Two lines of the doc string are too long.

The doc string is close to incomprehensible.

 Special hook to tell if `self-insert-command' will use the region.
 It must be called via `run-hook-with-args-until-success' with no arguments.
 Any `post-self-insert-command' which consumes the region should
 register a function on this hook so that things like `delete-selection-mode'
 can refrain from consuming the region.

HOW does this hook "tell if `self-insert-command' will use the region"?
What does a function on this hook need to do or return, to tell that?
And what does it mean for `self-insert-command' to "use the region"?

What does "any `post-self-insert-command'" even mean?  What is meant by
"consumes the region"?  Does "consume" mean the same thing in each
occurrence here?  What is meant by "registering" a function on a hook?

Is this ONLY about `delete-selection-mode'?  If not, what else is it
for?

A guess is that this is just a way to conditionalize the treatment of
`self-insert-command' by `delete-selection-mode'.  In some mode
(`electric-pair-mode'), you want to use a certain function as the value
of property `delete-selection' for symbol `self-insert-command'.

If so, then things would likely be clearer if that was said.

In that case, it needs to be understood that `delete-selection-mode'
checks the value of property `delete-selection' on
`self-insert-command', and if that value is nil or is a function that
returns nil then `delete-selection-mode' does nothing.

Say that, together with the fact that the function value of property
`delete-selection-mode' for `self-insert-command' runs the hook
functions in order, returning nil if any of them returns non-nil.

So a function on the hook should return non-nil when its aim is to
prevent deletion of the region by `self-insert-command'.  Say that.

Looking at the uses in the Emacs code, the aim seems to be only to
prevent deletion of the region by `delete-selection-mode' before
`self-insert-command' does its thing.  Whether `self-insert-command'
"uses" the region or "consumes" it, or even deletes it or ignores it, is
irrelevant, AFAICT.  All that matters is that this gives you a way to
decide conditionally whether `delete-selection-mode' deletes the region
on its pre-hook.


In GNU Emacs 24.5.1 (i686-pc-mingw32)
 of 2015-04-11 on LEG570
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/usr --host=i686-pc-mingw32'

bug#29373: 24.5; doc string of `self-insert-uses-region-functions'

[Eli Zaretskii]

> Date: Mon, 20 Nov 2017 18:56:15 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> Two lines of the doc string are too long.

Longer than 79 characters?  I didn't find any such lines.

> The doc string is close to incomprehensible.

Such derogatory remarks are best kept out of bug reports.  The facts
are grave enough to tell we should fix this.

>  Special hook to tell if `self-insert-command' will use the region.
>  It must be called via `run-hook-with-args-until-success' with no arguments.
>  Any `post-self-insert-command' which consumes the region should
>  register a function on this hook so that things like `delete-selection-mode'
>  can refrain from consuming the region.
> 
> HOW does this hook "tell if `self-insert-command' will use the region"?
> What does a function on this hook need to do or return, to tell that?
> And what does it mean for `self-insert-command' to "use the region"?
> 
> What does "any `post-self-insert-command'" even mean?  What is meant by
> "consumes the region"?  Does "consume" mean the same thing in each
> occurrence here?  What is meant by "registering" a function on a hook?

post-self-insert-command really meant to refer to post-self-insert-hook.

> A guess is that this is just a way to conditionalize the treatment of
> `self-insert-command' by `delete-selection-mode'.  In some mode
> (`electric-pair-mode'), you want to use a certain function as the value
> of property `delete-selection' for symbol `self-insert-command'.
> 
> If so, then things would likely be clearer if that was said.

I clarified that.

> In that case, it needs to be understood that `delete-selection-mode'
> checks the value of property `delete-selection' on
> `self-insert-command', and if that value is nil or is a function that
> returns nil then `delete-selection-mode' does nothing.

The first part was already in delsel.el.  I made clear that
self-insert-command uses this hook via delete-selection-uses-region-p.

> Say that, together with the fact that the function value of property
> `delete-selection-mode' for `self-insert-command' runs the hook
> functions in order, returning nil if any of them returns non-nil.

This is the usual behavior of run-hook-with-args-until-success.

> So a function on the hook should return non-nil when its aim is to
> prevent deletion of the region by `self-insert-command'.  Say that.

Done.