bug#2738: Content-free doc string: `handle-shift-selection'.

bug#2738: Content-free doc string: `handle-shift-selection'.

[Alan Mackenzie]

Hi, Emacs!

The doc-string for `handle-shift-selection' is of little or no help in
informing the reader what the function does.  It is, in fact,
infuriatingly useless.  This should be fixed.

In detail:

    Check for shift translation,

"Check for"??  This means possibly, "According to whether or not shift
translation is enabled".  Maybe

What is "shift translation"?  Is this something which might be
getting done somewhere in the raw processing of key strokes?  Or does it
mean "Check to see if the shift key is currently depressed"?  OK, I,
personally, have a vague idea it's to do with moving point whilst holding
down the shift key, but that's far from obvious from the doc string.

    and operate on the mark accordingly.

This is vacuous.  "Operate on" says NOTHING; it is equivalent to "do
something with".  So this command is going to "do something" with my
mark.  I'd far rather the doc string told me what.

    This is called whenever a command with a `^' character in its
    `interactive' spec is invoked while `shift-select-mode' is
    non-nil.

Great.  So I know one of the occasions on which `handle-shift-selection'
is called, but not what it does.  Does this mean that somebody can foul
up my mark (explicitly a USER feature), by programming a "^" in her doc
string?

    If the command was invoked through shift-translation,

"THROUGH" shift-translation.  Is "shift-translation" some sort of
processing step?  Is a translation being shifted here, or is a shift
being translated?

    set the mark and activate the region temporarily, unless it was
    already set in this way.  If the command was invoked without
    shift-translation and a region is temporarily active, deactivate the
    mark.

This reads like a flowchart, and it's uncomfortably close to gibberish.
Is it not possible to state the function's FUNCTION, rather than leaving
the reader to figure this out from its quasi-flowchart?

What is the semantic significance of "without" here?  What business has
some random command got setting my mark without my permission, or
"deactivating" it?

    With optional arg DEACTIVATE, only perform region deactivation.

Er, excuse me, but I'm an Emacs power user, I have a region from the very
moment I first set the mark in a buffer.  What does this all mean?

OK, I, personally, do have a some idea of what's meant, but why on earth
is this sort of functionality being inserted into the interactive string?
When getting arguments from the user, what does "^" at the beginning of
the string instruct the command loop to do?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#2738: Content-free doc string: `handle-shift-selection'.

[Eli Zaretskii]

> Date: Sat, 21 Mar 2009 17:23:43 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: 
> 
> The doc-string for `handle-shift-selection' is of little or no help in
> informing the reader what the function does.  It is, in fact,
> infuriatingly useless.  This should be fixed.

I changed it to say this:

     "Activate/deactivate mark depending on invocation thru ``shift translation.''

    \(See `this-command-keys-shift-translated' for the meaning of
    shift translation.)

    This is called whenever a command with a `^' character in its
    `interactive' spec is invoked while `shift-select-mode' is
    non-nil.

    If the command was invoked through shift translation, set the
    mark and activate the region temporarily, unless it was already
    set in this way.  If the command was invoked without shift
    translation, or if the optional argument DEACTIVATE is non-nil,
    deactivate the mark if the region is temporarily active."

Is this good enough to close the bug?  If not, please tell what still
needs improvement.

> In detail:
> 
>     Check for shift translation,
> 
> "Check for"??  This means possibly, "According to whether or not shift
> translation is enabled".  Maybe
> [...]
>     and operate on the mark accordingly.

The author tried to come up with something that would fit on a single
line, for `apropos's sake.  Sometimes the result is not entirely
correct for English grammar purists, but there's no need to become so
sarcastic.  (My modified doc string still sacrifices some of the
grammar correctness for brevity.)

> This is vacuous.  "Operate on" says NOTHING; it is equivalent to "do
> something with".  So this command is going to "do something" with my
> mark.  I'd far rather the doc string told me what.

It does, a few lines below.  Again, the first line of the doc string
can never tell the whole story, because it's a kind of executive
summary.

>     If the command was invoked through shift-translation,
> 
> "THROUGH" shift-translation.  Is "shift-translation" some sort of
> processing step?  Is a translation being shifted here, or is a shift
> being translated?

I added a direct link to where shift-translation is explained.
(Before that, it was reachable only from the doc string of
`shift-select-mode', i.e. by following one more link.)

>     set the mark and activate the region temporarily, unless it was
>     already set in this way.  If the command was invoked without
>     shift-translation and a region is temporarily active, deactivate the
>     mark.
> 
> This reads like a flowchart, and it's uncomfortably close to gibberish.
> Is it not possible to state the function's FUNCTION, rather than leaving
> the reader to figure this out from its quasi-flowchart?

Is the replacement better?

> When getting arguments from the user, what does "^" at the beginning of
> the string instruct the command loop to do?

This is not about getting arguments, this is about the `interactive'
spec, as the doc string says.  See "(elisp)Interactive Codes".

bug#2738: Content-free doc string: `handle-shift-selection'.

[Alan Mackenzie]

Hi, Eli!

On Sat, Mar 21, 2009 at 08:46:54PM +0200, Eli Zaretskii wrote:
> > Date: Sat, 21 Mar 2009 17:23:43 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: 

> > The doc-string for `handle-shift-selection' is of little or no help in
> > informing the reader what the function does.  It is, in fact,
> > infuriatingly useless.  This should be fixed.

> I changed it to say this:

>      "Activate/deactivate mark depending on invocation thru ``shift translation.''

>     \(See `this-command-keys-shift-translated' for the meaning of
>     shift translation.)

>     This is called whenever a command with a `^' character in its
>     `interactive' spec is invoked while `shift-select-mode' is
>     non-nil.

>     If the command was invoked through shift translation, set the
>     mark and activate the region temporarily, unless it was already
>     set in this way.  If the command was invoked without shift
>     translation, or if the optional argument DEACTIVATE is non-nil,
>     deactivate the mark if the region is temporarily active."

> Is this good enough to close the bug?  If not, please tell what still
> needs improvement.

I don't think it is.  I'm not sure this function can be documented
coherently.  I think it's a bad function.  Sorry, I'm not being very
constructive here.  I just find the whole thing distasteful in the
extreme, the idea that rather than binding commands to key sequences,
with a clean separation between the interactive commander, the key
sequences and the commands, we've now got a hodge podge where the
command loop now actually performs part of a command's function -
sometimes, depending on the key binding.

In fact, how about just saying something like "this function ensures the
mark is set for a movement command making a CUA region", or something
like that?

In addition this, the function is buggy.  It spuriously enables
transient-mark-mode in certain circumstances (I'll be submitting a bug
report soon).

[ .... ]

> >     If the command was invoked through shift-translation,

> > "THROUGH" shift-translation.  Is "shift-translation" some sort of
> > processing step?  Is a translation being shifted here, or is a shift
> > being translated?

> I added a direct link to where shift-translation is explained.
> (Before that, it was reachable only from the doc string of
> `shift-select-mode', i.e. by following one more link.)

Thanks!

> >     set the mark and activate the region temporarily, unless it was
> >     already set in this way.  If the command was invoked without
> >     shift-translation and a region is temporarily active, deactivate
> >     the mark.

> > This reads like a flowchart, and it's uncomfortably close to
> > gibberish.  Is it not possible to state the function's FUNCTION,
> > rather than leaving the reader to figure this out from its
> > quasi-flowchart?

> Is the replacement better?

A bit better, yes.

> > When getting arguments from the user, what does "^" at the beginning of
> > the string instruct the command loop to do?

> This is not about getting arguments, this is about the `interactive'
> spec, as the doc string says.  See "(elisp)Interactive Codes".

Does this bit not perhaps need a warning that "^" in an interactive spec
is really intended for Emacs's internal use and will throw an error on
anything but Emacs 23 (or later)?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#2738: Content-free doc string: `handle-shift-selection'.

[Stefan Monnier]

> In fact, how about just saying something like "this function ensures the
> mark is set for a movement command making a CUA region", or something
> like that?

It's probably a good idea to mention that it's used to implement
CUA-style behavior, indeed.

> Does this bit not perhaps need a warning that "^" in an interactive spec
> is really intended for Emacs's internal use and will throw an error on
> anything but Emacs 23 (or later)?

Our documentation is generally specific to the version of Emacs at hand
(plus some implicit "promise" that we'll try to maintain backward
compatibility in future releases), so "^" is not special in this regard.


        Stefan

bug#2738: Content-free doc string: `handle-shift-selection'.

[Eli Zaretskii]

> Date: Sun, 22 Mar 2009 22:59:15 +0000
> Cc: 2738@emacsbugs.donarmstrong.com
> From: Alan Mackenzie <acm@muc.de>
> 
> > Is this good enough to close the bug?  If not, please tell what still
> > needs improvement.
> 
> I don't think it is.  I'm not sure this function can be documented
> coherently.  I think it's a bad function.  Sorry, I'm not being very
> constructive here.  I just find the whole thing distasteful in the
> extreme, the idea that rather than binding commands to key sequences,
> with a clean separation between the interactive commander, the key
> sequences and the commands, we've now got a hodge podge where the
> command loop now actually performs part of a command's function -
> sometimes, depending on the key binding.
> 
> In fact, how about just saying something like "this function ensures the
> mark is set for a movement command making a CUA region", or something
> like that?
> 
> In addition this, the function is buggy.  It spuriously enables
> transient-mark-mode in certain circumstances (I'll be submitting a bug
> report soon).

Well, I'm sure doc strings cannot fix buggy or bad implementation, so
please do submit a separate bug report about that.

bug#2738: Content-free doc string: `handle-shift-selection'.

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Well, I'm sure doc strings cannot fix buggy or bad implementation, so
> please do submit a separate bug report about that.

The doc string has apparently been clarified at least one more time
since this, so I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/