bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

I've installed the expand-region.el package and noticed that transient
mark mode started misbehaving.  I'm using Emacs with
`transient-mark-mode' disabled, and enable it occasionally via `C-SPC
C-SPC' when needed.  After using `er/expand-region' function I've
noticed that `C-SPC C-SPC' doesn't highlight the region.  Upon further
inspection I've noticed that the `transient-mark-mode' variable is set
to `(only only only only only ... only) with a total of 26 `only'
symbols.  Symbols disappear one by one when I move the point or invoke
other commands, so eventually `C-SPC C-SPC' starts working again.

I've added a watcher to the `transient-mark-mode' variable and upon
first activation of expand region I see this:

    transient-mark-mode (only) set #<buffer *scratch*>
    transient-mark-mode (only only) set #<buffer *scratch*>
    transient-mark-mode (only only only) set #<buffer *scratch*>
    transient-mark-mode (only only only only) set #<buffer *scratch*>
    Type = to expand again, - to contract, 0 to reset

Upon further investigation I've found that this happens inside the
`exchange-point-and-mark' function.  I've added a message right before
`cons' call to confirm that:

    transient-mark-mode (only) set #<buffer *scratch*>
    exchange-point-and-mark
    transient-mark-mode (only only) set #<buffer *scratch*>
    exchange-point-and-mark
    transient-mark-mode (only only only) set #<buffer *scratch*>
    exchange-point-and-mark
    transient-mark-mode (only only only only) set #<buffer *scratch*>
    Type = to expand again, - to contract, 0 to reset

The function explicitly checks if ``(eq (car-safe transient-mark-mode)
'only)'' is true and then conses `only' to the `transient-mark-mode',
causing it to grow.  I'm not sure what is the intention here, but this
makes it harder using `C-SPC C-SPC` to temporarily activate the
transient mark mode.

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.30, cairo version 1.17.4)
 of 2021-12-29 built on toolbox
Repository revision: 3fc8032a1aac535e8f6ba03547bd9fa2b0880193
Repository branch: master
System Description: Fedora Linux 35 (Workstation Edition)

Configured using:
 'configure --with-native-compilation --with-pgtk
 --prefix=/home/alist/.local'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XIM
GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug sendmail comp comp-cstr cl-extra
help-mode pcase org-macs cap-words superword subword ruby-mode smie rx
python tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat shell pcomplete parse-time iso8601 ls-lisp format-spec
rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt
rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap nxml-util
nxml-enc xmltok html-mode-expansions sgml-mode facemenu dom cc-cmds
cc-engine cc-vars cc-defs er-basic-expansions thingatpt
expand-region-core advice expand-region-custom warnings cl compile
comint ansi-color ring autoload radix-tree lisp-mnt tar-mode arc-mode
archive-mode mm-archive message yank-media dired dired-loaddefs rfc822
mml mml-sec epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date mailabbrev gmm-utils mailheader mm-decode
mm-bodies mm-encode mail-utils gnutls network-stream url-http mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr url-gw nsm rmc
puny url-cache url-auth cus-edit pp cus-start cus-load wid-edit
finder-inf package browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util mailcap
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map url-vars seq gv subr-x byte-opt
bytecomp byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc
paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode
mwheel term/pgtk-win pgtk-win term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
button loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit pgtk lcms2 multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 193794 12810)
 (symbols 48 17181 1)
 (strings 32 60612 2225)
 (string-bytes 1 2133315)
 (vectors 16 30994)
 (vector-slots 8 449511 20505)
 (floats 8 76 163)
 (intervals 56 595 0)
 (buffers 992 15))

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> From: Andrey Listopadov <andreyorst@gmail.com>
> Date: Thu, 30 Dec 2021 12:53:08 +0300
> 
> I've installed the expand-region.el package and noticed that transient
> mark mode started misbehaving.  I'm using Emacs with
> `transient-mark-mode' disabled, and enable it occasionally via `C-SPC
> C-SPC' when needed.  After using `er/expand-region' function I've
> noticed that `C-SPC C-SPC' doesn't highlight the region.  Upon further
> inspection I've noticed that the `transient-mark-mode' variable is set
> to `(only only only only only ... only) with a total of 26 `only'
> symbols.  Symbols disappear one by one when I move the point or invoke
> other commands, so eventually `C-SPC C-SPC' starts working again.
> 
> I've added a watcher to the `transient-mark-mode' variable and upon
> first activation of expand region I see this:
> 
>     transient-mark-mode (only) set #<buffer *scratch*>
>     transient-mark-mode (only only) set #<buffer *scratch*>
>     transient-mark-mode (only only only) set #<buffer *scratch*>
>     transient-mark-mode (only only only only) set #<buffer *scratch*>
>     Type = to expand again, - to contract, 0 to reset
> 
> Upon further investigation I've found that this happens inside the
> `exchange-point-and-mark' function.  I've added a message right before
> `cons' call to confirm that:
> 
>     transient-mark-mode (only) set #<buffer *scratch*>
>     exchange-point-and-mark
>     transient-mark-mode (only only) set #<buffer *scratch*>
>     exchange-point-and-mark
>     transient-mark-mode (only only only) set #<buffer *scratch*>
>     exchange-point-and-mark
>     transient-mark-mode (only only only only) set #<buffer *scratch*>
>     Type = to expand again, - to contract, 0 to reset
> 
> The function explicitly checks if ``(eq (car-safe transient-mark-mode)
> 'only)'' is true and then conses `only' to the `transient-mark-mode',
> causing it to grow.  I'm not sure what is the intention here, but this
> makes it harder using `C-SPC C-SPC` to temporarily activate the
> transient mark mode.

How come transient-mark-mode became set to 'only' in the first place?
That is supposed to happen only when shift-translated commands are
used, and should be reset to the old value whenever the first
non-shift-translated command is used.  See the doc string of
transient-mark-mode (the variable).

You can see how it works if, in "emacs -Q", you press and hold Shift
and use arrow keys to move point, then evaluate

  M-: transient-mark-mode RET

You should then see the value '(only . t)' of the variable.  Once you
type some unshifted command, like C-f, the value becomes t again.

So I suspect that the expand-region.el package you installed does
something to cause this, and in particular doesn't take care to reset
the value when it should.

Bottom line: you should look in expand-region.el for the reasons of
this misbehavior, and perhaps report a bug to its developers.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

> How come transient-mark-mode became set to 'only' in the first place?
> That is supposed to happen only when shift-translated commands are
> used, and should be reset to the old value whenever the first
> non-shift-translated command is used.  See the doc string of
> transient-mark-mode (the variable).

The expand-region package sets transient mark mode to `only' when tmm
is disabled:

https://github.com/magnars/expand-region.el/blob/4b8322774d9c1d8b64a0049d1dbbc1e7ce80c1a0/expand-region-core.el#L57
https://github.com/magnars/expand-region.el/blob/4b8322774d9c1d8b64a0049d1dbbc1e7ce80c1a0/expand-region-core.el#L163

> You can see how it works if, in "emacs -Q", you press and hold Shift
> and use arrow keys to move point, then evaluate
>
>   M-: transient-mark-mode RET
>
> You should then see the value '(only . t)' of the variable.  Once you
> type some unshifted command, like C-f, the value becomes t again.

Yes, I know, but in the case when transient mark mode is disabled, its
value is not `(only . t)', but `(only . nil)' which is just `(only)'.
And since expand-region conses `only' initially, the
`exchange-point-and-mark' function then rapidly conses a lot of
symbols to it as well, since expand region uses it.

> So I suspect that the expand-region.el package you installed does
> something to cause this, and in particular doesn't take care to reset
> the value when it should.

I don't think it should reset it, e.g. it behaves well, and sets it to
allow shift-select to work:

https://github.com/magnars/expand-region.el/pull/225

> Bottom line: you should look in expand-region.el for the reasons of
> this misbehavior, and perhaps report a bug to its developers.

Unfortunately, Magnar Sveen disabled the issue-reporting feature of
github for this particular project.
However, looking at the docstring, I guess what expand-region does is
correct, and the `exchange-point-and-mark' function does kind of a
weird check - e.g. it checks if the car of `transient-mark-mode' is
`only' and then conses another only to that.
But again, I'm not sure what's the intention here, maybe it is expected.

-- 
Andrey Listopadov

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> From: Andrey Listopadov <andreyorst@gmail.com>
> Date: Thu, 30 Dec 2021 22:07:26 +0300
> Cc: 52896@debbugs.gnu.org
> 
> > How come transient-mark-mode became set to 'only' in the first place?
> > That is supposed to happen only when shift-translated commands are
> > used, and should be reset to the old value whenever the first
> > non-shift-translated command is used.  See the doc string of
> > transient-mark-mode (the variable).
> 
> The expand-region package sets transient mark mode to `only' when tmm
> is disabled:
> 
> https://github.com/magnars/expand-region.el/blob/4b8322774d9c1d8b64a0049d1dbbc1e7ce80c1a0/expand-region-core.el#L57
> https://github.com/magnars/expand-region.el/blob/4b8322774d9c1d8b64a0049d1dbbc1e7ce80c1a0/expand-region-core.el#L163
> 
> > You can see how it works if, in "emacs -Q", you press and hold Shift
> > and use arrow keys to move point, then evaluate
> >
> >   M-: transient-mark-mode RET
> >
> > You should then see the value '(only . t)' of the variable.  Once you
> > type some unshifted command, like C-f, the value becomes t again.
> 
> Yes, I know, but in the case when transient mark mode is disabled, its
> value is not `(only . t)', but `(only . nil)' which is just `(only)'.

That's true, but in "emacs -Q", once I use a non-shifted command, the
value becomes just nil, as intended.

> > Bottom line: you should look in expand-region.el for the reasons of
> > this misbehavior, and perhaps report a bug to its developers.
> 
> Unfortunately, Magnar Sveen disabled the issue-reporting feature of
> github for this particular project.
> However, looking at the docstring, I guess what expand-region does is
> correct, and the `exchange-point-and-mark' function does kind of a
> weird check - e.g. it checks if the car of `transient-mark-mode' is
> `only' and then conses another only to that.
> But again, I'm not sure what's the intention here, maybe it is expected.

It sounds like expand-region.el assumes something about an internal
implementation detail of this variable.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

> That's true, but in "emacs -Q", once I use a non-shifted command, the
> value becomes just nil, as intended.

Here's how to reproduce this without expand-region in `emacs -Q'

1. `M-x transient-mark-mode RET' - disabling tmm;
2. Shift-right on some word in the *scratch* buffer - initial `only'
is consed to `transient-mark-mode';
3. `C-x C-x' `C-x C-x' `C-x C-x' to invoke the
`exchange-point-and-mark' function several times;
4. `C-h v transient-mark-mode RET' - the value is `(only only only only only)'.

So I guess this is the `exchange-point-and-mark' issue.

-- 
Andrey Listopadov

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> From: Andrey Listopadov <andreyorst@gmail.com>
> Date: Thu, 30 Dec 2021 23:02:13 +0300
> Cc: 52896@debbugs.gnu.org
> 
> > That's true, but in "emacs -Q", once I use a non-shifted command, the
> > value becomes just nil, as intended.
> 
> Here's how to reproduce this without expand-region in `emacs -Q'
> 
> 1. `M-x transient-mark-mode RET' - disabling tmm;
> 2. Shift-right on some word in the *scratch* buffer - initial `only'
> is consed to `transient-mark-mode';
> 3. `C-x C-x' `C-x C-x' `C-x C-x' to invoke the
> `exchange-point-and-mark' function several times;
> 4. `C-h v transient-mark-mode RET' - the value is `(only only only only only)'.

If you then perform enough non-shifted commands, the value goes back
to nil.  So I'm not sure I see the problem.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

> If you then perform enough non-shifted commands, the value goes back
> to nil.  So I'm not sure I see the problem.

No, it doesn't.  Instead, it removes a single `once' from the list per
executed command.  So if you have a list of `(once once once once)',
pressing `C-a' will turn it to `(once once once)'.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

sorry, misinterpreted.

Yes, if I perform *enough* commands, it goes back to `nil'.  But the
problem is that I can easily make this list a giant one, the way I've
described in the previous message - repeated execution of exchange
point and mark clearly misbehaves here

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

Perhaps the intention was that the execution of
`exchange-point-and-mark' would remove `once' from the list, so the
function should add it back thus ensuring that the highlighting added
by the shift select would not go away.  However, this instead results
in an uncontrollably growing list, which then interferes with `C-SPC
C-SPC'.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

So, to be clear, the issue is that `C-SPC C-SPC' doesn't enable region
highlighting, preventing from running functions that operate on the
active region.
Full reproduction steps for `emacs -Q' once again:

1. M-x transient-mark-mode RET - turn tmm off;
2. S-right - select the default text in the *scratch* buffer;
3. `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x` -
this would cons some `only' symbols to the list;
4. `C-g' - remove the region highlighting, this removes one `only'
from the list;
5. `C-SPC C-SPC' - displays a message "mark set" and then "mark
activated",  but no highlighting happens, because of remaining `only'
in the list.

Only after *enough* `C-SPC C-SPC' or other commands, the region
highlighting begins to work again.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> From: Andrey Listopadov <andreyorst@gmail.com>
> Date: Thu, 30 Dec 2021 23:40:25 +0300
> Cc: 52896@debbugs.gnu.org
> 
> 1. M-x transient-mark-mode RET - turn tmm off;
> 2. S-right - select the default text in the *scratch* buffer;
> 3. `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x`, `C-x C-x` -
> this would cons some `only' symbols to the list;
> 4. `C-g' - remove the region highlighting, this removes one `only'
> from the list;
> 5. `C-SPC C-SPC' - displays a message "mark set" and then "mark
> activated",  but no highlighting happens, because of remaining `only'
> in the list.

Thanks.  Does the patch below give good results, i.e. does it fix this
problem without introducing any others?

diff --git a/lisp/simple.el b/lisp/simple.el
index 070d276..31463a7 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -6664,7 +6664,7 @@ exchange-point-and-mark
         (user-error "No mark set in this buffer"))
     (set-mark (point))
     (goto-char omark)
-    (cond (temp-highlight
+    (cond ((not temp-highlight)
 	   (setq-local transient-mark-mode (cons 'only transient-mark-mode)))
 	  ((xor arg (not (region-active-p)))
 	   (deactivate-mark))

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> Date: Mon, 03 Jan 2022 16:21:52 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 52896@debbugs.gnu.org
> 
> Thanks.  Does the patch below give good results, i.e. does it fix this
> problem without introducing any others?

Sorry, please use the patch below instead:

diff --git a/lisp/simple.el b/lisp/simple.el
index 070d276..cbcde9f 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -6664,7 +6664,7 @@ exchange-point-and-mark
         (user-error "No mark set in this buffer"))
     (set-mark (point))
     (goto-char omark)
-    (cond (temp-highlight
+    (cond ((and (not arg) (not temp-highlight))
 	   (setq-local transient-mark-mode (cons 'only transient-mark-mode)))
 	  ((xor arg (not (region-active-p)))
 	   (deactivate-mark))

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

> Sorry, please use the patch below instead:
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 070d276..cbcde9f 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -6664,7 +6664,7 @@ exchange-point-and-mark
>          (user-error "No mark set in this buffer"))
>      (set-mark (point))
>      (goto-char omark)
> -    (cond (temp-highlight
> +    (cond ((and (not arg) (not temp-highlight))
>            (setq-local transient-mark-mode (cons 'only transient-mark-mode)))
>           ((xor arg (not (region-active-p)))
>            (deactivate-mark))

Seems to work without issues

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> From: Andrey Listopadov <andreyorst@gmail.com>
> Date: Fri, 7 Jan 2022 21:17:41 +0300
> Cc: 52896@debbugs.gnu.org
> 
> > Sorry, please use the patch below instead:
> >
> > diff --git a/lisp/simple.el b/lisp/simple.el
> > index 070d276..cbcde9f 100644
> > --- a/lisp/simple.el
> > +++ b/lisp/simple.el
> > @@ -6664,7 +6664,7 @@ exchange-point-and-mark
> >          (user-error "No mark set in this buffer"))
> >      (set-mark (point))
> >      (goto-char omark)
> > -    (cond (temp-highlight
> > +    (cond ((and (not arg) (not temp-highlight))
> >            (setq-local transient-mark-mode (cons 'only transient-mark-mode)))
> >           ((xor arg (not (region-active-p)))
> >            (deactivate-mark))
> 
> Seems to work without issues

Thanks for testing.  I installed this on the master branch, and will
consider backporting to emacs-28 if no issues are reported for it.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

> Thanks for testing.  I installed this on the master branch, and will
> consider backporting to emacs-28 if no issues are reported for it.

Thank you! Will keep an eye on this in case any errors show up.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Lars Ingebrigtsen]

Andrey Listopadov <andreyorst@gmail.com> writes:

>> Thanks for testing.  I installed this on the master branch, and will
>> consider backporting to emacs-28 if no issues are reported for it.
>
> Thank you! Will keep an eye on this in case any errors show up.

This was a month ago -- Eli, should this be backported?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Andrey Listopadov]

On February 13, 2022 10:11:10 AM UTC, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>Andrey Listopadov <andreyorst@gmail.com> writes:
>
>>> Thanks for testing.  I installed this on the master branch, and will
>>> consider backporting to emacs-28 if no issues are reported for it.
>>
>> Thank you! Will keep an eye on this in case any errors show up.
>
>This was a month ago -- Eli, should this be backported?
>

No issues so far on my side either

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Eli Zaretskii]

> Date: Sun, 13 Feb 2022 11:31:44 +0000
> From: Andrey Listopadov <andreyorst@gmail.com>
> CC: Eli Zaretskii <eliz@gnu.org>, 52896@debbugs.gnu.org
> 
> On February 13, 2022 10:11:10 AM UTC, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> >Andrey Listopadov <andreyorst@gmail.com> writes:
> >
> >>> Thanks for testing.  I installed this on the master branch, and will
> >>> consider backporting to emacs-28 if no issues are reported for it.
> >>
> >> Thank you! Will keep an eye on this in case any errors show up.
> >
> >This was a month ago -- Eli, should this be backported?
> >
> 
> No issues so far on my side either

Alright, cherry-picked now.

bug#52896: 29.0.50; transient-mark-mode is set to a lot of 'only syms

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> No issues so far on my side either
>
> Alright, cherry-picked now.

Thanks; I'm closing this bug report, then.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no