GNU bug report logs - #47566
28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'

Previous Next

Package: emacs;

Reported by: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>

Date: Fri, 2 Apr 2021 14:51:02 UTC

Severity: normal

Found in version 28.0.50

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47566 in the body.
You can then email your comments to 47566 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Fri, 02 Apr 2021 14:51:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 02 Apr 2021 14:51:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'
Date: Fri, 2 Apr 2021 11:10:13 +0530
28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'.

Commentary in `diff-hl' says

    ;; `diff-hl-mode' highlights uncommitted changes on the side of the
    ;; window (using the fringe, by default), allows you to jump between
    ;; the hunks and revert them selectively.

    ;; Provided commands:
    ;;
    ;; `diff-hl-diff-goto-hunk'  C-x v =
    ;; `diff-hl-revert-hunk'     C-x v n
    ;; `diff-hl-previous-hunk'   C-x v [
    ;; `diff-hl-next-hunk'       C-x v ]
    ;;
    ;; The mode takes advantage of `smartrep' if it is installed.

FWIW, `smartrep' is not part of GNU Emacs or GNU ELPA but available
elsewhere.  (I install it via MELPA)

Now, that `repeat-mode' is part of GNU Emacs, I think this dependency
on `smartrep' can be removed.

FWIW, this is what I have in my `.emacs'.

    (progn
      (defvar diff-hl--repeat-map
	(let
	    ((map
	      (make-sparse-keymap)))
	  map))
      (message "Installed %s" 'diff-hl--repeat-map)
      (cl-loop for
	       (cmd . key)
	       in
	       '((diff-hl-diff-goto-hunk . "=")
		 (diff-hl-revert-hunk . "n")
		 (diff-hl-previous-hunk . "[")
		 (diff-hl-next-hunk . "]"))
	       do
	       (define-key diff-hl--repeat-map key cmd)
	       (put cmd 'repeat-map 'diff-hl--repeat-map)))

----------------

In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-03-20 built on debian
Repository revision: 31544bc908d35bff513450bc4bea1d0283a7ddb0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sat, 03 Apr 2021 22:10:02 GMT) Full text and rfc822 format available.

Message #8 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 47566 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Sun, 4 Apr 2021 01:08:55 +0300
Hi!

On 02.04.2021 08:40, Ramesh Nedunchezian wrote:
> FWIW, `smartrep' is not part of GNU Emacs or GNU ELPA but available
> elsewhere.  (I install it via MELPA)
> 
> Now, that `repeat-mode' is part of GNU Emacs, I think this dependency
> on `smartrep' can be removed.

I'd be happy to migrate away from smartrep, as soon as the functionality 
is equal, and all supported Emacs versions have repeat-mode.

> FWIW, this is what I have in my `.emacs'.
> 
>      (progn
>        (defvar diff-hl--repeat-map
> 	(let
> 	    ((map
> 	      (make-sparse-keymap)))
> 	  map))
>        (message "Installed %s" 'diff-hl--repeat-map)
>        (cl-loop for
> 	       (cmd . key)
> 	       in
> 	       '((diff-hl-diff-goto-hunk . "=")
> 		 (diff-hl-revert-hunk . "n")
> 		 (diff-hl-previous-hunk . "[")
> 		 (diff-hl-next-hunk . "]"))
> 	       do
> 	       (define-key diff-hl--repeat-map key cmd)
> 	       (put cmd 'repeat-map 'diff-hl--repeat-map)))

Try this alternative too, seems shorter:

(map-keymap
 (lambda (_key cmd)
   (put cmd 'repeat-map 'diff-hl-command-map))
 diff-hl-command-map)

But either version (together with enabling repeat-mode) seems to break 
diff-hl-revert-hunk: as soon as it reaches the the y-or-n prompt, and I 
try to answer 'n', it enters the repeat loop again, instead of sending 
the result to the command.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Mon, 05 Apr 2021 20:47:02 GMT) Full text and rfc822 format available.

Message #11 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>, 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 05 Apr 2021 23:43:32 +0300
>>        (cl-loop for
>> 	       (cmd . key)
>> 	       in
>> 	       '((diff-hl-diff-goto-hunk . "=")
>> 		 (diff-hl-revert-hunk . "n")
>> 		 (diff-hl-previous-hunk . "[")
>> 		 (diff-hl-next-hunk . "]"))
>> 	       do
>> 	       (define-key diff-hl--repeat-map key cmd)
>> 	       (put cmd 'repeat-map 'diff-hl--repeat-map)))
>
> Try this alternative too, seems shorter:
>
> (map-keymap
>  (lambda (_key cmd)
>    (put cmd 'repeat-map 'diff-hl-command-map))
>  diff-hl-command-map)
>
> But either version (together with enabling repeat-mode) seems to break
> diff-hl-revert-hunk: as soon as it reaches the the y-or-n prompt, and I try
> to answer 'n', it enters the repeat loop again, instead of sending the
> result to the command.

Could you provide a minimal test case?  I tried with:

#+begin_src emacs-lisp
(defun hl-test ()
  (interactive)
  (message "OK")
  (y-or-n-p "Yes? "))

(defvar hl-repeat-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "r") 'hl-test)
    map)
  "Keymap to repeat hl-test.")
(put 'hl-test 'repeat-map 'hl-repeat-map)
#+end_src

Then typing `M-x hl-test RET', and `y r y r y r ...' keeps the loop.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Tue, 06 Apr 2021 22:45:02 GMT) Full text and rfc822 format available.

Message #14 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>, 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Wed, 7 Apr 2021 01:44:07 +0300
Hi Juri,

On 05.04.2021 23:43, Juri Linkov wrote:
> Could you provide a minimal test case?  I tried with:
> 
> #+begin_src emacs-lisp
> (defun hl-test ()
>    (interactive)
>    (message "OK")
>    (y-or-n-p "Yes? "))
> 
> (defvar hl-repeat-map
>    (let ((map (make-sparse-keymap)))
>      (define-key map (kbd "r") 'hl-test)
>      map)
>    "Keymap to repeat hl-test.")
> (put 'hl-test 'repeat-map 'hl-repeat-map)
> #+end_src
> 
> Then typing `M-x hl-test RET', and `y r y r y r ...' keeps the loop.

Here you go:

(defun hl-test ()
  (interactive)
  (message "OK")
  (message "result: %s"
           (y-or-n-p "Yes? ")))

(defvar hl-repeat-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "n") 'hl-test) ; Note the changed key binding.
    map)
  "Keymap to repeat hl-test.")

(put 'hl-test 'repeat-map 'hl-repeat-map)

To try it:

1. M-x hl-test.
2. Press 'n' a few times.

Expected behavior:

It alternates between the prompt "Yes? " and message "result: nil".

Actual behavior:

It enters some sort of recursive state, only showing the prompt. I have 
to press 'y' a bunch of times to get out of it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Thu, 08 Apr 2021 18:58:02 GMT) Full text and rfc822 format available.

Message #17 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>, 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Thu, 08 Apr 2021 21:57:18 +0300
> (defun hl-test ()
>   (interactive)
>   (message "OK")
>   (message "result: %s"
>            (y-or-n-p "Yes? ")))
>
> (defvar hl-repeat-map
>   (let ((map (make-sparse-keymap)))
>     (define-key map (kbd "n") 'hl-test) ; Note the changed key binding.
>     map)
>   "Keymap to repeat hl-test.")
>
> (put 'hl-test 'repeat-map 'hl-repeat-map)
>
> To try it:
>
> 1. M-x hl-test.
> 2. Press 'n' a few times.
>
> Expected behavior:
>
> It alternates between the prompt "Yes? " and message "result: nil".
>
> Actual behavior:
>
> It enters some sort of recursive state, only showing the prompt. I have to
> press 'y' a bunch of times to get out of it.

Thanks for the detailed test case.  Now it's fixed in 580c4c6510.




Reply sent to Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility. (Sat, 10 Apr 2021 01:41:02 GMT) Full text and rfc822 format available.

Notification sent to Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>:
bug acknowledged by developer. (Sat, 10 Apr 2021 01:41:02 GMT) Full text and rfc822 format available.

Message #22 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Sat, 10 Apr 2021 04:40:49 +0300
On 08.04.2021 21:57, Juri Linkov wrote:
> Thanks for the detailed test case.  Now it's fixed in 580c4c6510.

Thanks! repeat-mode is looking good now (*).

I've added integration for it to diff-hl master, and with that we can 
close this issue.

Thanks all.

(*) Though the first impression, in comparison, was that it is too 
chatty. The hints are definitely helpful for discovery at first, though.

Maybe something like this would be an improvement? Experimental code 
warning.

diff --git a/lisp/repeat.el b/lisp/repeat.el
index b3c58f2f81..e704e4da56 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -404,7 +404,7 @@ repeat-post-hook
                                        (key-description repeat-exit-key))
                              ""))))
                 (if (current-message)
-                    (message "%s [%s]" (current-message) mess)
+                    (message #("%s [%s]" 3 7 (face deemphasized)) 
(current-message) mess)
                   (message mess))))

             ;; Adding an exit key




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sat, 10 Apr 2021 08:40:02 GMT) Full text and rfc822 format available.

Message #25 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Juri Linkov <juri <at> linkov.net>
Cc: 47566-done <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Sat, 10 Apr 2021 14:08:50 +0530

On 10/04/21 7:10 am, Dmitry Gutov wrote:

> (*) Though the first impression, in comparison, was that it is too chatty. The hints are definitely helpful for discovery at first, though.
> 
> Maybe something like this would be an improvement? Experimental code warning.
> 
> diff --git a/lisp/repeat.el b/lisp/repeat.el
> index b3c58f2f81..e704e4da56 100644
> --- a/lisp/repeat.el
> +++ b/lisp/repeat.el
> @@ -404,7 +404,7 @@ repeat-post-hook
>                                         (key-description repeat-exit-key))
>                               ""))))
>                  (if (current-message)
> -                    (message "%s [%s]" (current-message) mess)
> +                    (message #("%s [%s]" 3 7 (face deemphasized)) (current-message) mess)
>                    (message mess))))
> 
>              ;; Adding an exit key

I created a repeat map for rectangle commands, and the echo area
becomes much more chattier. See
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47688.

It seems to me that repeat-mode is essentially a poor man's hydra.
May be the the symbol that holds the repeat map can specify a
`:help'-er property.

This `:help'-er can either

(a) give a fancy help string which the `repet-mode' can display in a
    pop-up

or

(b) the helper itself can prepare the string and arrange for providing
    a pop-up.

What I am saying is let the repeat map provide it's own `:help'-er
which the `repeat-mode' can hook in to.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sat, 10 Apr 2021 23:23:01 GMT) Full text and rfc822 format available.

Message #28 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Sun, 11 Apr 2021 01:58:13 +0300
> Maybe something like this would be an improvement? Experimental
> code warning.
>
> diff --git a/lisp/repeat.el b/lisp/repeat.el
> index b3c58f2f81..e704e4da56 100644
> --- a/lisp/repeat.el
> +++ b/lisp/repeat.el
> @@ -404,7 +404,7 @@ repeat-post-hook
>                                         (key-description repeat-exit-key))
>                               ""))))
>                  (if (current-message)
> -                    (message "%s [%s]" (current-message) mess)
> +                    (message #("%s [%s]" 3 7 (face deemphasized)) (current-message) mess)

I can't find the face named 'deemphasized'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sat, 10 Apr 2021 23:23:02 GMT) Full text and rfc822 format available.

Message #31 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Cc: 47566-done <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Sun, 11 Apr 2021 01:59:47 +0300
> It seems to me that repeat-mode is essentially a poor man's hydra.

It is.  It provides only the basic functionality.
Don't expect many fancy things from repeat-mode.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sat, 10 Apr 2021 23:28:01 GMT) Full text and rfc822 format available.

Message #34 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Sun, 11 Apr 2021 02:27:04 +0300
On 11.04.2021 01:58, Juri Linkov wrote:
> I can't find the face named 'deemphasized'.

Sorry.

Apparently, it's a face that only the tango-plus theme defines (unwisely).

It simply looks like less contrasting text. Try 'shadow' instead, it's 
about the same.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sun, 11 Apr 2021 06:50:01 GMT) Full text and rfc822 format available.

Message #37 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566-done <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Sun, 11 Apr 2021 12:18:43 +0530
On 11/04/21 4:29 am, Juri Linkov wrote:
>> It seems to me that repeat-mode is essentially a poor man's hydra.
> 
> It is.  It provides only the basic functionality.
> Don't expect many fancy things from repeat-mode.

The code below is pulled out from the existing code i.e., it doesn't
add any additional layer of complexity or does anything fanciful.  But
it does add a bit of extensibility.

This is what I was suggesting:

    (defvar repeat-mode-helper
      (defun repeat-mode-helper (map)
	(let (keys)
	  (message "Coming here")
	  (map-keymap (lambda (key _) (push key keys)) map)
	  (let ((mess (format-message
		       "Repeat with %s%s"
		       (mapconcat (lambda (key)
				    (key-description (vector key)))
				  keys ", ")
		       (if repeat-exit-key
			   (format ", or exit with %s"
				   (key-description repeat-exit-key))
			 ""))))
	    (if (current-message)
		(message "%s [%s]" (current-message) mess)
	      (message mess))))))


And in `repeat-post-hook', do something like

    ;; Messaging
    (unless prefix-arg
      (funcall (or (get rep-sym 'help) repeat-mode-helper) rep-map))


For those who don't want hints, they can use

    (setq repeat-mode-helper #'ignore)

For those who want hints, but do /not/ want the hints hogging the echo
area, they could have their own custom helper like the one above,
after replacing

	(message mess)

with

	(tooltip-show mess)

--------------------------------

I would also appreciate if you could assess adding the exit key as a
property to the repeat mode symbol.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sun, 11 Apr 2021 23:13:01 GMT) Full text and rfc822 format available.

Message #40 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Cc: 47566-done <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 12 Apr 2021 01:51:53 +0300
> This is what I was suggesting:
>
>     (defvar repeat-mode-helper
>       (defun repeat-mode-helper (map)
>[...]
>
> For those who don't want hints, they can use
>
>     (setq repeat-mode-helper #'ignore)

Thanks, this is a good idea.  Maybe the name is not the best one
since the word "helper" often means a place that provides some
utility functions.

Using the Emacs terminology, a better name would be 'repeat-mode-echo'.
I'll add such option soon.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Mon, 12 Apr 2021 16:19:03 GMT) Full text and rfc822 format available.

Message #43 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Cc: 47566-done <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 12 Apr 2021 19:16:38 +0300
> For those who don't want hints, they can use
>
>     (setq repeat-mode-helper #'ignore)
>
> For those who want hints, but do /not/ want the hints hogging the echo
> area, they could have their own custom helper like the one above,
> after replacing
>
> 	(message mess)
>
> with
>
> 	(tooltip-show mess)

This is implemented now, so you can customize it to disable messaging,
to do truncation of long messages, or to use tooltip-show, etc.

> I would also appreciate if you could assess adding the exit key as a
> property to the repeat mode symbol.

The exit key specific to some keymap can be easily added to that keymap.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Mon, 12 Apr 2021 16:19:03 GMT) Full text and rfc822 format available.

Message #46 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 12 Apr 2021 19:17:36 +0300
>> I can't find the face named 'deemphasized'.
>
> Sorry.
>
> Apparently, it's a face that only the tango-plus theme defines (unwisely).
>
> It simply looks like less contrasting text. Try 'shadow' instead, it's
> about the same.

I'm not sure about deemphasizing by default.  But now it's easy to
customize this, and add any emphazing.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Tue, 13 Apr 2021 23:55:02 GMT) Full text and rfc822 format available.

Message #49 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Wed, 14 Apr 2021 02:54:06 +0300
On 12.04.2021 19:17, Juri Linkov wrote:
>>> I can't find the face named 'deemphasized'.
>>
>> Sorry.
>>
>> Apparently, it's a face that only the tango-plus theme defines (unwisely).
>>
>> It simply looks like less contrasting text. Try 'shadow' instead, it's
>> about the same.
> 
> I'm not sure about deemphasizing by default.  But now it's easy to
> customize this, and add any emphazing.

Thanks.

Consider adding a repeat-mode-echo option which follows what Smartrep 
does: instead of adding to the echo area, it just highlights the changed 
state in the mode line (basically a long mode lighter saying 
"========SMARTREP=======") and also colors the mode-line green.

IME that works well as an indicator that we're in a "special" state.

Though perhaps we could use a smaller text (like "[REPEAT]") and only 
color the text inside the brackets, rather than the whole mode-line. 
Could use some experimentation.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 07:15:01 GMT) Full text and rfc822 format available.

Message #52 received at 47566-done <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 47566-done <at> debbugs.gnu.org,
 Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: 28.0.50; diff-hl should use `repeat-mode' ... and not `smartrep'
Date: Wed, 14 Apr 2021 09:14:12 +0200
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Consider adding a repeat-mode-echo option which follows what Smartrep
> does: instead of adding to the echo area, it just highlights the
> changed state in the mode line (basically a long mode lighter saying
> "========SMARTREP=======") and also colors the mode-line green.
>
> IME that works well as an indicator that we're in a "special" state.
>
> Though perhaps we could use a smaller text (like "[REPEAT]") and only
> color the text inside the brackets, rather than the whole
> mode-line. Could use some experimentation.

An entry in mode-line-modes, à la compilation-in-progress, would be
lovely IMO.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 18:16:01 GMT) Full text and rfc822 format available.

Message #55 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50;
 diff-hl should use `repeat-mode' ... and not `smartrep'
Date: Wed, 14 Apr 2021 21:10:12 +0300
>> Consider adding a repeat-mode-echo option which follows what Smartrep
>> does: instead of adding to the echo area, it just highlights the
>> changed state in the mode line (basically a long mode lighter saying
>> "========SMARTREP=======") and also colors the mode-line green.
>>
>> IME that works well as an indicator that we're in a "special" state.
>>
>> Though perhaps we could use a smaller text (like "[REPEAT]") and only
>> color the text inside the brackets, rather than the whole
>> mode-line. Could use some experimentation.
>
> An entry in mode-line-modes, à la compilation-in-progress, would be
> lovely IMO.

compilation-in-progress is a good example of an unobtrusive indicator
in the mode line.  Now added to repeat-mode.

PS: like your message-subject-re-regexp turned the subject into
"Re: peat lambda" on emacs-devel, it removed the bug number
from the subject here :)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 19:14:02 GMT) Full text and rfc822 format available.

Message #58 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Wed, 14 Apr 2021 21:12:55 +0200
Juri Linkov <juri <at> linkov.net> writes:

>> An entry in mode-line-modes, à la compilation-in-progress, would be
>> lovely IMO.
>
> compilation-in-progress is a good example of an unobtrusive indicator
> in the mode line.  Now added to repeat-mode.

Thanks!  Was this bit intentional?

> ;;;###autoload
> (put 'mode-line-defining-kbd-macro 'risky-local-variable t)

Also, AFAICT the indication (whether in the mode-line or in the echo
area) only shows up once I start repeating a key (e.g. after C-x o o),
not when I first hit a repeatable key (e.g. after C-x o).

Is this intended?  I figure it would make sense to display the
indication before the user starts repeating, since the indication can
serve as a warning that repetition might occur.


At any rate, thanks a lot for working on this.  (And thanks to all for
the addition to diff-hl!)


> PS: like your message-subject-re-regexp turned the subject into
> "Re: peat lambda" on emacs-devel, it removed the bug number
> from the subject here :)

🤦 Thanks for the heads-up.  IIUC message-subject-re-regexp is used both
by Gnus summary buffers to "canonicalize" subjects in order to only show
them for thread roots, *and* by message-mode to strip cruft before
adding "Re:" to a reply.  I'll need to figure a way to decorrelate those
two uses…

(This is completely off-topic of course; I'm just thinking out loud)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 19:57:02 GMT) Full text and rfc822 format available.

Message #61 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Wed, 14 Apr 2021 22:52:28 +0300
> Thanks!  Was this bit intentional?
>
>> ;;;###autoload
>> (put 'mode-line-defining-kbd-macro 'risky-local-variable t)

Thanks for noticing this, I stole this from bindings.el,
but now that repeat-echo-mode-line-string is highlighted
correctly in the mode-line, I wonder if such property is needed
for repeat-echo-mode-line-string at all?  And why it was needed
for mode-line-defining-kbd-macro to keep its text properties?

After removing this line from bindings.el:
  (put 'mode-line-defining-kbd-macro 'risky-local-variable t)
mode-line-defining-kbd-macro loses its text property
face=font-lock-warning-face, but repeat-echo-mode-line-string keeps it.

This is explained in (info "(elisp) Properties in Mode"),
but I don't understand why text properties are preserved for
repeat-echo-mode-line-string without ‘risky-local-variable’ property?

> Also, AFAICT the indication (whether in the mode-line or in the echo
> area) only shows up once I start repeating a key (e.g. after C-x o o),
> not when I first hit a repeatable key (e.g. after C-x o).

Strange, I see it immediately after C-x o.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 20:24:01 GMT) Full text and rfc822 format available.

Message #64 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Wed, 14 Apr 2021 23:21:08 +0300
>> Also, AFAICT the indication (whether in the mode-line or in the echo
>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>> not when I first hit a repeatable key (e.g. after C-x o).
>
> Strange, I see it immediately after C-x o.

Could you please try to move 'force-mode-line-update' out of condition,
so it updates the mode-line on activating the repeat map as well:

#+begin_src emacs-lisp
(defun repeat-echo-mode-line (map)
  "Display the repeat indicator in the mode line."
  (if map
      (unless (assq 'repeat-in-progress mode-line-modes)
        (add-to-list 'mode-line-modes (list 'repeat-in-progress
                                            repeat-echo-mode-line-string))))
  (force-mode-line-update t))
#+end_src

Maybe your mode-line is not updated immediately?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 20:49:02 GMT) Full text and rfc822 format available.

Message #67 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Wed, 14 Apr 2021 22:48:51 +0200
Juri Linkov <juri <at> linkov.net> writes:

>> Also, AFAICT the indication (whether in the mode-line or in the echo
>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>> not when I first hit a repeatable key (e.g. after C-x o).
>
> Strange, I see it immediately after C-x o.

🤦 My apologies, I should have tried with emacs -Q before jumping to
conclusions; I actually had left C-x o bound to my own command…  I hope
I didn't waste too much of your time on this.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 22:50:02 GMT) Full text and rfc822 format available.

Message #70 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 47566 <at> debbugs.gnu.org, Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Thu, 15 Apr 2021 00:58:16 +0300
>>> Also, AFAICT the indication (whether in the mode-line or in the echo
>>> area) only shows up once I start repeating a key (e.g. after C-x o o),
>>> not when I first hit a repeatable key (e.g. after C-x o).
>>
>> Strange, I see it immediately after C-x o.
>
> 🤦 My apologies, I should have tried with emacs -Q before jumping to
> conclusions; I actually had left C-x o bound to my own command…  I hope
> I didn't waste too much of your time on this.

No problem.  I just wondered why the mode-line is automatically updated
when repeat-in-progress in mode-line-modes is non-nil, but not updated
when it becomes nil.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Wed, 14 Apr 2021 23:36:02 GMT) Full text and rfc822 format available.

Message #73 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>,
 Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: Ramesh Nedunchezian <rameshnedunchezian <at> outlook.com>, 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and not
 `smartrep'
Date: Thu, 15 Apr 2021 02:35:19 +0300
On 14.04.2021 21:10, Juri Linkov wrote:
> compilation-in-progress is a good example of an unobtrusive indicator
> in the mode line.  Now added to repeat-mode.

Looks nice, thank you.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 13 May 2021 11:24:07 GMT) Full text and rfc822 format available.

bug unarchived. Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Sat, 13 Nov 2021 19:36:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Sun, 14 Nov 2021 20:33:02 GMT) Full text and rfc822 format available.

Message #80 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Sun, 14 Nov 2021 22:25:07 +0200
>> (defun hl-test ()
>>   (interactive)
>>   (message "OK")
>>   (message "result: %s"
>>            (y-or-n-p "Yes? ")))
>>
>> (defvar hl-repeat-map
>>   (let ((map (make-sparse-keymap)))
>>     (define-key map (kbd "n") 'hl-test) ; Note the changed key binding.
>>     map)
>>   "Keymap to repeat hl-test.")
>>
>> (put 'hl-test 'repeat-map 'hl-repeat-map)
>>
>> To try it:
>>
>> 1. M-x hl-test.
>> 2. Press 'n' a few times.
>>
>> Expected behavior:
>>
>> It alternates between the prompt "Yes? " and message "result: nil".
>>
>> Actual behavior:
>>
>> It enters some sort of recursive state, only showing the prompt. I have to
>> press 'y' a bunch of times to get out of it.
>
> Thanks for the detailed test case.  Now it's fixed in 580c4c6510.

This was a pretty bad fix.  It broke a lot of useful workflows.
For example, when the minibuffer is active, it's not possible anymore
to switch from the minibuffer to the original buffer and back using 'C-x o o'.
Also multiple undo with 'C-x u u u ...' is not available in the
minibuffer anymore.  I'm trying to find a proper fix.

The first thing how I tried to handle the above test case is to
disable repeat-mode only in the minibuffer activated from y-or-n-p:

diff --git a/lisp/subr.el b/lisp/subr.el
index 8ff403e113..de5a512946 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3324,9 +3324,12 @@ y-or-n-p
                        map))
              ;; Protect this-command when called from pre-command-hook (bug#45029)
              (this-command this-command)
-             (str (read-from-minibuffer
-                   prompt nil keymap nil
-                   (or y-or-n-p-history-variable 'empty-history))))
+             (str (minibuffer-with-setup-hook
+                      (lambda ()
+                        (setq-local repeat-mode nil))
+                    (read-from-minibuffer
+                     prompt nil keymap nil
+                     (or y-or-n-p-history-variable 'empty-history)))))
         (setq answer (if (member str '("y" "Y")) 'act 'skip)))))
     (let ((ret (eq answer 'act)))
       (unless noninteractive

But it doesn't seems appropriate to mention repeat-mode in y-or-n-p.

The next thing tried was to detect the y-or-n-p minibuffer in repeat-mode:

diff --git a/lisp/repeat.el b/lisp/repeat.el
index ac08952eaa..53046714bd 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -417,7 +419,7 @@ repeat-post-hook
 
             ;; Exit when the last char is not among repeatable keys,
             ;; so e.g. `C-x u u' repeats undo, whereas `C-/ u' doesn't.
-            (when (and (zerop (minibuffer-depth)) ; avoid remapping in prompts
+            (when (and (not (eq (key-binding "n") 'y-or-n-p-insert-n))
                        (or (lookup-key map (this-command-keys-vector))
                            prefix-arg))
 
But special handling of y-or-n in repeat-mode seems inappropriate too.

What would be a general rule when repeating should be disabled?
Looks like the most relevant event is when the minibuffer pops up
in the middle of repeating sequence.  But minibuffer-depth can't be used
to detect changes in the minibuffer presence.  Because there are cases
when minibuffer-depth is not changed when the minibuffer is activated.

For example, with the above test case: after typing `M-x hl-test RET',
minibuffer-depth is 1 in pre-command-hook after typing RET.  Then the
next command activates own minibuffer with y-or-n-p, and again
minibuffer-depth is 1 in post-command-hook.

What I'm going to try is a combination of minibuffer-depth and
current-minibuffer-command.  Then in pre-command-hook,
current-minibuffer-command is 'execute-extended-command',
but in post-command-hook it's 'hl-test'.

Also worth to note that 'hl-test' that uses 'y-or-n-p' (and 'y-or-n-p'
preserves the original 'this-command') can be repeatable only with
the following patch.  Some time ago 'this-command' was replaced with
'real-this-command' on the request in bug#47688, but actually
need to check both 'this-command' and 'real-this-command':

diff --git a/lisp/repeat.el b/lisp/repeat.el
index ac08952eaa..c136b0cee4 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -408,6 +411,8 @@ repeat-post-hook
     (setq repeat-in-progress nil)
     (when repeat-mode
       (let ((rep-map (or repeat-map
+                         (and (symbolp this-command)
+                              (get this-command 'repeat-map))
                          (and (symbolp real-this-command)
                               (get real-this-command 'repeat-map)))))
         (when rep-map




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Mon, 15 Nov 2021 05:28:01 GMT) Full text and rfc822 format available.

Message #83 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 15 Nov 2021 06:26:50 +0100
Juri Linkov <juri <at> linkov.net> writes:

> What would be a general rule when repeating should be disabled?
> Looks like the most relevant event is when the minibuffer pops up
> in the middle of repeating sequence.

I think that sounds correct.

> What I'm going to try is a combination of minibuffer-depth and
> current-minibuffer-command.  Then in pre-command-hook,
> current-minibuffer-command is 'execute-extended-command',
> but in post-command-hook it's 'hl-test'.

Sounds promising.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#47566; Package emacs. (Mon, 15 Nov 2021 17:47:02 GMT) Full text and rfc822 format available.

Message #86 received at 47566 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 47566 <at> debbugs.gnu.org
Subject: Re: bug#47566: 28.0.50; diff-hl should use `repeat-mode' ... and
 not `smartrep'
Date: Mon, 15 Nov 2021 19:40:51 +0200
>> What would be a general rule when repeating should be disabled?
>> Looks like the most relevant event is when the minibuffer pops up
>> in the middle of repeating sequence.
>
> I think that sounds correct.
>
>> What I'm going to try is a combination of minibuffer-depth and
>> current-minibuffer-command.  Then in pre-command-hook,
>> current-minibuffer-command is 'execute-extended-command',
>> but in post-command-hook it's 'hl-test'.
>
> Sounds promising.

So the fix is pushed now.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 14 Dec 2021 12:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 95 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.