bug#15525: 24.3; register-read-with-preview

bug#15525: 24.3; register-read-with-preview

[Leo Liu]

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

register-read-with-preview pops up a window showing existing registers
and brief description. Any comments?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reg.diff --]
[-- Type: text/x-patch, Size: 7833 bytes --]

=== modified file 'lisp/register.el'
--- lisp/register.el	2013-08-09 00:30:24 +0000
+++ lisp/register.el	2013-10-04 05:48:34 +0000
@@ -89,6 +89,11 @@
   :type '(choice (const :tag "None" nil)
 		 (character :tag "Use register" :value ?+)))
 
+(defcustom register-preview-delay 0.5
+  "Delay in seconds before popping up the preview window."
+  :type 'number
+  :group 'register)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (cdr (assq register register-alist)))
@@ -102,12 +107,54 @@
       (push (cons register value) register-alist))
     value))
 
+(defun register-describe-oneline (c)
+  (let ((d (replace-regexp-in-string
+            "\n[ \t]*" " "
+            (with-output-to-string (describe-register-1 c)))))
+    (if (string-match "Register.+? contains \\(?:an? \\|the \\)?" d)
+        (substring d (match-end 0))
+      d)))
+
+(defvar register-preview-functions nil)
+
+(defun register-preview (buffer)
+  "Pop up a window to show register preview in BUFFER."
+  (when (consp register-alist)
+    (let ((split-height-threshold 0))
+      (with-temp-buffer-window
+       buffer
+       (cons 'display-buffer-below-selected
+	     '((window-height . fit-window-to-buffer)))
+       nil
+       (with-current-buffer standard-output
+	 (setq cursor-in-non-selected-windows nil)
+	 (mapc (lambda (r)
+		 (insert
+		  (format
+		   "%s %s\n"
+		   (concat (single-key-description (car r)) ":")
+		   (or (run-hook-with-args 'register-preview-functions (cdr r))
+		       (register-describe-oneline (car r))))))
+	       register-alist))))))
+
+(defun register-read-with-preview (prompt)
+  (let* ((buffer "*Register Preview*")
+	 (timer (run-with-timer register-preview-delay nil
+				#'register-preview buffer)))
+    (unwind-protect
+	(read-event (propertize prompt 'face 'minibuffer-prompt))
+      (and (timerp timer) (cancel-timer timer))
+      (let ((w (get-buffer-window buffer)))
+        (and (window-live-p w) (delete-window w)))
+      (and (get-buffer buffer) (kill-buffer buffer)))))
+
 (defun point-to-register (register &optional arg)
   "Store current location of point in register REGISTER.
 With prefix argument, store current frame configuration.
 Use \\[jump-to-register] to go to that location or restore that configuration.
 Argument is a character, naming the register."
-  (interactive "cPoint to register: \nP")
+  (interactive (list (register-read-with-preview "Point to register: ")
+		     current-prefix-arg))
   ;; Turn the marker into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
   (set-register register
@@ -118,7 +165,9 @@
   "Store the window configuration of the selected frame in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cWindow configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Window configuration to register: ")
+		     current-prefix-arg))
   ;; current-window-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-window-configuration) (point-marker))))
@@ -127,7 +176,9 @@
   "Store the window configuration of all frames in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cFrame configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Frame configuration to register: ")
+		     current-prefix-arg))
   ;; current-frame-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-frame-configuration) (point-marker))))
@@ -143,7 +194,8 @@
 Optional second arg non-nil (interactively, prefix argument) says to
 delete any existing frames that the frameset doesn't mention.
 \(Otherwise, these frames are iconified.)"
-  (interactive "cJump to register: \nP")
+  (interactive (list (register-read-with-preview "Jump to register: ")
+		     current-prefix-arg))
   (let ((val (get-register register)))
     (cond
      ((registerv-p val)
@@ -190,7 +242,8 @@
 If NUMBER is nil, a decimal number is read from the buffer starting
 at point, and point moves to the end of that number.
 Interactively, NUMBER is the prefix arg (none means nil)."
-  (interactive "P\ncNumber to register: ")
+  (interactive (list current-prefix-arg
+		     (register-read-with-preview "Number to register: ")))
   (set-register register
 		(if number
 		    (prefix-numeric-value number)
@@ -222,7 +275,7 @@
 (defun view-register (register)
   "Display what is contained in register named REGISTER.
 The Lisp value REGISTER is a character."
-  (interactive "cView register: ")
+  (interactive (list (register-read-with-preview "View register: ")))
   (let ((val (get-register register)))
     (if (null val)
 	(message "Register %s is empty" (single-key-description register))
@@ -323,7 +376,10 @@
 Normally puts point before and mark after the inserted text.
 If optional second arg is non-nil, puts mark before and point after.
 Interactively, second arg is non-nil if prefix arg is supplied."
-  (interactive "*cInsert register: \nP")
+  (interactive (progn
+		 (barf-if-buffer-read-only)
+		 (register-read-with-preview "Insert register: ")
+		 current-prefix-arg))
   (push-mark)
   (let ((val (get-register register)))
     (cond
@@ -349,7 +405,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to copy."
-  (interactive "cCopy to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Copy to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (set-register register (filter-buffer-substring start end))
   (setq deactivate-mark t)
   (cond (delete-flag
@@ -362,7 +421,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to append."
-  (interactive "cAppend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Append to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -381,7 +443,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to prepend."
-  (interactive "cPrepend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Prepend to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -402,7 +467,11 @@
 
 Called from a program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions giving two corners of rectangle."
-  (interactive "cCopy rectangle to register: \nr\nP")
+  (interactive (list (register-read-with-preview
+		      "Copy rectangle to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((rectangle (if delete-flag
 		       (delete-extract-rectangle start end)
 		     (extract-rectangle start end))))
@@ -412,6 +481,5 @@
       (setq deactivate-mark t)
       (indicate-copied-region (length (car rectangle))))))
 
-
 (provide 'register)
 ;;; register.el ends here

bug#15525: 24.3; register-read-with-preview

[Stefan Monnier]

> register-read-with-preview pops up a window showing existing registers
> and brief description. Any comments?

I like the idea of displaying the available choices, but I'm not sure
"0.5s delay" is a good enough trigger: it's probably a fine choice for
some users, but usually Emacs doesn't like to use such time-dependent
behavior too much.

Could we offer, additionally to the delay, a different trigger
(e.g. help-char and help-event-list)?


        Stefan


PS: How does it compare to register-list?  Could they be consolidated?

bug#15525: 24.3; register-read-with-preview

[Bastien]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> PS: How does it compare to register-list?  Could they be
> consolidated?

I hope so.  The Org merge is higher on my todo-list, but working on
register-list.el comes right after.

-- 
 Bastien

bug#15525: 24.3; register-read-with-preview

[Leo Liu]

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

On 2013-10-04 21:57 +0800, Stefan Monnier wrote:
[snipped 3 lines]
> I like the idea of displaying the available choices, but I'm not sure
> "0.5s delay" is a good enough trigger: it's probably a fine choice for
> some users, but usually Emacs doesn't like to use such time-dependent
> behavior too much.
>
> Could we offer, additionally to the delay, a different trigger
> (e.g. help-char and help-event-list)?
>
>
>         Stefan

Good idea. The attached patch implements this. i.e. if one sets
register-preview-delay to t it means forever, i.e. the timer is never
set. Users can customise register-read-help-char to nil if they don't
want any help-char to bring up the preview window.

BTW, the delay is so that if you can remember which register to choose
within some time (0.5s) you shall never see the preview window.

[snipped 10 lines]
>> PS: How does it compare to register-list?  Could they be
>> consolidated?
>
> I hope so.  The Org merge is higher on my todo-list, but working on
> register-list.el comes right after.

I leave a hook i.e. register-preview-functions for customising how the
preview should be presented. I am not sure other ways to have
integration with register-list. Comments welcome.

Best wishes,
Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reg2.diff --]
[-- Type: text/x-patch, Size: 8241 bytes --]

=== modified file 'lisp/register.el'
--- lisp/register.el	2013-08-09 00:30:24 +0000
+++ lisp/register.el	2013-10-05 03:08:31 +0000
@@ -89,6 +89,16 @@
   :type '(choice (const :tag "None" nil)
 		 (character :tag "Use register" :value ?+)))
 
+(defcustom register-preview-delay 0.5
+  "Delay in seconds before popping up the preview window."
+  :type '(choice number (const :tag "Indefinitely" t))
+  :group 'register)
+
+(defcustom register-read-help-char help-char
+  "Character to recognize as meaning help."
+  :type '(choice character (const :tag "Never" nil))
+  :group 'register)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (cdr (assq register register-alist)))
@@ -102,12 +112,60 @@
       (push (cons register value) register-alist))
     value))
 
+(defun register-describe-oneline (c)
+  (let ((d (replace-regexp-in-string
+            "\n[ \t]*" " "
+            (with-output-to-string (describe-register-1 c)))))
+    (if (string-match "Register.+? contains \\(?:an? \\|the \\)?" d)
+        (substring d (match-end 0))
+      d)))
+
+(defvar register-preview-functions nil)
+
+(defun register-preview (buffer)
+  "Pop up a window to show register preview in BUFFER."
+  (when (consp register-alist)
+    (let ((split-height-threshold 0))
+      (with-temp-buffer-window
+       buffer
+       (cons 'display-buffer-below-selected
+	     '((window-height . fit-window-to-buffer)))
+       nil
+       (with-current-buffer standard-output
+	 (setq cursor-in-non-selected-windows nil)
+	 (mapc (lambda (r)
+		 (insert
+		  (format
+		   "%s %s\n"
+		   (concat (single-key-description (car r)) ":")
+		   (or (run-hook-with-args 'register-preview-functions (cdr r))
+		       (register-describe-oneline (car r))))))
+	       register-alist))))))
+
+(defun register-read-with-preview (prompt)
+  (let* ((buffer "*Register Preview*")
+	 (timer (when (numberp register-preview-delay)
+		  (run-with-timer register-preview-delay nil
+				  #'register-preview buffer))))
+    (unwind-protect
+	(progn
+	  (while (eq (read-event (propertize prompt 'face 'minibuffer-prompt))
+		     register-read-help-char)
+	    (unless (get-buffer-window buffer)
+	      (register-preview buffer)))
+	  last-input-event)
+      (and (timerp timer) (cancel-timer timer))
+      (let ((w (get-buffer-window buffer)))
+        (and (window-live-p w) (delete-window w)))
+      (and (get-buffer buffer) (kill-buffer buffer)))))
+
 (defun point-to-register (register &optional arg)
   "Store current location of point in register REGISTER.
 With prefix argument, store current frame configuration.
 Use \\[jump-to-register] to go to that location or restore that configuration.
 Argument is a character, naming the register."
-  (interactive "cPoint to register: \nP")
+  (interactive (list (register-read-with-preview "Point to register: ")
+		     current-prefix-arg))
   ;; Turn the marker into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
   (set-register register
@@ -118,7 +176,9 @@
   "Store the window configuration of the selected frame in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cWindow configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Window configuration to register: ")
+		     current-prefix-arg))
   ;; current-window-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-window-configuration) (point-marker))))
@@ -127,7 +187,9 @@
   "Store the window configuration of all frames in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cFrame configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Frame configuration to register: ")
+		     current-prefix-arg))
   ;; current-frame-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-frame-configuration) (point-marker))))
@@ -143,7 +205,8 @@
 Optional second arg non-nil (interactively, prefix argument) says to
 delete any existing frames that the frameset doesn't mention.
 \(Otherwise, these frames are iconified.)"
-  (interactive "cJump to register: \nP")
+  (interactive (list (register-read-with-preview "Jump to register: ")
+		     current-prefix-arg))
   (let ((val (get-register register)))
     (cond
      ((registerv-p val)
@@ -190,7 +253,8 @@
 If NUMBER is nil, a decimal number is read from the buffer starting
 at point, and point moves to the end of that number.
 Interactively, NUMBER is the prefix arg (none means nil)."
-  (interactive "P\ncNumber to register: ")
+  (interactive (list current-prefix-arg
+		     (register-read-with-preview "Number to register: ")))
   (set-register register
 		(if number
 		    (prefix-numeric-value number)
@@ -222,7 +286,7 @@
 (defun view-register (register)
   "Display what is contained in register named REGISTER.
 The Lisp value REGISTER is a character."
-  (interactive "cView register: ")
+  (interactive (list (register-read-with-preview "View register: ")))
   (let ((val (get-register register)))
     (if (null val)
 	(message "Register %s is empty" (single-key-description register))
@@ -323,7 +387,10 @@
 Normally puts point before and mark after the inserted text.
 If optional second arg is non-nil, puts mark before and point after.
 Interactively, second arg is non-nil if prefix arg is supplied."
-  (interactive "*cInsert register: \nP")
+  (interactive (progn
+		 (barf-if-buffer-read-only)
+		 (register-read-with-preview "Insert register: ")
+		 current-prefix-arg))
   (push-mark)
   (let ((val (get-register register)))
     (cond
@@ -349,7 +416,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to copy."
-  (interactive "cCopy to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Copy to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (set-register register (filter-buffer-substring start end))
   (setq deactivate-mark t)
   (cond (delete-flag
@@ -362,7 +432,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to append."
-  (interactive "cAppend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Append to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -381,7 +454,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to prepend."
-  (interactive "cPrepend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Prepend to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -402,7 +478,11 @@
 
 Called from a program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions giving two corners of rectangle."
-  (interactive "cCopy rectangle to register: \nr\nP")
+  (interactive (list (register-read-with-preview
+		      "Copy rectangle to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((rectangle (if delete-flag
 		       (delete-extract-rectangle start end)
 		     (extract-rectangle start end))))
@@ -412,6 +492,5 @@
       (setq deactivate-mark t)
       (indicate-copied-region (length (car rectangle))))))
 
-
 (provide 'register)
 ;;; register.el ends here

bug#15525: 24.3; register-read-with-preview

[Stefan Monnier]

> Good idea. The attached patch implements this. i.e. if one sets
> register-preview-delay to t it means forever,

Good (I'd have used nil rather than t, but it doesn't matter).

I wonder if the default shouldn't be "infinite delay", so as to preserve
existing behavior?

> Users can customise register-read-help-char to nil if they don't want
> any help-char to bring up the preview window.

I think the only valid reason to disable the help-char here is if it
prevents access to an existing register.  So we can get rid of this
custom var and instead make sure we only obey help-char if there is no
existing register by that name.


        Stefan

bug#15525: 24.3; register-read-with-preview

[Leo Liu]

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

On 2013-10-06 00:30 +0800, Stefan Monnier wrote:
> Good (I'd have used nil rather than t, but it doesn't matter).

Done.

> I wonder if the default shouldn't be "infinite delay", so as to preserve
> existing behavior?

Done. (If it is OK I like to turn it on for a month or two for some
testing and then turn it off eventually)

> I think the only valid reason to disable the help-char here is if it
> prevents access to an existing register.  So we can get rid of this
> custom var and instead make sure we only obey help-char if there is no
> existing register by that name.

The trouble is when you create new registers. But I remove that custom
variable anyway since it won't be used much. It is unlikely C-h be used
as a register.

If no one objects I would like to install the attached patch.

Thanks,
Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reg3.diff --]
[-- Type: text/x-patch, Size: 8635 bytes --]

=== modified file 'lisp/register.el'
--- lisp/register.el	2013-08-09 00:30:24 +0000
+++ lisp/register.el	2013-10-06 01:56:29 +0000
@@ -1,4 +1,4 @@
-;;; register.el --- register commands for Emacs
+;;; register.el --- register commands for Emacs      -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1985, 1993-1994, 2001-2013 Free Software Foundation,
 ;; Inc.
@@ -89,6 +89,11 @@
   :type '(choice (const :tag "None" nil)
 		 (character :tag "Use register" :value ?+)))
 
+(defcustom register-preview-delay nil
+  "If non-nil delay in seconds to pop up the preview window."
+  :type '(choice number (const :tag "Indefinitely" nil))
+  :group 'register)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (cdr (assq register register-alist)))
@@ -102,12 +107,66 @@
       (push (cons register value) register-alist))
     value))
 
+(defun register-describe-oneline (c)
+  "One-line description of register C."
+  (let ((d (replace-regexp-in-string
+            "\n[ \t]*" " "
+            (with-output-to-string (describe-register-1 c)))))
+    (if (string-match "Register.+? contains \\(?:an? \\|the \\)?" d)
+        (substring d (match-end 0))
+      d)))
+
+(defvar register-preview-functions nil)
+
+(defun register-preview (buffer)
+  "Pop up a window to show register preview in BUFFER."
+  (when (consp register-alist)
+    (let ((split-height-threshold 0))
+      (with-temp-buffer-window
+       buffer
+       (cons 'display-buffer-below-selected
+	     '((window-height . fit-window-to-buffer)))
+       nil
+       (with-current-buffer standard-output
+	 (setq cursor-in-non-selected-windows nil)
+	 (mapc
+	  (lambda (r)
+	    (insert (or (run-hook-with-args-until-success
+			 'register-preview-functions r)
+			(format "%s %s\n"
+				(concat (single-key-description (car r)) ":")
+				(register-describe-oneline (car r))))))
+	  register-alist))))))
+
+(defun register-read-with-preview (prompt)
+  "Read an event with register preview using PROMPT.
+Pop up a register preview window if the input is help-char but is
+not a register. Alternatively if `register-preview-delay' is a
+number the preview window is popped up after some delay."
+  (let* ((buffer "*Register Preview*")
+	 (timer (when (numberp register-preview-delay)
+		  (run-with-timer register-preview-delay nil
+				  #'register-preview buffer)))
+	 (help-char (and (not (get-register help-char)) help-char)))
+    (unwind-protect
+	(progn
+	  (while (eq (read-event (propertize prompt 'face 'minibuffer-prompt))
+		     help-char)
+	    (unless (get-buffer-window buffer)
+	      (register-preview buffer)))
+	  last-input-event)
+      (and (timerp timer) (cancel-timer timer))
+      (let ((w (get-buffer-window buffer)))
+        (and (window-live-p w) (delete-window w)))
+      (and (get-buffer buffer) (kill-buffer buffer)))))
+
 (defun point-to-register (register &optional arg)
   "Store current location of point in register REGISTER.
 With prefix argument, store current frame configuration.
 Use \\[jump-to-register] to go to that location or restore that configuration.
 Argument is a character, naming the register."
-  (interactive "cPoint to register: \nP")
+  (interactive (list (register-read-with-preview "Point to register: ")
+		     current-prefix-arg))
   ;; Turn the marker into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
   (set-register register
@@ -118,7 +177,9 @@
   "Store the window configuration of the selected frame in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cWindow configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Window configuration to register: ")
+		     current-prefix-arg))
   ;; current-window-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-window-configuration) (point-marker))))
@@ -127,7 +188,9 @@
   "Store the window configuration of all frames in register REGISTER.
 Use \\[jump-to-register] to restore the configuration.
 Argument is a character, naming the register."
-  (interactive "cFrame configuration to register: \nP")
+  (interactive (list (register-read-with-preview
+		      "Frame configuration to register: ")
+		     current-prefix-arg))
   ;; current-frame-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
   (set-register register (list (current-frame-configuration) (point-marker))))
@@ -143,7 +206,8 @@
 Optional second arg non-nil (interactively, prefix argument) says to
 delete any existing frames that the frameset doesn't mention.
 \(Otherwise, these frames are iconified.)"
-  (interactive "cJump to register: \nP")
+  (interactive (list (register-read-with-preview "Jump to register: ")
+		     current-prefix-arg))
   (let ((val (get-register register)))
     (cond
      ((registerv-p val)
@@ -190,7 +254,8 @@
 If NUMBER is nil, a decimal number is read from the buffer starting
 at point, and point moves to the end of that number.
 Interactively, NUMBER is the prefix arg (none means nil)."
-  (interactive "P\ncNumber to register: ")
+  (interactive (list current-prefix-arg
+		     (register-read-with-preview "Number to register: ")))
   (set-register register
 		(if number
 		    (prefix-numeric-value number)
@@ -222,7 +287,7 @@
 (defun view-register (register)
   "Display what is contained in register named REGISTER.
 The Lisp value REGISTER is a character."
-  (interactive "cView register: ")
+  (interactive (list (register-read-with-preview "View register: ")))
   (let ((val (get-register register)))
     (if (null val)
 	(message "Register %s is empty" (single-key-description register))
@@ -323,7 +388,10 @@
 Normally puts point before and mark after the inserted text.
 If optional second arg is non-nil, puts mark before and point after.
 Interactively, second arg is non-nil if prefix arg is supplied."
-  (interactive "*cInsert register: \nP")
+  (interactive (progn
+		 (barf-if-buffer-read-only)
+		 (register-read-with-preview "Insert register: ")
+		 current-prefix-arg))
   (push-mark)
   (let ((val (get-register register)))
     (cond
@@ -349,7 +417,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to copy."
-  (interactive "cCopy to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Copy to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (set-register register (filter-buffer-substring start end))
   (setq deactivate-mark t)
   (cond (delete-flag
@@ -362,7 +433,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to append."
-  (interactive "cAppend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Append to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -381,7 +455,10 @@
 With prefix arg, delete as well.
 Called from program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions indicating what to prepend."
-  (interactive "cPrepend to register: \nr\nP")
+  (interactive (list (register-read-with-preview "Prepend to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((reg (get-register register))
         (text (filter-buffer-substring start end))
 	(separator (and register-separator (get-register register-separator))))
@@ -402,7 +479,11 @@
 
 Called from a program, takes four args: REGISTER, START, END and DELETE-FLAG.
 START and END are buffer positions giving two corners of rectangle."
-  (interactive "cCopy rectangle to register: \nr\nP")
+  (interactive (list (register-read-with-preview
+		      "Copy rectangle to register: ")
+		     (region-beginning)
+		     (region-end)
+		     current-prefix-arg))
   (let ((rectangle (if delete-flag
 		       (delete-extract-rectangle start end)
 		     (extract-rectangle start end))))
@@ -412,6 +493,5 @@
       (setq deactivate-mark t)
       (indicate-copied-region (length (car rectangle))))))
 
-
 (provide 'register)
 ;;; register.el ends here

bug#15525: 24.3; register-read-with-preview

[Stefan Monnier]

>> I wonder if the default shouldn't be "infinite delay", so as to preserve
>> existing behavior?
> Done. (If it is OK I like to turn it on for a month or two for some
> testing and then turn it off eventually)

I'm OK with enabling it tentatively, and see if people complain.
You might like to use a slightly longer delay, tho.  For me, 0.5s makes
it pop up the list pretty much all the time (I guess I'm slow).

>> I think the only valid reason to disable the help-char here is if it
>> prevents access to an existing register.  So we can get rid of this
>> custom var and instead make sure we only obey help-char if there is no
>> existing register by that name.
> The trouble is when you create new registers.

The code should not let you create new registers that match help-char.

> But I remove that custom variable anyway since it won't be used
> much. It is unlikely C-h be used as a register.

BTW, we could/should also accept help-event-list events.

> If no one objects I would like to install the attached patch.

Go ahead,


        Stefan

bug#15525: 24.3; register-read-with-preview

[Leo Liu]

On 2013-10-07 00:59 +0800, Stefan Monnier wrote:
> I'm OK with enabling it tentatively, and see if people complain.
> You might like to use a slightly longer delay, tho.  For me, 0.5s makes
> it pop up the list pretty much all the time (I guess I'm slow).

I have set it to 1s for now and will set it to nil after 60 days.

[snipped 5 lines]
> The code should not let you create new registers that match help-char.
> BTW, we could/should also accept help-event-list events.

OK, though ? is in help-event-list.

> Go ahead,

Installed.

Leo

bug#15525: 24.3; register-read-with-preview

[Stefan Monnier]

>> I'm OK with enabling it tentatively, and see if people complain.
>> You might like to use a slightly longer delay, tho.  For me, 0.5s makes
>> it pop up the list pretty much all the time (I guess I'm slow).
> I have set it to 1s for now and will set it to nil after 60 days.

Not sure why you'd want to set it back to nil if noone complains.

>> The code should not let you create new registers that match help-char.
>> BTW, we could/should also accept help-event-list events.
> OK, though ? is in help-event-list.

I know, and I think it's an important trigger key since it's the one
used in completion (i.e. it's the one I'd try instinctively).


        Stefan

bug#15525: 24.3; register-read-with-preview

[Leo Liu]

Fixed in 24.4

On 2013-10-07 12:13 +0800, Stefan Monnier wrote:
> Not sure why you'd want to set it back to nil if noone complains.

OK, I'll DTRT.

>> OK, though ? is in help-event-list.
>
> I know, and I think it's an important trigger key since it's the one
> used in completion (i.e. it's the one I'd try instinctively).

OK.

Leo