Re: [PATCH] Add "into-register" functions for buffers and files.

[Eli Zaretskii]

> Date: Sat, 03 Feb 2024 23:55:32 +0000
> From:  Barra Ó Catháin via "Emacs development discussions." 
> <emacs-devel@gnu.org>
> 
> I noticed that files and buffers don't have "to-register" functions of
> their own, rather, relying on (set-register). To that end, I have
> written 4 functions that I feel may be useful and "complete the set" of
> register functions, and some minor tweaks to the relevant manual
> section. 

Thanks.

Thierry, any comments?

I have some comments below.

> Subject: [PATCH 1/4] Added buffer and file to register functions.
> 
> Added buffer-to-register, file-to-register, current-buffer-to-register,
> and current-file-to-register to register.el.

Please accompany the changes with a ChangeLog-style commit log message,
according to our conventions.  You can see a lot of examples of that
in "git log"s output, and the file CONTRIBUTE in the top-level
directory of the Emacs source tree describes the conventions.

>  lisp/register.el | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)

This contribution is large enough to require copyright assignment
paperwork.  Would you like to start the paperwork now?  If yes, I will
send you the form to fill and the instructions to go with the form.

> +(defun file-to-register (register file-name)
> +  "Inserts a given file into a register.  To visit the file, use
> +  \\[jump-to-register].

The first line of a doc string should be a single complete sentence,
and it should mention the function's arguments.  This rule is because
the various apropos commands in Emacs show only the first line of the
doc string.

This comment is relevant to all the rest of the functions you added.

> +  (set-register register `(file . ,file-name)))

Is it necessary to use backticks here?  Would the below also work?

   (set-register register (cons 'file file-name))

This is simpler and easier to read and understand, since backticks are
considered "advanced ELisp", and not every Lisp programmer is
proficient with their usage.

Same comment to all the other functions.

> +(defun current-file-to-register (register)
> +  "Places the current file name into a register.  To visit the file, use
> +\\[jump-to-register].
> +
> +Called from Lisp, takes one arg: REGISTER.
> +
> +Interactively, prompt for REGISTER using `register-read-with-preview."
> +  (interactive (list (register-read-with-preview "Current file to register: 
> ")))                
> +  (set-register register `(file . ,(buffer-file-name))))

Instead of having 2 separate commands for placing a file into a
register, how about having a single command, which would by default
put into a register the current buffer, and will prompt for a file if
invoked with a prefix argument?

Same question about putting buffers into registers.

> --- a/lisp/register.el
> +++ b/lisp/register.el
> @@ -695,7 +695,9 @@ Interactively, prompt for REGISTER using 
> `register-read-with-preview',
>  and prompt for FILE-NAME using `read-file-name'."
>    (interactive (list (register-read-with-preview "File to register: ")
>                       (read-file-name "File: ")))
> -  (set-register register `(file . ,file-name)))
> +  (if (file-exists-p file-name)
> +      (set-register register `(file . ,file-name)))
> +  (user-error "File does not exist.")

Why this limitation?  What's wrong with having a non-existing file in
a register?  Even if the file does exist when put into a register, it
could cease to exist later, so why do we need this restriction?

>  prompt for BUFFER-NAME using `read-buffer'."
>    (interactive (list (register-read-with-preview "Buffer to register: ")
>                       (read-buffer "Buffer: ")))
> -  (set-register register `(buffer . ,buffer)))
> +  (if (buffer-p buffer)
> +      (set-register register `(buffer . ,buffer))
> +    (user-error "Not a buffer."))

You could instead prevent the possibility of non-existing buffers in
the call to read-buffer, no?

> --- a/doc/emacs/regs.texi
> +++ b/doc/emacs/regs.texi
> @@ -291,11 +291,13 @@ numeric argument stores zero in the register.
>  @cindex saving buffer name in a register
>  
>    If you visit certain file names frequently, you can visit them more
> -conveniently if you put their names in registers.  Here's the Lisp code
> -used to put a file @var{name} into register @var{r}:
> +conveniently if you put their names in registers.  You may use @kbd{C-x
> +r F} to place the currently visited file in a register. 
> +
> +Here's the Lisp code used to put a file @var{name} into register @var{r}:

First, if you look at the previous sections of the manual, you will
see that we have a convention of showing the commands with a short
description at the beginning of the section, followed by a longer and
more detailed description; please use the same format here.  If we
keep the restrictions of existing files/buffers, that should be
mentioned in the detailed description.

And second, since we are adding user commands, the Lisp code part
seems unnecessary in a user manual, so I think it should be removed.

Thanks.