RE: PATCH: isearch-yank-until-char

[Drew Adams <drew.adams@oracle.com>]

> Remember that you started out that new thread
> (on August 14th) with these words:
> 
> > This is similar to what Karl submitted today.
> > Not a replacement for that; something different.

That was only the intro sentence.  Yes, the
general idea is something different, additional.
Which is why I created a new thread.

But the message also directly relates to your
command, which was the starting point.  A few
sentences later I said this:

  I think Karl's `isearch-yank-until-char'
  should also be made to support backward
  search.

And the patch modifies your command to do that.
And the message argues for a different key for
your command (after my earlier message where I
said that `C-M-c' was OK by me).

The intro sentence could perhaps have made clear
that it enhances your command, but that it's not
limited to doing that.

The more important message is that it provides
something new that applies generally.

> Since your followups were all in that thread,
> I assumed they were about something that was
> "not a replacement" for what I'd posted, and
> therefore wouldn't affect the question of
> whether what I posted should be installed.
> (I rely pretty heavily on thread discipline,
> not just in Emacs Devel but generally.)

Fair enough.  But my message really does relate
to your command and patch.  That's not the main
message, but it's closely related to that message.

> > Commands like `isearch-yank-until-char', which
> > yank consecutive buffer text at the search
> > point, are better if they can also work with
> > backward search.  My patch implements that for
> > this command and others.
> 
> It sounds like your patch does two conceptually
> distinct things:
> 
> 1) Implement one or more new commands
     [which work with backward search].
> 2) Make a bunch of isearch commands work
>    with backward search.

Yes, if you want to look at it that way.  But
switch the order, and then see if you still see
them as so different/separate.

It lets yank commands work properly with
backward search.  And so it fixes the relevant
yank commands to do that.  There are only a few
yank commands that are relevant (not really "a
bunch").

My message also categorizes Isearch yank commands,
saying that the relevant ones here are those that
yank buffer text at the search hit.  That is, it
makes clear that this doesn't apply to ad hoc
yanks, such as yanking from the kill-ring.

> If I were to install my patch (as it's currently
> written, though maybe with the keybinding changed),
> that doesn't really affect any of your new code.

Correct, with the exception of the key binding.

But my patch subsumes your patch (except that it
doesn't address the manuals or News).

It seemed appropriate to fix your command at the
same time as the others, especially since it was
still being discussed.

> However, I would still say your patch should be
> divided into two conceptually distinct patches,
> one for (1) and one for (2).  This is not a
> judgement about the technical merits or the UX
> merits of your patch.  I'm just saying that we
> should do one thing at a time.

I could do that.  But they're not so separate.

I added the Boolean option because Juri requested
it for the existing commands, to not change their
default behavior.  (I don't think such an option
is needed, but I respected that request.)

I could also split the patch into 3, splitting
off the command-defining macro and the two
commands it defines.  Or I could split it into 4.

Different approaches are possible.  I thought
(and think) it makes more sense to present all
of those changes together with a general
explanation of the feature - they all go together.

If someone wants to understand the behavior and
rationale, it makes sense to have code that
demonstrates it.

But if you want to try the patch and ignore the
changes to existing commands, that's possible,
just by setting the option value to nil.  Or if
you want to try it and ignore the new commands,
you can do that too.

If you consider them "conceptually distinct"
then just use the option to separate them.

And if it really helps to split the patch, I
can do that.

> >And I argued that `C-M-c' would be better used in
> >Isearch for my command `isearch-yank-through-move',
> >which initiates a recursive edit to allow arbitrary
> >cursor movement.  In that case, `C-M-c' both starts
> >and ends such movement (since globally it is
> >`exit-recursive-edit').
> 
> *nod*  That's a separate question from the above.
> I don't think it's very important that C-M-c be the
> binding for `isearch-yank-until-char'.  If we want
> to save C-M-c for this other potential use, that
> seems reasonable to me.

Good to hear.

> Your patch suggested "C-M-." for
> 'isearch-yank-until-char', which seems good.
> Another possibility, to keep the "c" for "char"
> mnemonic, is to use M-c.

I'm open on the key for `isearch-yank-until-char',
except that I think it makes sense to save `C-M-c'
in case we have a command that involves using that
to end recursive editing.

There are keys available.  The usual problem with
Isearch is that people will argue, with reason,
that they like to use such-and-such a key to exit
Isearch and act on the buffer text.

As you say, in Isearch `M-c' is currently for
toggling case.  If it were not, I could imagine
someone saying that s?he wants to continue using
`M-c' as `capitalize-word', to end search and
capitalize the word at the destination.

Incidentally, this is why, in `isearch+.el', I
put all Isearch yank commands on a prefix key,
`C-y'.  (I have more Isearch yank commands.)

Some, like `isearchp-yank-sexp-symbol-or-char',
are also on other keys. for ease of use - e.g.
`C-(' as well as `C-y C-('.

> Right now that key seems to toggle
> case-sensitivity, but I'm not sure
> that's deliberate -- according to the
> `isearch-forward' documentation, "M-s c" is
> for that, while "M-c" isn't documented at all.
> Given that the current action of M-c isn't
> documented, and given that another keybinding
> both does that action and is documented to do
> so, using "M-c" for `isearch-yank-until-char'
> might be okay.

I'm all for that kind of discussion - finding a
good fit.

> The code above suggests that it is not
> important for M-c to remain redundantly bound
> to `isearch-toggle-case-fold', but I could be
> wrong.  If anyone knows more, please say.  If
> we can't figure out the answer, I guess I'd say
> let's go with "C-M-.", out of general conservatism.

I like the fact that you provide reasons, and you
discuss openly and with details.

I suggest that we run a discussion about possible
key bindings for this command and others in
parallel with the overall discussion that's begun
about Isearch yank commands.

As opposed, e.g., to trying to first decide about
a key for `isearch-yank-until-char'.  Let's work
out the commands first - including consideration
of my proposed commands, and then figure out what
keys to use.  Just a suggestion.

And given that we're discussing adding to the set
of yank commands, maybe we want to consider
putting them all on some prefix key.  That would
certainly give us more options.