Subject: RE: bug#21275: 24.5; Selection deleted with electric pair mode in cc mode

From: Drew Adams <[email-masked]>
Date: Thu, 20 Aug 2015 22:46:42 -0700 (PDT)

Hi Alan,

Yes, it's a workaround for electric-pair mode. And yes, delete-selection mode got there first. A few decades earlier. ;-)

My point is that this workaround for electric-pair should have an "electric-pair" name, not a "delete-selection" name. It is not something used by delete-selection.

> > > > +(defun delete-selection-uses-region-p ()
> > >
> > > The way I read it, this name says "a function which tells us if
> > > delete-selection uses the region".
> 
> > No idea what this is all about, ....
> 
> It's for when delete-selection-mode and electric-pair-mode are both
> enabled.  What was happening (in C Mode, etc.) was that the user
> would mark an object and type "(", expecting e-p-m to put a pair
> of parens around the marked object; however d-s-m got in first,
> and deleted (?killed) the region before the parens were put around
> the now empty region.
> 
> There was already a solution for this for when "(" is bound to
> self-insert-command - this is fairly arcane, and involves setting a
> particular variable to function which returns t in the pertinent
> circumstances.
> 
> This function was previously coded as a lambda.  However, I needed
> to use it for the pertinent CC Mode functions too, so I extracted
> it into a defun, giving it the name `delete-selection-uses-region-p'.  
> The "delete-seleection" bit is the prefix, shared by the other
> de\(fun\|var\)s in the file.

In what file?  Why is the prefix "delete-selection", and not
"electric-pair"?

> > .... but if this is for `electric-pair-mode' and not for
> > `delete-selection-mode' then the name should reflect that -\
> > call it `electric-pair-SOMETHING'.
> 
> It's for when BOTH minor modes are enabled, to enable them to play
> nicely with eachother.  But it's in delsel.el, hence the prefix.

It's for electric-pair to move into the delete-selection
neighborhood.  It's an electric-pair thing.  Delete-selection
mode doesn't need it - never has.

> > Is this something that `delete-selection-mode' needs?
> > Or is it for something else?
> 
> See above.

AFAICT, it is not for delete-selection mode.  It is a hack
so that electric-pair mode can adapt to delete-selection 
mode.  Nothing wrong with that, in principle.  When a new
feature is added, it often has to accommodate existing
behavior.  My point is that the name should reflect the
fact that this is a workaround for electric-pair mode, so
that it DTRT.  If you had to add another workaround for e-p
mode, so that it DTRT with commas or whatever, that would
be no different.  Such things are about e-p; their names
should reflect that.

If you also want to refer, in the name, to the thing you
are working around for e-p mode, that's OK, but the prefix
of the name should be e-p: `elec-pair-hack-to-tolerate-delsel'
or whatever.

> > > > +  "Return non-nil when the current command uses the region.
> 
> > What does it mean for a command to "use the region"?
> 
> That isn't very good, is it?  It basically means, from e-p-m's
> point of view "I'm going to be "using" the region, so don't
> you go and delete it!".

That still doesn't speak to what "using the region" means.
It sounds like what is important is that you are protecting
the region for e-p.  If so, the name and description should
reflect that need, not just "using the region".

> > > It's not about "the current command" but about
> > > self-insert-command (which may be the current command or
> > > may be called by the current command).
> 
> > What does it mean for such a command (or any other command) to
> > "use the region"?  That info should presumably be in the doc
> > string.
> 
> I think you're right, here.  I'll have another look at it.

Anyway, it's not very important.  Just one opinion.  I'd
recommend writing the explanation and names from the point
of view of e-p (only).

HTH - Drew