Re: PATCH: isearch-yank-until-char

[Karl Fogel]

On 26 Aug 2019, Drew Adams wrote:
>> 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.

The ordering matters in terms of workload to me, though.  If I install my 
patch, which is fairly small and self-contained, then all the other things in 
your patch could be installed on top of it (following discussion, consensus, 
etc).  If we arrange the dependency flow this way, then my further 
participation becomes optional -- which is good, because I have only very 
limited time to work on Emacs.

>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.

Right.  So let's have it subsume my patch by coming after my patch :-).  Hence 
my "do one thing at a time" preference: that significantly reduces the amount 
of work for me, while not increasing it (or only increasing it a tiny amount) 
for you or anyone else.

>> 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.

That's an interesting discussion, but not one I have time to get into.  If I 
install my original patch (with an updated keybinding), then those who have 
time to have that discussion can do so, and it will be the same discussion as 
it would be otherwise.

>> 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.

I agree.

>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.

I don't really know if people still exit isearch by directly starting their 
next command.  For many years now, I have always exited it explicitly with RET, 
because there are so many keybindings already claimed by isearch-mode that it's 
not worth it to me any more to keep track of which ones aren't yet claimed -- 
even ones that aren't claimed now might be claimed tomorrow, so I figured it 
was best to just get in the habit of using the designated exit binding, RET, to 
leave the search.

However, maybe other people might do something like M-c to exit and 
capitalize-word all at once.

The global binding for "C-M-." is `xref-find-apropos', which is extremely 
unlikely to be the command with which someone exits isearch (indeed, it is at 
least *possible* that no one in history has ever exited isearch that way), so I 
think I like it as a keybinding.  Thanks for the suggestion.

An updated patch is attached.  The only new changes in this patch are:

* Updated the keybinding in isearch-mode-map to C-M-.

* Make the search for the character case-sensitive.  (Seems like a pretty 
obvious improvement, given the use cases: when the goal character is a letter 
at all, one is either looking at that letter on the screen *or* the letter is 
some known syntactic delimiter and its case is therefore known as well even if 
the letter is off the screen right now.)

>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.

If "in parallel" means "separately but in parallel", then yes.  I'll probably 
only take part in the first discussion.

And that discussion may be done already, as I have now updated the keybinding 
to M-C-. for the (good) reason you gave.

>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.

Oh, you meant "in parallel and together".  I'd like to avoid that.  My goal is 
to keep the discussions as separate as possible, and I believe they are 
separate now that the keybinding issue is resolved.

>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.

That's way more change than I wanted to get into, and I don't think it's 
necessary (for my original patch), since M-C-. is not taking away a likely 
isearch exit.

If there is a discussion to be had about having an isearch-mode prefix key, 
let's please keep it separate.  It is related to your larger patch, perhaps, 
but not to my original small one.

New patch attached.

Best regards,
-Karl

isearch-yank-until-char-20190826.txt
Description: Text document