unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#46469: 27.1; `isearch-del-char' should move point further back
@ 2021-02-12 18:56 Augusto Stoffel
  2021-02-12 20:00 ` Eli Zaretskii
  2021-02-13 18:31 ` Juri Linkov
  0 siblings, 2 replies; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-12 18:56 UTC (permalink / raw)
  To: 46469

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Make-isearch-del-char-backtrack-the-search-more-aggr.patch --]
[-- Type: text/x-patch, Size: 1509 bytes --]

From 1b1fad0d106390d907a805c9e8c81dd9831b5e1e Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Fri, 12 Feb 2021 19:37:14 +0100
Subject: [PATCH 2/2] Make `isearch-del-char' backtrack the search more
 aggressively

This allows to always find the first occurrence of the search string
after the last `isearch-repeat' or the start of search.
* lisp/isearch.el (isearch-del-char): Call `isearch-fallback' before
updating the search.
---
 lisp/isearch.el | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index aae4aeb261..b9c4cd2a65 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2475,13 +2475,9 @@ isearch-del-char
                                      isearch-string "")))
   ;; Do the following before moving point.
   (funcall (or isearch-message-function #'isearch-message) nil t)
-  ;; Use the isearch-other-end as new starting point to be able
-  ;; to find the remaining part of the search string again.
-  ;; This is like what `isearch-search-and-update' does,
-  ;; but currently it doesn't support deletion of characters
-  ;; for the case where unsuccessful search may become successful
-  ;; by deletion of characters.
-  (if isearch-other-end (goto-char isearch-other-end))
+  ;; Use `isearch-fallback' to be able to find an earlier occurrence
+  ;; of the remaining part of the search string.
+  (isearch-fallback nil nil t)
   (isearch-search)
   (isearch-push-state)
   (isearch-update))
-- 
2.29.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Small-correction-to-isearch-lazy-highlight-buffer-up.patch --]
[-- Type: text/x-patch, Size: 1297 bytes --]

From 3707102e4a87ae31105ca1dc71f0f1c349ed806f Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Fri, 12 Feb 2021 19:29:54 +0100
Subject: [PATCH 1/2] Small correction to
 `isearch-lazy-highlight-buffer-update'

The value of point is now read after a potential change of buffer.
* lisp/isearch.el (isearch-lazy-highlight-buffer-update): move call
to `point' after `select-window'.
---
 lisp/isearch.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index b58ca8a6f7..aae4aeb261 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -4127,13 +4127,13 @@ isearch-lazy-highlight-buffer-update
   "Update highlighting of other matches in the full buffer."
   (let ((max lazy-highlight-buffer-max-at-a-time)
         (looping t)
-        nomore window-start window-end
-        (opoint (point)))
+        nomore window-start window-end opoint)
     (with-local-quit
       (save-selected-window
 	(if (and (window-live-p isearch-lazy-highlight-window)
 		 (not (memq (selected-window) isearch-lazy-highlight-window-group)))
 	    (select-window isearch-lazy-highlight-window))
+        (setq opoint (point))
 	(setq window-start (window-group-start))
 	(setq window-end (window-group-end))
 	(save-excursion
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-12 18:56 bug#46469: 27.1; `isearch-del-char' should move point further back Augusto Stoffel
@ 2021-02-12 20:00 ` Eli Zaretskii
  2021-02-12 20:20   ` Augusto Stoffel
  2021-02-13 18:31 ` Juri Linkov
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-12 20:00 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

> From: Augusto Stoffel <arstoffel@gmail.com>
> 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"?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-12 20:00 ` Eli Zaretskii
@ 2021-02-12 20:20   ` Augusto Stoffel
  2021-02-13  7:04     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-12 20:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46469

On Fri, 12 Feb 2021 at 22:00, Eli Zaretskii <eliz@gnu.org> 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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-12 20:20   ` Augusto Stoffel
@ 2021-02-13  7:04     ` Eli Zaretskii
  2021-02-13  7:32       ` Augusto Stoffel
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-13  7:04 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: 46469@debbugs.gnu.org
> Date: Fri, 12 Feb 2021 21:20:36 +0100
> 
> On Fri, 12 Feb 2021 at 22:00, Eli Zaretskii <eliz@gnu.org> 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?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13  7:04     ` Eli Zaretskii
@ 2021-02-13  7:32       ` Augusto Stoffel
  2021-02-13  8:59         ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-13  7:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46469

On Sat, 13 Feb 2021 at 09:04, Eli Zaretskii <eliz@gnu.org> 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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13  7:32       ` Augusto Stoffel
@ 2021-02-13  8:59         ` Eli Zaretskii
  2021-02-13  9:53           ` Augusto Stoffel
  2021-02-13 23:14           ` bug#46469: [External] : " Drew Adams
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-13  8:59 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: 46469@debbugs.gnu.org
> Date: Sat, 13 Feb 2021 08:32:42 +0100
> 
> On Sat, 13 Feb 2021 at 09:04, Eli Zaretskii <eliz@gnu.org> 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.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13  8:59         ` Eli Zaretskii
@ 2021-02-13  9:53           ` Augusto Stoffel
  2021-02-13 13:28             ` Eli Zaretskii
  2021-02-13 23:14           ` bug#46469: [External] : " Drew Adams
  1 sibling, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-13  9:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46469

On Sat, 13 Feb 2021 at 10:59, Eli Zaretskii <eliz@gnu.org> wrote:

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

In this case, the distinction between being buggy or just weird and not
very useful is a bit blurry.  Let me note, however (assuming here that
DEL is bound to ‘isearch-del-char’):

1) The patched `isearch-del-char' serves the purpose of undoing
something you typed by mistake.  The result of ‘C-s y DEL x’ is
equivalent to the result of ‘C-s x’.  Just like, in normal editing,
typing ‘x DEL y’ is equivalent to typing ‘y’.

2) The current `isearch-del-char' serves a different purpose altogether.
‘C-s y DEL x’ is equivalent to ‘C-s y RET C-s x’, i.e., you are
searching for an ‘x’ occurring after an ‘y’.

As to this being just a personal preference: I'm sure you'll find people
attached to the current behavior if you poll the mailing list, but let
me note that, in other sensible search UIs, DEL works like the patched
‘isearch-del-char’.  I've just tested Swiper, gedit and zile.  They're
all like the patched version.  I see no reason for a defcustom here.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13  9:53           ` Augusto Stoffel
@ 2021-02-13 13:28             ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-13 13:28 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: 46469@debbugs.gnu.org
> Date: Sat, 13 Feb 2021 10:53:44 +0100
> 
> As to this being just a personal preference: I'm sure you'll find people
> attached to the current behavior if you poll the mailing list, but let
> me note that, in other sensible search UIs, DEL works like the patched
> ‘isearch-del-char’.  I've just tested Swiper, gedit and zile.  They're
> all like the patched version.  I see no reason for a defcustom here.

I'm afraid we have no choice, since the current behavior is very old.
We cannot make incompatible behavior changes in such old commands
unconditionally.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-12 18:56 bug#46469: 27.1; `isearch-del-char' should move point further back Augusto Stoffel
  2021-02-12 20:00 ` Eli Zaretskii
@ 2021-02-13 18:31 ` Juri Linkov
  2021-02-14  7:00   ` Augusto Stoffel
  1 sibling, 1 reply; 22+ messages in thread
From: Juri Linkov @ 2021-02-13 18:31 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13  8:59         ` Eli Zaretskii
  2021-02-13  9:53           ` Augusto Stoffel
@ 2021-02-13 23:14           ` Drew Adams
  2021-02-14  3:42             ` Eli Zaretskii
  1 sibling, 1 reply; 22+ messages in thread
From: Drew Adams @ 2021-02-13 23:14 UTC (permalink / raw)
  To: Eli Zaretskii, Augusto Stoffel; +Cc: 46469@debbugs.gnu.org

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13 23:14           ` bug#46469: [External] : " Drew Adams
@ 2021-02-14  3:42             ` Eli Zaretskii
  2021-02-14  7:18               ` Augusto Stoffel
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-14  3:42 UTC (permalink / raw)
  To: Drew Adams; +Cc: arstoffel, 46469

> From: Drew Adams <drew.adams@oracle.com>
> CC: "46469@debbugs.gnu.org" <46469@debbugs.gnu.org>
> 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.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-13 18:31 ` Juri Linkov
@ 2021-02-14  7:00   ` Augusto Stoffel
  2021-02-14 17:45     ` Juri Linkov
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-14  7:00 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 46469

Hi Juri,

On Sat, 13 Feb 2021 at 20:31, Juri Linkov <juri@linkov.net> 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.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14  3:42             ` Eli Zaretskii
@ 2021-02-14  7:18               ` Augusto Stoffel
  2021-02-14 15:24                 ` Eli Zaretskii
  2021-02-14 17:49                 ` Juri Linkov
  0 siblings, 2 replies; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-14  7:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 46469, Juri Linkov

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'm glad this is not the case.  Also,
`isearch-del-char' changed from one obscure key to another obscure key
in Emacs 27.  So clearly things can change.

Re this being a personal preference: I wouldn't bother to send a patch
if I thought so.  As already mentioned, lots of programs copied Emacs's
incremental search, and apparently all changed how DEL works in a
similar way.  This indicates that the patched `isearch-del-char' is what
most people expect (but I'm not suggesting to bind it to DEL, just have
it around for those who want it).

With all that said, could we discuss the merits of the change itself?

If it has a real drawback in comparison with the current
`isearch-del-char', then it would be fine to add a defcustom.  But I
suspect that 3 alternative ways to delete characters from a search
string is a bit over the top.  We can probably come up with a solid
default behavior here.

I already explained my rationale for wanting the change: 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.  Refer to my third message in this thread for details.

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?

Thanks,
Augusto





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14  7:18               ` Augusto Stoffel
@ 2021-02-14 15:24                 ` Eli Zaretskii
  2021-02-14 17:49                 ` Juri Linkov
  1 sibling, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-14 15:24 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469, juri

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Drew Adams <drew.adams@oracle.com>,  Juri Linkov <juri@linkov.net>,
>  46469@debbugs.gnu.org
> 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.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14  7:00   ` Augusto Stoffel
@ 2021-02-14 17:45     ` Juri Linkov
  2021-04-27 18:44       ` Augusto Stoffel
  0 siblings, 1 reply; 22+ messages in thread
From: Juri Linkov @ 2021-02-14 17:45 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14  7:18               ` Augusto Stoffel
  2021-02-14 15:24                 ` Eli Zaretskii
@ 2021-02-14 17:49                 ` Juri Linkov
  2021-02-15 19:31                   ` Augusto Stoffel
  1 sibling, 1 reply; 22+ messages in thread
From: Juri Linkov @ 2021-02-14 17:49 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

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





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14 17:49                 ` Juri Linkov
@ 2021-02-15 19:31                   ` Augusto Stoffel
  2021-02-15 19:41                     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-02-15 19:31 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 46469

On Sun, 14 Feb 2021 at 19:49, Juri Linkov <juri@linkov.net> wrote:

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

I've used `isearch-delete-char' for years and never really got used to
how it undoes a `C-s'.  It's too electric for me, somehow.

In any case, I can add a defcustom (or a third independent command,
which would be more or less equivalent) if anyone else has interest in
this.  But since that doesn't seem to be case, I would rather take back
my suggestion.

My interest was to make Isearch a (very tiny) bit more user-friendly,
but having 3 versions of delete would have the opposite effect, IMHO.
I can live with the current one, even if it's suboptimal.

Thanks,
Augusto





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: [External] : bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-15 19:31                   ` Augusto Stoffel
@ 2021-02-15 19:41                     ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-02-15 19:41 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469, juri

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Drew Adams <drew.adams@oracle.com>,
>   46469@debbugs.gnu.org
> Date: Mon, 15 Feb 2021 20:31:27 +0100
> 
> In any case, I can add a defcustom (or a third independent command,
> which would be more or less equivalent) if anyone else has interest in
> this.  But since that doesn't seem to be case, I would rather take back
> my suggestion.

As I said, I have no objection to supporting this behavior as an
option, especially since you believe (and I have no reason to doubt)
that others could like it.  Please don't feel discouraged that both
Juri and myself don't share your preferences: we are just 2 people.

> My interest was to make Isearch a (very tiny) bit more user-friendly,
> but having 3 versions of delete would have the opposite effect, IMHO.
> I can live with the current one, even if it's suboptimal.

I don't think what you suggest is really a 3rd command, it's more like
a minor variant of one of the 2 existing commands.  So I see no reason
to withdraw your proposal, certainly not because of the above.

Thanks.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-02-14 17:45     ` Juri Linkov
@ 2021-04-27 18:44       ` Augusto Stoffel
  2021-04-28 20:53         ` Juri Linkov
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-04-27 18:44 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 46469

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

For the record, the search ring advance/retreat commands suffer from a
bug similar to the `isearch-del-char' one described here.

Namely, `C-s M-p M-p M-p' presumably takes you to the first match, after
the starting point, of the third history element.  It would be very
strange to expect that the result of this key sequence depends on the
first and second history elements, or on the contents of the buffer
beyond the first match of the third history element.

And, in fact, everything is fine when `search-ring-update' is nil.
However:
  
(progn
  (setq search-ring-update t
        search-ring '("1" "2" "3")
        search-ring-yank-pointer nil)
  (save-excursion
    (insert "expect point here -> 3 2 1 2 3 <- but instead get here"))
  (isearch-mode t)
  (isearch-ring-retreat)
  (isearch-ring-retreat)
  (isearch-ring-retreat))

On Sun, 14 Feb 2021 at 19:45, Juri Linkov <juri@linkov.net> wrote:

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

Now I see, my old patch only worked in regexp mode (which is `C-s' for
me).  The attached one seems to work (but this is subtle stuff, there
may be edge cases).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-isearch-del-char-backtrack-the-search-more-aggr.patch --]
[-- Type: text/x-patch, Size: 1591 bytes --]

From a29f3c0b2b43af6b9c283205643a1e2ac43030b7 Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Fri, 12 Feb 2021 19:37:14 +0100
Subject: [PATCH] Make `isearch-del-char' backtrack the search more
 aggressively

This allows to always find the first occurrence of the search string
after the last `isearch-repeat' or the start of search.
* lisp/isearch.el (isearch-del-char): Go to barrier before updating
the search.
---
 lisp/isearch.el | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 9f3cfd70fb..f6a55c7918 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2505,13 +2505,12 @@ isearch-del-char
                                      isearch-string "")))
   ;; Do the following before moving point.
   (funcall (or isearch-message-function #'isearch-message) nil t)
-  ;; Use the isearch-other-end as new starting point to be able
-  ;; to find the remaining part of the search string again.
-  ;; This is like what `isearch-search-and-update' does,
-  ;; but currently it doesn't support deletion of characters
-  ;; for the case where unsuccessful search may become successful
-  ;; by deletion of characters.
-  (if isearch-other-end (goto-char isearch-other-end))
+  ;; Go to `isearch-barrier' to be able to find an earlier occurrence
+  ;; of the remaining part of the search string.
+  (if isearch-regexp
+      (isearch-fallback nil nil t)
+    (goto-char isearch-barrier)
+    (setq isearch-adjusted t))
   (isearch-search)
   (isearch-push-state)
   (isearch-update))
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-04-27 18:44       ` Augusto Stoffel
@ 2021-04-28 20:53         ` Juri Linkov
  2021-04-28 21:16           ` Augusto Stoffel
  0 siblings, 1 reply; 22+ messages in thread
From: Juri Linkov @ 2021-04-28 20:53 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

> For the record, the search ring advance/retreat commands suffer from a
> bug similar to the `isearch-del-char' one described here.
>
> Namely, `C-s M-p M-p M-p' presumably takes you to the first match, after
> the starting point, of the third history element.  It would be very
> strange to expect that the result of this key sequence depends on the
> first and second history elements, or on the contents of the buffer
> beyond the first match of the third history element.
>
> And, in fact, everything is fine when `search-ring-update' is nil.
> However:
>
> (progn
>   (setq search-ring-update t

Your proposed change makes sense.  Since this is a non-default value
of search-ring-update, I'm not sure how the users who customized it to t
would like such change.

>> I tried again, but your patch still doesn't work.  With
>> It signals the error "Keyboard macro terminated by a command ringing the bell".
>
> Now I see, my old patch only worked in regexp mode (which is `C-s' for
> me).  The attached one seems to work (but this is subtle stuff, there
> may be edge cases).

Thanks, now it's clear why it didn't work when I tried.

But this change will cause a problem to me.  Usually, I type 'C-s'
several times with a non-empty search string to arrive to Nth occurrence.
Then to be able to continue the search with a shorter string, I remove
some characters from the search string with 'C-M-d' (isearch-del-char).
Then continue searching with 'C-s' from the same place.  But with the patch,
'C-M-d' unexpectedly jumps to a previous match, and every next 'C-M-d'
jumps back it its previous match that makes no sense.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-04-28 20:53         ` Juri Linkov
@ 2021-04-28 21:16           ` Augusto Stoffel
  2021-04-28 21:37             ` Juri Linkov
  0 siblings, 1 reply; 22+ messages in thread
From: Augusto Stoffel @ 2021-04-28 21:16 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 46469

> But this change will cause a problem to me.  Usually, I type 'C-s'
> several times with a non-empty search string to arrive to Nth occurrence.
> Then to be able to continue the search with a shorter string, I remove
> some characters from the search string with 'C-M-d' (isearch-del-char).
> Then continue searching with 'C-s' from the same place.  But with the patch,
> 'C-M-d' unexpectedly jumps to a previous match, and every next 'C-M-d'
> jumps back it its previous match that makes no sense.

I can verify this.  But I think you just found a related but independent
issue.  Consider this example:

(progn
  (save-excursion (insert "a1 b2 a3"))
  (isearch-mode t t)
  (isearch-printing-char ?a)
  (isearch-repeat-forward)
  (isearch-printing-char ?\\)
  (isearch-printing-char ?|)
  (isearch-printing-char ?b))

When "\\|b" is added to search string, the point is at "3".  So it
shouldn't move, right?  But instead it goes back to "2".

Do you get the same behavior?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#46469: 27.1; `isearch-del-char' should move point further back
  2021-04-28 21:16           ` Augusto Stoffel
@ 2021-04-28 21:37             ` Juri Linkov
  0 siblings, 0 replies; 22+ messages in thread
From: Juri Linkov @ 2021-04-28 21:37 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 46469

>> But this change will cause a problem to me.  Usually, I type 'C-s'
>> several times with a non-empty search string to arrive to Nth occurrence.
>> Then to be able to continue the search with a shorter string, I remove
>> some characters from the search string with 'C-M-d' (isearch-del-char).
>> Then continue searching with 'C-s' from the same place.  But with the patch,
>> 'C-M-d' unexpectedly jumps to a previous match, and every next 'C-M-d'
>> jumps back it its previous match that makes no sense.
>
> I can verify this.  But I think you just found a related but independent
> issue.  Consider this example:
>
> (progn
>   (save-excursion (insert "a1 b2 a3"))
>   (isearch-mode t t)
>   (isearch-printing-char ?a)
>   (isearch-repeat-forward)
>   (isearch-printing-char ?\\)
>   (isearch-printing-char ?|)
>   (isearch-printing-char ?b))
>
> When "\\|b" is added to search string, the point is at "3".  So it
> shouldn't move, right?  But instead it goes back to "2".
>
> Do you get the same behavior?

I tested it in non-regexp mode, and the aforementioned use case is also
for non-regexp mode.

But in the regexp mode, maybe what you found is a bug.





^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-04-28 21:37 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12 18:56 bug#46469: 27.1; `isearch-del-char' should move point further back Augusto Stoffel
2021-02-12 20:00 ` Eli Zaretskii
2021-02-12 20:20   ` Augusto Stoffel
2021-02-13  7:04     ` Eli Zaretskii
2021-02-13  7:32       ` Augusto Stoffel
2021-02-13  8:59         ` Eli Zaretskii
2021-02-13  9:53           ` Augusto Stoffel
2021-02-13 13:28             ` Eli Zaretskii
2021-02-13 23:14           ` bug#46469: [External] : " Drew Adams
2021-02-14  3:42             ` Eli Zaretskii
2021-02-14  7:18               ` Augusto Stoffel
2021-02-14 15:24                 ` Eli Zaretskii
2021-02-14 17:49                 ` Juri Linkov
2021-02-15 19:31                   ` Augusto Stoffel
2021-02-15 19:41                     ` Eli Zaretskii
2021-02-13 18:31 ` Juri Linkov
2021-02-14  7:00   ` Augusto Stoffel
2021-02-14 17:45     ` Juri Linkov
2021-04-27 18:44       ` Augusto Stoffel
2021-04-28 20:53         ` Juri Linkov
2021-04-28 21:16           ` Augusto Stoffel
2021-04-28 21:37             ` Juri Linkov

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).