bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Drew Adams]

> > I've tested this out on emacs 25 as well, and it still is an issue.
> 
> For what it's worth, it works fine for me with the emacs-25 branch in
> X-Windows (more precisely, XFCE).

It works for me on MS Windows also, using an old Emacs 25 build.
Following Alan's recipe gives the behavior he describes:

> I have `isearch-allow-scroll' set to t, and I search for a word in
> Isearch, which is found.  I then type C-S-right just once, and the next
> word after what had been the highlighted search region gets highlighted
> for me (in grey).  The Isearch operation is now terminated.

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Andrew Hyatt]

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

On Sun, Jun 19, 2016 at 10:36 AM Drew Adams <drew.adams@oracle.com> wrote:

> > > I've tested this out on emacs 25 as well, and it still is an issue.
> >
> > For what it's worth, it works fine for me with the emacs-25 branch in
> > X-Windows (more precisely, XFCE).
>
> It works for me on MS Windows also, using an old Emacs 25 build.
> Following Alan's recipe gives the behavior he describes:


> > I have `isearch-allow-scroll' set to t, and I search for a word in
> > Isearch, which is found.  I then type C-S-right just once, and the next
> > word after what had been the highlighted search region gets highlighted
> > for me (in grey).  The Isearch operation is now terminated.
>

Perhaps I misunderstood the original bug, but I thought the problem was
that when you C-S-right, the text that was highlighted with isearch is no
longer highlighted.

Are you saying that that original selection that you were initially on
before the C-S-right is still highlighted, or are you saying it isn't, and
that's not a bug?

[-- Attachment #2: Type: text/html, Size: 1544 bytes --]

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Sun, Jun 19, 2016 at 8:57 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
> On Sun, Jun 19, 2016 at 10:36 AM Drew Adams <drew.adams@oracle.com> wrote:
>>
>> It works for me on MS Windows also, using an old Emacs 25 build.
>> Following Alan's recipe gives the behavior he describes:
>>
>> > I have `isearch-allow-scroll' set to t, and I search for a word in
>> > Isearch, which is found.  I then type C-S-right just once, and the next
>> > word after what had been the highlighted search region gets highlighted
>> > for me (in grey).  The Isearch operation is now terminated.

Yes, that was my meaning, and it looks like it works fine even in
v24.5.1.


> Perhaps I misunderstood the original bug, but I thought the problem
> was that when you C-S-right, the text that was highlighted with
> isearch is no longer highlighted.
>
> Are you saying that that original selection that you were initially on
> before the C-S-right is still highlighted, or are you saying it isn't,
> and that's not a bug?

My expectation (which is what it's doing now) is that a C-S-right would
terminate isearch and select the next word.

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Andrew Hyatt]

Eli Barzilay <eli@barzilay.org> writes:

> On Sun, Jun 19, 2016 at 8:57 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
>> On Sun, Jun 19, 2016 at 10:36 AM Drew Adams <drew.adams@oracle.com> wrote:
>>>
>>> It works for me on MS Windows also, using an old Emacs 25 build.
>>> Following Alan's recipe gives the behavior he describes:
>>>
>>> > I have `isearch-allow-scroll' set to t, and I search for a word in
>>> > Isearch, which is found.  I then type C-S-right just once, and the next
>>> > word after what had been the highlighted search region gets highlighted
>>> > for me (in grey).  The Isearch operation is now terminated.
>
> Yes, that was my meaning, and it looks like it works fine even in
> v24.5.1.
>
>
>> Perhaps I misunderstood the original bug, but I thought the problem
>> was that when you C-S-right, the text that was highlighted with
>> isearch is no longer highlighted.
>>
>> Are you saying that that original selection that you were initially on
>> before the C-S-right is still highlighted, or are you saying it isn't,
>> and that's not a bug?
>
> My expectation (which is what it's doing now) is that a C-S-right would
> terminate isearch and select the next word.

OK, if that's the case, then it seems to me that this bug report is
either unreproducible, or (if I understand the report correctly) not a
bug in the first place.  I'll close as not a bug for now.

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Wed, Jun 22, 2016 at 11:23 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
>
> OK, if that's the case, then it seems to me that this bug report is
> either unreproducible, or (if I understand the report correctly) not a
> bug in the first place.

Um, I reported it for 23.1, seven years ago.  Back this it definitely
*was* a bug (read the emails: we discussed a hack I had around it, and
other ways to solve it).  Your conclusion that it's "not a bug a bug in
the first place" is therefore very strange.

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Andrew Hyatt]

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

On Thu, Jun 23, 2016 at 12:18 AM Eli Barzilay <eli@barzilay.org> wrote:

> On Wed, Jun 22, 2016 at 11:23 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
> >
> > OK, if that's the case, then it seems to me that this bug report is
> > either unreproducible, or (if I understand the report correctly) not a
> > bug in the first place.
>
> Um, I reported it for 23.1, seven years ago.  Back this it definitely
> *was* a bug (read the emails: we discussed a hack I had around it, and
> other ways to solve it).  Your conclusion that it's "not a bug a bug in
> the first place" is therefore very strange.
>

Perhaps I've misunderstood your original bug report or reply.  To be as
clear as possible, here's what I think is true, so tell me what part I'm
misunderstanding:

The original bug report was that if you do an isearch, then C-S-right, the
word that was selected as part of isearch is no longer selected, and the
selection starts out to the right of the word.  So, if you searched for
"foo" and the buffer text was "foo bar", and isearch is selecting "foo",
then C-S-right will leave you with only "bar" selected, instead of "foo
bar" selected.

This is something I can reproduce right now on Emacs 25.

You said you could no longer reproduce this, and that your expectation is
that "C-s-right would terminate isearch and select the next word".  I
understood this to mean that C-s-right would, in our example, only select
"bar", and that's the expected behavior.  Is it?


>
> --
>                    ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
>                    http://barzilay.org/                  Maze is Life!
>

[-- Attachment #2: Type: text/html, Size: 2411 bytes --]

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Thu, Jun 23, 2016 at 2:15 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
> On Thu, Jun 23, 2016 at 12:18 AM Eli Barzilay <eli@barzilay.org> wrote:
>>
>> On Wed, Jun 22, 2016 at 11:23 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
>> >
>> > OK, if that's the case, then it seems to me that this bug report is
>> > either unreproducible, or (if I understand the report correctly) not a
>> > bug in the first place.
>>
>> Um, I reported it for 23.1, seven years ago.  Back this it definitely
>> *was* a bug (read the emails: we discussed a hack I had around it, and
>> other ways to solve it).  Your conclusion that it's "not a bug a bug in
>> the first place" is therefore very strange.
>
>
> Perhaps I've misunderstood your original bug report or reply.  To be
> as clear as possible, here's what I think is true, so tell me what
> part I'm misunderstanding:
>
> The original bug report was that if you do an isearch, then C-S-right,
> the word that was selected as part of isearch is no longer selected,
> and the selection starts out to the right of the word.  So, if you
> searched for "foo" and the buffer text was "foo bar", and isearch is
> selecting "foo", then C-S-right will leave you with only "bar"
> selected, instead of "foo bar" selected.

No: what you're describing was the behavior I *expected*.  The bug was
that the key that was used to exit isearch -- C-S-right in my reported
case -- would "lose" the shift bit, making it be treated as just
C-right, and leaving the buffer with the cursor after the "foo bar" and
with no selection.  If you look at the past emails for this bug, you'll
see that the reason for that was discussed, including a solution that
was most likely implemented.


> This is something I can reproduce right now on Emacs 25.

... And the result of the above is that it is indeed working as you
describe on v25 which means that it was probably fine for a while now
but the bug was just not closed.


> You said you could no longer reproduce this, and that your expectation
> is that "C-s-right would terminate isearch and select the next word".
> I understood this to mean that C-s-right would, in our example, only
> select "bar", and that's the expected behavior.  Is it?

To summarize it: all is fine and this bug should be closed -- I just
objected to your reasoning that it was "not a bug in the first place".
It *was* a bug, and it got resolved -- a long time ago.

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Andrew Hyatt]

Eli Barzilay <eli@barzilay.org> writes:

> On Thu, Jun 23, 2016 at 2:15 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
>> On Thu, Jun 23, 2016 at 12:18 AM Eli Barzilay <eli@barzilay.org> wrote:
>>>
>>> On Wed, Jun 22, 2016 at 11:23 PM, Andrew Hyatt <ahyatt@gmail.com> wrote:
>>> >
>>> > OK, if that's the case, then it seems to me that this bug report is
>>> > either unreproducible, or (if I understand the report correctly) not a
>>> > bug in the first place.
>>>
>>> Um, I reported it for 23.1, seven years ago.  Back this it definitely
>>> *was* a bug (read the emails: we discussed a hack I had around it, and
>>> other ways to solve it).  Your conclusion that it's "not a bug a bug in
>>> the first place" is therefore very strange.
>>
>>
>> Perhaps I've misunderstood your original bug report or reply.  To be
>> as clear as possible, here's what I think is true, so tell me what
>> part I'm misunderstanding:
>>
>> The original bug report was that if you do an isearch, then C-S-right,
>> the word that was selected as part of isearch is no longer selected,
>> and the selection starts out to the right of the word.  So, if you
>> searched for "foo" and the buffer text was "foo bar", and isearch is
>> selecting "foo", then C-S-right will leave you with only "bar"
>> selected, instead of "foo bar" selected.
>
> No: what you're describing was the behavior I *expected*.  The bug was
> that the key that was used to exit isearch -- C-S-right in my reported
> case -- would "lose" the shift bit, making it be treated as just
> C-right, and leaving the buffer with the cursor after the "foo bar" and
> with no selection.  If you look at the past emails for this bug, you'll
> see that the reason for that was discussed, including a solution that
> was most likely implemented.
>
>
>> This is something I can reproduce right now on Emacs 25.
>
> ... And the result of the above is that it is indeed working as you
> describe on v25 which means that it was probably fine for a while now
> but the bug was just not closed.
>
>
>> You said you could no longer reproduce this, and that your expectation
>> is that "C-s-right would terminate isearch and select the next word".
>> I understood this to mean that C-s-right would, in our example, only
>> select "bar", and that's the expected behavior.  Is it?
>
> To summarize it: all is fine and this bug should be closed -- I just
> objected to your reasoning that it was "not a bug in the first place".
> It *was* a bug, and it got resolved -- a long time ago.

Got it, thanks for the explanation.  I'll make sure this is closed.

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:


  When `isearch-allow-scroll' is turned on, then exiting isearch with
  a movement command loses its "shift" status.  To see this, use
  isearch and look for some string, then hit C-S-right a few times and
  you will see that the first one does not starts a selection.


In GNU Emacs 23.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.14)
 of 2009-08-01 on winooski.ccs.neu.edu
Windowing system distributor `The X.Org Foundation', version 11.0.10300000
configured using `configure  '--prefix=/home/eli/bin/local/emacs-dir''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: POSIX
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default-enable-multibyte-characters: t

Major mode: Text

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e p o <tab> r <tab> <return>

Recent messages:
Loading ~/MyEmacs/pre...done
Loading ~/EliEmacs/eliemacs...done
Loading ~/MyEmacs/post...done
Making completion list...

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

>   When `isearch-allow-scroll' is turned on, then exiting isearch with
>   a movement command loses its "shift" status.  To see this, use
>   isearch and look for some string, then hit C-S-right a few times and
>   you will see that the first one does not starts a selection.

When `isearch-allow-scroll' is non-nil, then `isearch-other-meta-char'
calls `isearch-reread-key-sequence-naturally', but `read-key-sequence'
in the latter function removes the shift modifier and sets
`this-command-keys-shift-translated' to t.

This is just an analysis.  I currently don't know what is the right way
to fix this.  Maybe simply add the shift modifier back to the key when
`this-command-keys-shift-translated' to t after `read-key-sequence'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Aug 12, Juri Linkov wrote:
> >   When `isearch-allow-scroll' is turned on, then exiting isearch with
> >   a movement command loses its "shift" status.  To see this, use
> >   isearch and look for some string, then hit C-S-right a few times and
> >   you will see that the first one does not starts a selection.
> 
> When `isearch-allow-scroll' is non-nil, then
> `isearch-other-meta-char' calls
> `isearch-reread-key-sequence-naturally', but `read-key-sequence' in
> the latter function removes the shift modifier and sets
> `this-command-keys-shift-translated' to t.

Yes, I hacked around it with my own version of
`isearch-other-meta-char'.  (But I don't think that it's a good
hack...)


> This is just an analysis.  I currently don't know what is the right
> way to fix this.  Maybe simply add the shift modifier back to the
> key when `this-command-keys-shift-translated' to t after
> `read-key-sequence'.

This sounds like a good strategy, given that shift is now very useful
in general.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

>> This is just an analysis.  I currently don't know what is the right
>> way to fix this.  Maybe simply add the shift modifier back to the
>> key when `this-command-keys-shift-translated' to t after
>> `read-key-sequence'.
>
> This sounds like a good strategy, given that shift is now very useful
> in general.

It's interesting that after loading s-region.el, your reported case works
correctly without fixes.

So it seems the current core shift-selection is not the exact
re-implementation of s-region.el.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Aug 16, Juri Linkov wrote:
> >> This is just an analysis.  I currently don't know what is the
> >> right way to fix this.  Maybe simply add the shift modifier back
> >> to the key when `this-command-keys-shift-translated' to t after
> >> `read-key-sequence'.
> >
> > This sounds like a good strategy, given that shift is now very
> > useful in general.
> 
> It's interesting that after loading s-region.el, your reported case
> works correctly without fixes.
> 
> So it seems the current core shift-selection is not the exact
> re-implementation of s-region.el.

I still believe that the problem is somewhere in how
`isearch-other-meta-char' restores the key.  FWIW, here is my hack; I
can't even explain how it works since I was basically guessing my way
trying to get the stupid thing to work.

At least in v22, this would cause the key that is used to exit isearch
to be replayed twice -- for example, using `C-x 2' would happen twice.
It just happens that I'm personally much more likely to shift-arrow my
way out of isearch than I am to splitting the window.

                (progn
                  ;; original code:
                  ;; (setq key (isearch-reread-key-sequence-naturally keylist))
                  ;; (setq keylist (listify-key-sequence key))
                  ;; ELI: use original prefix, so S-up keeps the S
                  (let* ((key1 (isearch-reread-key-sequence-naturally keylist))
                         (keylist1 (listify-key-sequence key1)))
                    (setq keylist (listify-key-sequence key)) ; original key
                    (setq keylist (append keylist (nthcdr (length keylist)
                                                          keylist1)))
                    (setq key key1)
                    (setq main-event (aref key 0))
                    (setq scroll-command (isearch-lookup-scroll-key key))))


-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

>> It's interesting that after loading s-region.el, your reported case
>> works correctly without fixes.
>>
>> So it seems the current core shift-selection is not the exact
>> re-implementation of s-region.el.
>
> I still believe that the problem is somewhere in how
> `isearch-other-meta-char' restores the key.  FWIW, here is my hack; I
> can't even explain how it works since I was basically guessing my way
> trying to get the stupid thing to work.
>
> At least in v22, this would cause the key that is used to exit isearch
> to be replayed twice -- for example, using `C-x 2' would happen twice.
> It just happens that I'm personally much more likely to shift-arrow my
> way out of isearch than I am to splitting the window.

Could you try the following hack:

Index: lisp/isearch.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.345
diff -u -r1.345 isearch.el
--- lisp/isearch.el	14 Feb 2009 09:04:46 -0000	1.345
+++ lisp/isearch.el	17 Aug 2009 00:47:38 -0000
@@ -1900,6 +1905,12 @@
           ((and isearch-allow-scroll
                 (progn (setq key (isearch-reread-key-sequence-naturally keylist))
                        (setq keylist (listify-key-sequence key))
+                       (when this-command-keys-shift-translated
+                         (setq keylist (list
+                                        (event-convert-list
+                                         (append (cons 'shift (event-modifiers keylist))
+                                                 (list (event-basic-type keylist))))))
+                         (setq this-command-keys-shift-translated nil))
                        (setq main-event (aref key 0))
                        (setq scroll-command (isearch-lookup-scroll-key key))))
            ;; From this point onwards, KEY, KEYLIST and MAIN-EVENT hold a

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Aug 17, Juri Linkov wrote:
> >> It's interesting that after loading s-region.el, your reported case
> >> works correctly without fixes.
> >>
> >> So it seems the current core shift-selection is not the exact
> >> re-implementation of s-region.el.
> >
> > I still believe that the problem is somewhere in how
> > `isearch-other-meta-char' restores the key.  FWIW, here is my hack; I
> > can't even explain how it works since I was basically guessing my way
> > trying to get the stupid thing to work.
> >
> > At least in v22, this would cause the key that is used to exit isearch
> > to be replayed twice -- for example, using `C-x 2' would happen twice.
> > It just happens that I'm personally much more likely to shift-arrow my
> > way out of isearch than I am to splitting the window.
> 
> Could you try the following hack:
> [...]

Yes, this works.  (But not on v22, of course.)

(And re what I said above, the duplication that I've had on `C-x 2' is
unrelated.)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Stefan Monnier]

> Index: lisp/isearch.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/isearch.el,v
> retrieving revision 1.345
> diff -u -r1.345 isearch.el
> --- lisp/isearch.el	14 Feb 2009 09:04:46 -0000	1.345
> +++ lisp/isearch.el	17 Aug 2009 00:47:38 -0000
> @@ -1900,6 +1905,12 @@
>            ((and isearch-allow-scroll
>                  (progn (setq key (isearch-reread-key-sequence-naturally keylist))
>                         (setq keylist (listify-key-sequence key))
> +                       (when this-command-keys-shift-translated
> +                         (setq keylist (list
> +                                        (event-convert-list
> +                                         (append (cons 'shift (event-modifiers keylist))
> +                                                 (list (event-basic-type keylist))))))
> +                         (setq this-command-keys-shift-translated nil))
>                         (setq main-event (aref key 0))
>                         (setq scroll-command (isearch-lookup-scroll-key key))))
>             ;; From this point onwards, KEY, KEYLIST and MAIN-EVENT hold a

If this works, it's good, but it shouldn't be installed as is: this has
no business being in isearch.el since other packages may need to do
the same.  Please try to abstract some useful function that we can put
in subr.el and then use here in isearch.el.


        Stefan

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Eli Barzilay]

On Aug 17, Stefan Monnier wrote:
> [...]
> If this works, it's good, but it shouldn't be installed as is: this
> has no business being in isearch.el since other packages may need to
> do the same.  Please try to abstract some useful function that we
> can put in subr.el and then use here in isearch.el.

Yes -- here's another case that suffers from the same problem:
`comint-dynamic-list-completions' reads one key (because it wants to
remove the completions window when space is used) -- so it reads one
key to test if it's space and uses `unread-command-events' otherwise.
So: run `shell', type one character and hit tab (needs to have several
files that start with it to pop the completions), then do some
shift-movements.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

> Yes -- here's another case that suffers from the same problem:
> `comint-dynamic-list-completions' reads one key (because it wants to
> remove the completions window when space is used) -- so it reads one
> key to test if it's space and uses `unread-command-events' otherwise.
> So: run `shell', type one character and hit tab (needs to have several
> files that start with it to pop the completions), then do some
> shift-movements.

This can be fixed using the argument `dont-downcase-last' of
`read-key-sequence':

Index: lisp/comint.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/comint.el,v
retrieving revision 1.390
diff -u -r1.390 comint.el
--- lisp/comint.el	23 Jun 2009 07:25:15 -0000	1.390
+++ lisp/comint.el	17 Aug 2009 21:19:29 -0000
@@ -3014,7 +3014,7 @@
       (if (with-current-buffer (get-buffer "*Completions*")
 	    (set (make-local-variable 'comint-displayed-dynamic-completions)
 		 completions)
-	    (setq key (read-key-sequence nil)
+	    (setq key (read-key-sequence nil nil t)
 		  first (aref key 0))
 	    (and (consp first) (consp (event-start first))
 		 (eq (window-buffer (posn-window (event-start first)))

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

> If this works, it's good, but it shouldn't be installed as is: this has
> no business being in isearch.el since other packages may need to do
> the same.  Please try to abstract some useful function that we can put
> in subr.el and then use here in isearch.el.

Actually the solution is much simpler.  I just noticed that
`read-key-sequence' already has the necessary argument
`dont-downcase-last':

  The third (optional) arg dont-downcase-last, if non-nil, means do not
  convert the last event to lower case.  (Normally any upper case event
  is converted to lower case if the original event is undefined and the lower
  case equivalent is defined.)  A non-nil value is appropriate for reading
  a key sequence to be defined.

So the fix is simple:

Index: lisp/isearch.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.345
diff -u -r1.345 isearch.el
--- lisp/isearch.el	14 Feb 2009 09:04:46 -0000	1.345
+++ lisp/isearch.el	17 Aug 2009 21:17:57 -0000
@@ -1805,7 +1810,7 @@
 Return the key sequence as a string/vector."
   (isearch-unread-key-sequence keylist)
   (let (overriding-terminal-local-map)
-    (read-key-sequence nil)))  ; This will go through function-key-map, if nec.
+    (read-key-sequence nil nil t)))  ; This will go through function-key-map, if nec.
 
 (defun isearch-lookup-scroll-key (key-seq)
   "If KEY-SEQ is bound to a scrolling command, return it as a symbol.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Stefan Monnier]

> Actually the solution is much simpler.  I just noticed that
> `read-key-sequence' already has the necessary argument
> `dont-downcase-last':

>   The third (optional) arg dont-downcase-last, if non-nil, means do not
>   convert the last event to lower case.  (Normally any upper case event
>   is converted to lower case if the original event is undefined and the lower
>   case equivalent is defined.)  A non-nil value is appropriate for reading
>   a key sequence to be defined.

Yes, I guess that's better.  But at the same time, it still seems
incorrect: the sequence we get has gone through input-decode-map,
function-key-map, key-translation-map, keyboard-translate-table,
down-mouse and drag-mouse event may have been dropped, etc...

So stuffing it back into unread-command-events is incorrect.  I think
the right solution requires the use of something like
this-single-command-raw-keys.


        Stefan

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Juri Linkov]

> So stuffing it back into unread-command-events is incorrect.  I think
> the right solution requires the use of something like
> this-single-command-raw-keys.

How this would help to reread a key sequence with Isearch
keymap deactivated (i.e. to do the same that
`isearch-reread-key-sequence-naturally' currently does)?

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Stefan Monnier]

>> So stuffing it back into unread-command-events is incorrect.  I think
>> the right solution requires the use of something like
>> this-single-command-raw-keys.

> How this would help to reread a key sequence with Isearch
> keymap deactivated (i.e. to do the same that
> `isearch-reread-key-sequence-naturally' currently does)?

I think it would make the funny business like

  (if (and (> (length keylist) 1)
           (symbolp (car keylist))
           (listp (cadr keylist))
           (not (numberp (posn-point
                          (event-start (cadr keylist)  )))))
      (pop unread-command-events)))

unnecessary, and it would fix various bugs such as the one discussed.

`unread-command-events' should hold events that have not gone through
input-decode-map, function-key-map, dwon-mouse and drag-mouse demotion,
double-click -> click demotion, shift->unshift demotion, ...
i.e. raw-events.
So isearch-unread should be called with events that come from read-event
or from this-single-command-raw-keys but not from read-key-sequence.


        Stefan

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Andrew Hyatt]

I've tested this out on emacs 25 as well, and it still is an issue.

Eli Barzilay <eli@barzilay.org> writes:

> Please describe exactly what actions triggered the bug
> and the precise symptoms of the bug:
>
>
>   When `isearch-allow-scroll' is turned on, then exiting isearch with
>   a movement command loses its "shift" status.  To see this, use
>   isearch and look for some string, then hit C-S-right a few times and
>   you will see that the first one does not starts a selection.
>
>
> In GNU Emacs 23.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.14)
>  of 2009-08-01 on winooski.ccs.neu.edu
> Windowing system distributor `The X.Org Foundation', version 11.0.10300000
> configured using `configure  '--prefix=/home/eli/bin/local/emacs-dir''
>
> Important settings:
>   value of $LC_ALL: nil
>   value of $LC_COLLATE: POSIX
>   value of $LC_CTYPE: nil
>   value of $LC_MESSAGES: nil
>   value of $LC_MONETARY: nil
>   value of $LC_NUMERIC: nil
>   value of $LC_TIME: nil
>   value of $LANG: en_US
>   value of $XMODIFIERS: nil
>   locale-coding-system: iso-latin-1-unix
>   default-enable-multibyte-characters: t
>
> Major mode: Text
>
> Minor modes in effect:
>   tooltip-mode: t
>   mouse-wheel-mode: t
>   file-name-shadow-mode: t
>   global-font-lock-mode: t
>   font-lock-mode: t
>   global-auto-composition-mode: t
>   auto-composition-mode: t
>   auto-encryption-mode: t
>   auto-compression-mode: t
>   line-number-mode: t
>   transient-mark-mode: t
>
> Recent input:
> M-x r e p o <tab> r <tab> <return>
>
> Recent messages:
> Loading ~/MyEmacs/pre...done
> Loading ~/EliEmacs/eliemacs...done
> Loading ~/MyEmacs/post...done
> Making completion list...

bug#4117: 23.1; isearch + isearch-allow-scroll loses shift

[Alan Mackenzie]

Hello, Andrew.

In article <mailman.1719.1466222049.1216.bug-gnu-emacs@gnu.org> you wrote:

> I've tested this out on emacs 25 as well, and it still is an issue.

For what it's worth, it works fine for me with the emacs-25 branch in
X-Windows (more precisely, XFCE).  (Shift selection doesn't work at all
on my Linux virtual tty.)

I have `isearch-allow-scroll' set to t, and I search for a word in
Isearch, which is found.  I then type C-S-right just once, and the next
word after what had been the highlighted search region gets highlighted
for me (in grey).  The Isearch operation is now terminated.

> Eli Barzilay <eli@barzilay.org> writes:

>> Please describe exactly what actions triggered the bug
>> and the precise symptoms of the bug:
>>
>>
>>   When `isearch-allow-scroll' is turned on, then exiting isearch with
>>   a movement command loses its "shift" status.  To see this, use
>>   isearch and look for some string, then hit C-S-right a few times and
>>   you will see that the first one does not starts a selection.
>>
>>
>> In GNU Emacs 23.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.14)
>>  of 2009-08-01 on winooski.ccs.neu.edu
>> Windowing system distributor `The X.Org Foundation', version 11.0.10300000
>> configured using `configure  '--prefix=/home/eli/bin/local/emacs-dir''

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).