all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Drew Adams <drew.adams@oracle.com>
To: Alan Mackenzie <acm@muc.de>
Cc: "N. Jackson" <nljlistbox2@gmail.com>, 21275@debbugs.gnu.org
Subject: bug#21275: 24.5; Selection deleted with electric pair mode in cc mode
Date: Thu, 20 Aug 2015 22:46:42 -0700 (PDT)	[thread overview]
Message-ID: <550bc0c3-70ea-4b61-9d4b-b4e5656f49a3@default> (raw)
In-Reply-To: <20150820215506.GC3105@acm.fritz.box>

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





  reply	other threads:[~2015-08-21  5:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-16 13:31 bug#21275: 24.5; Selection deleted with electric pair mode in cc mode N. Jackson
     [not found] ` <mailman.8417.1439731993.904.bug-gnu-emacs@gnu.org>
2015-08-18 15:04   ` Alan Mackenzie
2015-08-18 16:43     ` N. Jackson
2015-08-19 21:52     ` Stefan Monnier
     [not found]     ` <jwv37zfosr4.fsf-monnier+emacsbugs@gnu.org>
2015-08-20 21:35       ` Drew Adams
2015-08-20 21:55         ` Alan Mackenzie
2015-08-21  5:46           ` Drew Adams [this message]
2015-08-21 13:42             ` Stefan Monnier
2015-08-21 15:01               ` Drew Adams
2015-08-19 17:22   ` Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=550bc0c3-70ea-4b61-9d4b-b4e5656f49a3@default \
    --to=drew.adams@oracle.com \
    --cc=21275@debbugs.gnu.org \
    --cc=acm@muc.de \
    --cc=nljlistbox2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.