unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#7534: 24.0.50; G-g within Isearch regexp mode
@ 2010-12-02 19:47 Dani Moncayo
  2010-12-02 22:31 ` Juri Linkov
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Dani Moncayo @ 2010-12-02 19:47 UTC (permalink / raw)
  To: 7534

Recipe:
0) Start Emacs (-Q)
1) Place point at the beginning of the *scratch* buffer.
2) Search for the regexp "iss" (C-M-s iss) --> The last `s' is
unmatched as expected, OK.
3) Type C-g --> The unmatched part disappears. Everything OK so far.
4) Now repeat steps #2 and #3 with the regexp "is[". --> This time C-g
behaves differently. Instead of deleting the unmatched part, it ends
Isearch mode.


The fail seems to appear whenever `[' is the first character in the
unmatched part. For instance:
 * C-M-s iss[ C-g --> Works as expected (unmatched part deleted).
 * C-M-s is[blablabla --> Doesn't work as expected (Isearch cancelled).


-- 
Dani Moncayo





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-02 19:47 bug#7534: 24.0.50; G-g within Isearch regexp mode Dani Moncayo
@ 2010-12-02 22:31 ` Juri Linkov
  2010-12-03  7:57   ` Dani Moncayo
  2011-01-15  9:57 ` bug#7534: C-g in " Dani Moncayo
  2011-02-04  7:54 ` bug#7534: what about committing the patch also to emacs-23? Dani Moncayo
  2 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2010-12-02 22:31 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 7534

> Recipe:
> 0) Start Emacs (-Q)
> 1) Place point at the beginning of the *scratch* buffer.
> 2) Search for the regexp "iss" (C-M-s iss) --> The last `s' is
> unmatched as expected, OK.
> 3) Type C-g --> The unmatched part disappears. Everything OK so far.
> 4) Now repeat steps #2 and #3 with the regexp "is[". --> This time C-g
> behaves differently. Instead of deleting the unmatched part, it ends
> Isearch mode.
>
> The fail seems to appear whenever `[' is the first character in the
> unmatched part. For instance:
>  * C-M-s iss[ C-g --> Works as expected (unmatched part deleted).
>  * C-M-s is[blablabla --> Doesn't work as expected (Isearch cancelled).

IMO, this is consistent behavior.  As the message says after typing `['
it's incomplete input, so C-g behaves exactly like if there is no input
for `[' (and an unfinished sequence of characters that follows it).

So the main principle here is that "incomplete input" means "no input"
for Isearch, and given this rule all your examples work as expected, i.e.

"C-M-s is[ C-g" works like "C-M-s is C-g"
"C-M-s iss[ C-g" works like "C-M-s iss C-g"
"C-M-s is[blablabla C-g" works like "C-M-s is C-g"





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-02 22:31 ` Juri Linkov
@ 2010-12-03  7:57   ` Dani Moncayo
  2010-12-05 23:11     ` Juri Linkov
  0 siblings, 1 reply; 13+ messages in thread
From: Dani Moncayo @ 2010-12-03  7:57 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 7534

On Thu, Dec 2, 2010 at 23:31, Juri Linkov <juri@jurta.org> wrote:
[...]
>
> IMO, this is consistent behavior.  As the message says after typing `['
> it's incomplete input, so C-g behaves exactly like if there is no input
> for `[' (and an unfinished sequence of characters that follows it).
>
> So the main principle here is that "incomplete input" means "no input"
> for Isearch, and given this rule all your examples work as expected, i.e.
>
> "C-M-s is[ C-g" works like "C-M-s is C-g"
> "C-M-s iss[ C-g" works like "C-M-s iss C-g"
> "C-M-s is[blablabla C-g" works like "C-M-s is C-g"
>

Thanks for thinking on this, Juri.

Indeed, the current behaviour seems to be like you mentioned
("incomplete input" means "no input"), but IMO that doesn't fit well
user's needs. If the user is searching for "is[blablabla", and
suddenly changes his(her) mind, I think C-g should allow to get rid of
the unmatched part.

So, IMO, the main principle for C-g (within Isearch mode) should be:
* if there is unmatched and/or incomplete input --> Delete it.
* Otherwise --> Exit Isearch mode.

-- 
Dani Moncayo





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-03  7:57   ` Dani Moncayo
@ 2010-12-05 23:11     ` Juri Linkov
  2010-12-06 10:41       ` Dani Moncayo
  0 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2010-12-05 23:11 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 7534

> So, IMO, the main principle for C-g (within Isearch mode) should be:
> * if there is unmatched and/or incomplete input --> Delete it.
> * Otherwise --> Exit Isearch mode.

Please try the following patch.  Does it provide the behavior
you are asking for?

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2010-10-02 22:37:21 +0000
+++ lisp/isearch.el	2010-12-05 23:08:19 +0000
@@ -1253,7 +1253,7 @@ (defun isearch-abort ()
   (interactive)
 ;;  (ding)  signal instead below, if quitting
   (discard-input)
-  (if isearch-success
+  (if (and isearch-success (not isearch-error))
       ;; If search is successful, move back to starting point
       ;; and really do quit.
       (progn





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-05 23:11     ` Juri Linkov
@ 2010-12-06 10:41       ` Dani Moncayo
  2010-12-25  2:47         ` Juri Linkov
  0 siblings, 1 reply; 13+ messages in thread
From: Dani Moncayo @ 2010-12-06 10:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 7534

On Mon, Dec 6, 2010 at 00:11, Juri Linkov <juri@jurta.org> wrote:
>> So, IMO, the main principle for C-g (within Isearch mode) should be:
>> * if there is unmatched and/or incomplete input --> Delete it.
>> * Otherwise --> Exit Isearch mode.
>
> Please try the following patch.  Does it provide the behavior
> you are asking for?
>

It does. Thank you.

PS: The patch bellow is like your's, but it also updates the comment
accordingly.


=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2010-10-16 14:11:06.000000000 +0200
+++ lisp/isearch.el	2010-12-06 11:27:33.000000000 +0100
@@ -1244,9 +1244,9 @@
   (interactive)
 ;;  (ding)  signal instead below, if quitting
   (discard-input)
-  (if isearch-success
-      ;; If search is successful, move back to starting point
-      ;; and really do quit.
+  (if (and isearch-success (not isearch-error))
+      ;; If search is successful and there is no incomplete regexp,
+      ;; move back to starting point and really do quit.
       (progn
         (setq isearch-success nil)
         (isearch-cancel))




-- 
Dani Moncayo





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-06 10:41       ` Dani Moncayo
@ 2010-12-25  2:47         ` Juri Linkov
  2011-01-04 11:42           ` Dani Moncayo
  0 siblings, 1 reply; 13+ messages in thread
From: Juri Linkov @ 2010-12-25  2:47 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: 7534

>>> So, IMO, the main principle for C-g (within Isearch mode) should be:
>>> * if there is unmatched and/or incomplete input --> Delete it.
>>> * Otherwise --> Exit Isearch mode.
>>
>> Please try the following patch.  Does it provide the behavior
>> you are asking for?
>
> It does. Thank you.

I think the behavior of Isearch that Dani proposes is reasonable because
it seems this is how `isearch-abort' was supposed to work since its
comment says "If search has an incomplete regexp" but this branch currently
is never reached.  So I'd like to install it to the trunk.





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

* bug#7534: 24.0.50; G-g within Isearch regexp mode
  2010-12-25  2:47         ` Juri Linkov
@ 2011-01-04 11:42           ` Dani Moncayo
  0 siblings, 0 replies; 13+ messages in thread
From: Dani Moncayo @ 2011-01-04 11:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 7534

On Sat, Dec 25, 2010 at 03:47, Juri Linkov <juri@jurta.org> wrote:
>
> I think the behavior of Isearch that Dani proposes is reasonable because
> it seems this is how `isearch-abort' was supposed to work since its
> comment says "If search has an incomplete regexp" but this branch currently
> is never reached.  So I'd like to install it to the trunk.
>

This is a fairly simple change. Please, could a maintainer take a look
to the proposed patch?

TIA.

-- 
Dani Moncayo





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

* bug#7534: C-g in Isearch regexp mode
  2010-12-02 19:47 bug#7534: 24.0.50; G-g within Isearch regexp mode Dani Moncayo
  2010-12-02 22:31 ` Juri Linkov
@ 2011-01-15  9:57 ` Dani Moncayo
  2011-01-15 15:51   ` Stefan Monnier
  2011-02-04  7:54 ` bug#7534: what about committing the patch also to emacs-23? Dani Moncayo
  2 siblings, 1 reply; 13+ messages in thread
From: Dani Moncayo @ 2011-01-15  9:57 UTC (permalink / raw)
  To: Stefan Monnier, Chong Yidong; +Cc: 7534

>On Sat, Dec 25, 2010 at 03:47, Juri Linkov <juri <at> jurta.org> wrote:
>>
>> I think the behavior of Isearch that Dani proposes is reasonable because
>> it seems this is how `isearch-abort' was supposed to work since its
>> comment says "If search has an incomplete regexp" but this branch currently
>> is never reached.  So I'd like to install it to the trunk.
>>

> This is a fairly simple change. Please, could a maintainer take a look
> to the proposed patch?

Hello? Is anyone out there?   ;-)

-- 
Dani Moncayo





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

* bug#7534: C-g in Isearch regexp mode
  2011-01-15  9:57 ` bug#7534: C-g in " Dani Moncayo
@ 2011-01-15 15:51   ` Stefan Monnier
  2011-01-16  1:06     ` Juri Linkov
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2011-01-15 15:51 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: Chong Yidong, 7534

>>> I think the behavior of Isearch that Dani proposes is reasonable because
>>> it seems this is how `isearch-abort' was supposed to work since its
>>> comment says "If search has an incomplete regexp" but this branch currently
>>> is never reached.  So I'd like to install it to the trunk.

I haven't had time to dig into it and understand the details, but
I'll trust Juri on this one.


        Stefan





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

* bug#7534: C-g in Isearch regexp mode
  2011-01-15 15:51   ` Stefan Monnier
@ 2011-01-16  1:06     ` Juri Linkov
  0 siblings, 0 replies; 13+ messages in thread
From: Juri Linkov @ 2011-01-16  1:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 7534-done, Chong Yidong

>>>> I think the behavior of Isearch that Dani proposes is reasonable because
>>>> it seems this is how `isearch-abort' was supposed to work since its
>>>> comment says "If search has an incomplete regexp" but this branch currently
>>>> is never reached.  So I'd like to install it to the trunk.
>
> I haven't had time to dig into it and understand the details, but
> I'll trust Juri on this one.

Installed to the trunk.





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

* bug#7534: what about committing the patch also to emacs-23?
  2010-12-02 19:47 bug#7534: 24.0.50; G-g within Isearch regexp mode Dani Moncayo
  2010-12-02 22:31 ` Juri Linkov
  2011-01-15  9:57 ` bug#7534: C-g in " Dani Moncayo
@ 2011-02-04  7:54 ` Dani Moncayo
  2011-02-04 14:40   ` Stefan Monnier
  2 siblings, 1 reply; 13+ messages in thread
From: Dani Moncayo @ 2011-02-04  7:54 UTC (permalink / raw)
  To: Chong Yidong, Stefan Monnier, Juri Linkov; +Cc: 7534

Hi,

Given that the patch for this bug was tiny and inoffensive, what do
you think about commiting it also in the emacs-23 branch (before the
23.3 release)?

TIA

--
Dani Moncayo





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

* bug#7534: what about committing the patch also to emacs-23?
  2011-02-04  7:54 ` bug#7534: what about committing the patch also to emacs-23? Dani Moncayo
@ 2011-02-04 14:40   ` Stefan Monnier
  2011-02-04 15:39     ` Dani Moncayo
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2011-02-04 14:40 UTC (permalink / raw)
  To: Dani Moncayo; +Cc: Chong Yidong, 7534

> Given that the patch for this bug was tiny and inoffensive, what do
> you think about commiting it also in the emacs-23 branch (before the
> 23.3 release)?

We expect to be able to ship 23.3 *real soon now*, so we only want to
commit there fixes for regressions now.


        Stefan





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

* bug#7534: what about committing the patch also to emacs-23?
  2011-02-04 14:40   ` Stefan Monnier
@ 2011-02-04 15:39     ` Dani Moncayo
  0 siblings, 0 replies; 13+ messages in thread
From: Dani Moncayo @ 2011-02-04 15:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Chong Yidong, 7534

On Fri, Feb 4, 2011 at 15:40, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> Given that the patch for this bug was tiny and inoffensive, what do
>> you think about commiting it also in the emacs-23 branch (before the
>> 23.3 release)?
>
> We expect to be able to ship 23.3 *real soon now*, so we only want to
> commit there fixes for regressions now.
>

mmm... so I've arrived a bit late. It is a pity, because IMO emacs
23.3 would be a little bit better with that bug fixed, but I
understand you.

Thanks anyway.


-- 
Dani Moncayo





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

end of thread, other threads:[~2011-02-04 15:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-02 19:47 bug#7534: 24.0.50; G-g within Isearch regexp mode Dani Moncayo
2010-12-02 22:31 ` Juri Linkov
2010-12-03  7:57   ` Dani Moncayo
2010-12-05 23:11     ` Juri Linkov
2010-12-06 10:41       ` Dani Moncayo
2010-12-25  2:47         ` Juri Linkov
2011-01-04 11:42           ` Dani Moncayo
2011-01-15  9:57 ` bug#7534: C-g in " Dani Moncayo
2011-01-15 15:51   ` Stefan Monnier
2011-01-16  1:06     ` Juri Linkov
2011-02-04  7:54 ` bug#7534: what about committing the patch also to emacs-23? Dani Moncayo
2011-02-04 14:40   ` Stefan Monnier
2011-02-04 15:39     ` Dani Moncayo

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