bug#46469: 27.1; `isearch-del-char' should move point further back

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Augusto Stoffel
Suppose my buffer contains the text "x1yx2" right after point, and that
`isearch-del-char' is bound to DEL in Isearch mode.  If I type

   C-s y DEL x1

then I would expect the search to succeed: the cursor would first go
after "y", then return to the starting point of search after pressing
DEL – this is how `isearch-delete-char' behaves, anyway.

Instead, the cursor remains after "y" after pressing DEL, and the search
fails with the cursor after the second "x".  I have attached a patch to
fix this.

I have also attached a second trivial patch for an unrelated, very tiny
bug.

Best,
Augusto


0002-Make-isearch-del-char-backtrack-the-search-more-aggr.patch (1K) Download Attachment
0001-Small-correction-to-isearch-lazy-highlight-buffer-up.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Eli Zaretskii
> From: Augusto Stoffel <[hidden email]>
> Date: Fri, 12 Feb 2021 19:56:36 +0100
>
> Suppose my buffer contains the text "x1yx2" right after point, and that
> `isearch-del-char' is bound to DEL in Isearch mode.  If I type
>
>    C-s y DEL x1
>
> then I would expect the search to succeed: the cursor would first go
> after "y", then return to the starting point of search after pressing
> DEL – this is how `isearch-delete-char' behaves, anyway.
>
> Instead, the cursor remains after "y" after pressing DEL, and the search
> fails with the cursor after the second "x".  I have attached a patch to
> fix this.

I cannot reproduce this.  Do you see this in "emacs -Q"?



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Augusto Stoffel
On Fri, 12 Feb 2021 at 22:00, Eli Zaretskii <[hidden email]> wrote:

> I cannot reproduce this.  Do you see this in "emacs -Q"?

Could you try hitting ‘C-x C-e’ with the point at the beginning of the
fourth line of the following text?

(progn
  (define-key isearch-mode-map (kbd "DEL") 'isearch-del-char)
  (execute-kbd-macro [?\C-s ?y backspace ?x ?1]))
x1yx2

In "emacs -Q" I get the error message "Keyboard macro terminated by a
command ringing the bell".

With my patch, there's no error and the point ends on "y", as expected.

Best,
Augusto



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Eli Zaretskii
> From: Augusto Stoffel <[hidden email]>
> Cc: [hidden email]
> Date: Fri, 12 Feb 2021 21:20:36 +0100
>
> On Fri, 12 Feb 2021 at 22:00, Eli Zaretskii <[hidden email]> wrote:
>
> > I cannot reproduce this.  Do you see this in "emacs -Q"?
>
> Could you try hitting ‘C-x C-e’ with the point at the beginning of the
> fourth line of the following text?
>
> (progn
>   (define-key isearch-mode-map (kbd "DEL") 'isearch-del-char)
>   (execute-kbd-macro [?\C-s ?y backspace ?x ?1]))
> x1yx2
>
> In "emacs -Q" I get the error message "Keyboard macro terminated by a
> command ringing the bell".
>
> With my patch, there's no error and the point ends on "y", as expected.

I think I understand the issue now: you meant isearch-del-char,
whereas I thought you meant isearch-delete-char (which is bound to DEL
by default).  Sorry, I was reading the body of your report and didn't
pay attention to the Subject.

For isearch-del-char, I think what you see is the intended behavior:
that command doesn't undo the effect of the last character you type at
the I-search prompt, it just removes the last character of the search
string.  So it isn't supposed to move point back to where the search
started.  What you wanted to happen is what isearch-delete-char does.

OK?



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Augusto Stoffel
On Sat, 13 Feb 2021 at 09:04, Eli Zaretskii <[hidden email]> wrote:

> For isearch-del-char, I think what you see is the intended behavior:
> that command doesn't undo the effect of the last character you type at
> the I-search prompt, it just removes the last character of the search
> string.  So it isn't supposed to move point back to where the search
> started.  What you wanted to happen is what isearch-delete-char does.
>
> OK?

No, isearch-delete-char is a misnamed command.  It undoes the last
isearch command, which is typically inserting a character OR going to
the next/previous match.  If you type ‘C-s y C-s DEL’ in emacs -q, the
search string is still y.

I frankly think DEL should be isearch-del-char by default.  I never got
used to isearch-delete-char in many years before finding out about
isearch-del-char.  But that's besides the point.

As to why the patched version of isearch-delete-char is the "expected"
or more useful one: this is to a degree a matter of taste, but I can
give my reasons if you are still not convinced.

Best,
Augusto



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Eli Zaretskii
> From: Augusto Stoffel <[hidden email]>
> Cc: [hidden email]
> Date: Sat, 13 Feb 2021 08:32:42 +0100
>
> On Sat, 13 Feb 2021 at 09:04, Eli Zaretskii <[hidden email]> wrote:
>
> > For isearch-del-char, I think what you see is the intended behavior:
> > that command doesn't undo the effect of the last character you type at
> > the I-search prompt, it just removes the last character of the search
> > string.  So it isn't supposed to move point back to where the search
> > started.  What you wanted to happen is what isearch-delete-char does.
> >
> > OK?
>
> No, isearch-delete-char is a misnamed command.

I could agree with that, but it's a tangent.

> It undoes the last isearch command, which is typically inserting a
> character OR going to the next/previous match.  If you type ‘C-s y
> C-s DEL’ in emacs -q, the search string is still y.

That's true; you need to type DEL twice to actually remove the last
character.  But that's how that command was supposed to work, and it
does make sense to me, at least.

> I frankly think DEL should be isearch-del-char by default.  I never got
> used to isearch-delete-char in many years before finding out about
> isearch-del-char.  But that's besides the point.
>
> As to why the patched version of isearch-delete-char is the "expected"
> or more useful one: this is to a degree a matter of taste, but I can
> give my reasons if you are still not convinced.

If we are talking about personal preferences, then I suggest to make
your desired behavior an opt-in one by introducing a defcustom.  We
cannot change the behavior of isearch-del-char unconditionally if the
existing behavior is not a bug, but just something some users may not
like.

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Juri Linkov-2
In reply to this post by Augusto Stoffel
> Suppose my buffer contains the text "x1yx2" right after point, and that
> `isearch-del-char' is bound to DEL in Isearch mode.  If I type
>
>    C-s y DEL x1
>
> then I would expect the search to succeed: the cursor would first go
> after "y", then return to the starting point of search after pressing
> DEL – this is how `isearch-delete-char' behaves, anyway.
>
> Instead, the cursor remains after "y" after pressing DEL, and the search
> fails with the cursor after the second "x".  I have attached a patch to
> fix this.

Sorry, the reason for the current behavior is explained in the comments.
Anyway isearch-fallback in your patch does nothing.  Maybe you intended
to use isearch-pop-state, but this what isearch-delete-char already does.

> I have also attached a second trivial patch for an unrelated, very tiny
> bug.

Thanks, this fix is now pushed to master.



Reply | Threaded
Open this post in threaded view
|

bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back

Drew Adams
In reply to this post by Eli Zaretskii
> If we are talking about personal preferences, then I suggest to make
> your desired behavior an opt-in one by introducing a defcustom.  We
> cannot change the behavior of isearch-del-char unconditionally if the
> existing behavior is not a bug, but just something some users may not
> like.

If we are talking about personal preferences
(and I think that's the case), a user can just
bind their preferred keys to their preferred
commands in `isearch-mode-map'.
Reply | Threaded
Open this post in threaded view
|

bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back

Eli Zaretskii
> From: Drew Adams <[hidden email]>
> CC: "[hidden email]" <[hidden email]>
> Date: Sat, 13 Feb 2021 23:14:48 +0000
> Accept-Language: en-US
>
> > If we are talking about personal preferences, then I suggest to make
> > your desired behavior an opt-in one by introducing a defcustom.  We
> > cannot change the behavior of isearch-del-char unconditionally if the
> > existing behavior is not a bug, but just something some users may not
> > like.
>
> If we are talking about personal preferences
> (and I think that's the case), a user can just
> bind their preferred keys to their preferred
> commands in `isearch-mode-map'.

We were discussing a possible change to an existing commands.
Keybindings cannot change the commends to which they are bound.



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Augusto Stoffel
In reply to this post by Juri Linkov-2
Hi Juri,

On Sat, 13 Feb 2021 at 20:31, Juri Linkov <[hidden email]> wrote:

> Anyway isearch-fallback in your patch does nothing.  Maybe you intended
> to use isearch-pop-state, but this what isearch-delete-char already does.

Please rest assured that the patch works, or at least works in all test
cases I could come up with.  It implements the behavior I described in
detail in my third message in this thread.

Put in another way, the patch makes ‘C-s y DEL x’ pretty much equivalent
to ‘C-s y☺\|x’, where DEL stands for `isearch-del-char' and ☺ stands for
an unmatchable regexp.



Reply | Threaded
Open this post in threaded view
|

bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back

Eli Zaretskii
In reply to this post by Eli Zaretskii
> From: Augusto Stoffel <[hidden email]>
> Cc: Drew Adams <[hidden email]>,  Juri Linkov <[hidden email]>,
>  [hidden email]
> Date: Sun, 14 Feb 2021 08:18:23 +0100
>
> Re the idea that we can't change the behavior of an old command: if
> taken too seriously, this principle would imply that the standard Emacs
> UI can never improve

I didn't say that behavior of old commands cannot change, I said it
cannot change _unconditionally_.  That is, users should have a way to
revert to the old behavior.  It is generally preferable to make any
change in behavior opt-in; but even if we decide to make it the
default, there should be a way to go back.  That is because Emacs is a
very old and stable program, with users who are used to its present
behavior, and we try very hard not to break things that were working
for many years, nor change them in incompatible ways, since such
changes tend rightfully annoy veteran users.

In this particular case, whatever new aspects of the behavior you
suggest to add, they should be conditioned on a user option, and users
should be told in NEWS about the new behavior and the way to go back
to the old one (if the new one is made the default).

> Also, `isearch-del-char' changed from one obscure key to another
> obscure key in Emacs 27.  So clearly things can change.

It is IMO unfortunate that so many changes are happening lately in the
Isearch area regarding key bindings.  But that doesn't mean we should
make more changes there too lightly, let alone make them
unconditional.  At least keybindings can be easily undone on the user
level.

> Re this being a personal preference: I wouldn't bother to send a patch
> if I thought so.

"Personal preference" should not be interpreted to mean you are the
only person to prefer that.  Rather, it means that some people may
want that and some won't.  At least two people in this thread opined
that the existing behavior is fine, so clearly at least some people
would like to have the existing behavior.  Which doesn't mean what you
suggest doesn't have its place, it just means it isn't the only
opinion among Emacs users.

Where some users prefer one behavior and some prefer another, a user
option to control which behavior actually happens is a good way to
allow everyone to have what they want at a price of flipping a
variable.

> Is there a case where the current behavior is much more convenient
> and/or takes the search to a state that can't be easily reproduced by
> the patched version?

It is more convenient to me because I'm used to it: when I want to
remove the last character from the search string, I type DEL twice
without even looking.  I'm guessing there are others who have the same
habits.



Reply | Threaded
Open this post in threaded view
|

bug#46469: 27.1; `isearch-del-char' should move point further back

Juri Linkov-2
In reply to this post by Augusto Stoffel
>> Anyway isearch-fallback in your patch does nothing.  Maybe you intended
>> to use isearch-pop-state, but this what isearch-delete-char already does.
>
> Please rest assured that the patch works, or at least works in all test
> cases I could come up with.  It implements the behavior I described in
> detail in my third message in this thread.
>
> Put in another way, the patch makes ‘C-s y DEL x’ pretty much equivalent
> to ‘C-s y☺\|x’, where DEL stands for `isearch-del-char' and ☺ stands for
> an unmatchable regexp.

I tried again, but your patch still doesn't work.  With

(progn
  (define-key isearch-mode-map (kbd "DEL") 'isearch-del-char)
  (execute-kbd-macro [?\C-s ?y backspace ?x ?1]))
x1yx2

It signals the error "Keyboard macro terminated by a command ringing the bell".



Reply | Threaded
Open this post in threaded view
|

bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back

Juri Linkov-2
In reply to this post by Eli Zaretskii
> I mistype things, but I never hit ‘C-s’ by mistake.  So I want a way
> to undo just what I typed.  The current `iserach-del-char' does
> something slightly different.

I wonder why you don't use `isearch-delete-char' in this case?
It does exactly what you want - it undoes what you typed.