[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: PATCH: isearch-yank-until-char
From: |
Karl Fogel |
Subject: |
Re: PATCH: isearch-yank-until-char |
Date: |
Mon, 26 Aug 2019 00:20:58 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
On 24 Aug 2019, Drew Adams wrote:
>> I've left the code as-is; the current patch is attached here.
>> Do we have consensus to install it?
>
>Well, I don't agree, FWIW. You seem to have ignored
>my message & patch (Aug 20). Did you read or try it?
>(You were cc'd: "RE: PATCH: isearch-yank-until-match".)
No, I didn't. I mean, I saw it, but didn't have time to try it. 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.
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.)
>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.
2) Make a bunch of isearch commands work with backward search.
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.
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.
>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.
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.
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.
The code and comments around "M-c" vs "M-s c" are a little more complex than I
expected. Here's the relevant block from the definition of `isearch-mode-map':
;; Some bindings you may want to put in your isearch-mode-hook.
;; Suggest some alternates...
(define-key map "\M-c" 'isearch-toggle-case-fold)
(define-key map "\M-r" 'isearch-toggle-regexp)
(define-key map "\M-e" 'isearch-edit-string)
(put 'isearch-toggle-case-fold :advertised-binding "\M-sc")
(put 'isearch-toggle-regexp :advertised-binding "\M-sr")
(put 'isearch-edit-string :advertised-binding "\M-se")
Both bindings actually work. But you won't see "\M-sc" explicitly bound
anywhere in the map, even though it's the advertised binding! Instead, the
actual binding happens elsewhere via a call to the macro
`isearch-define-mode-toggle':
(isearch-define-mode-toggle case-fold "c" nil
...)
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.
Best regards,
-Karl
- Re: PATCH: isearch-yank-until-char, (continued)
- Re: PATCH: isearch-yank-until-char, Noam Postavsky, 2019/08/14
- Re: PATCH: isearch-yank-until-char, Juri Linkov, 2019/08/14
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/16
- Re: PATCH: isearch-yank-until-char, Juri Linkov, 2019/08/16
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/24
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/24
- Re: PATCH: isearch-yank-until-char, Juri Linkov, 2019/08/25
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/25
- Re: PATCH: isearch-yank-until-char,
Karl Fogel <=
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/26
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/26
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/26
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/26
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/26
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/26
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/26
- Re: PATCH: isearch-yank-until-char, Juri Linkov, 2019/08/26
- Re: PATCH: isearch-yank-until-char, Karl Fogel, 2019/08/26
- RE: PATCH: isearch-yank-until-char, Drew Adams, 2019/08/26