From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add "into-register" functions for buffers and files. Date: Sun, 04 Feb 2024 08:57:08 +0200 Message-ID: <86jznk3eez.fsf@gnu.org> References: <87y1c1f6h7.fsf@ocathain.ie> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32193"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Barra =?utf-8?B?w5MgQ2F0aMOhaW4=?= , Thierry Volpiatto Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Feb 04 07:58:05 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rWWS5-0008AQ-Lr for ged-emacs-devel@m.gmane-mx.org; Sun, 04 Feb 2024 07:58:05 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rWWRN-0007aB-Nn; Sun, 04 Feb 2024 01:57:22 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rWWRG-0007Zv-Lu for emacs-devel@gnu.org; Sun, 04 Feb 2024 01:57:15 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rWWRE-0004Ir-IB; Sun, 04 Feb 2024 01:57:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=Jy9kW+rOHwphehIhfWuham6U48APNAAQkoWzo7eJrz0=; b=XKgsRs8jMUfvLYxJO/D2 t2VWPOSVwjQvMtRQlbi3a98Sqz4iwzVTWVOnitTR2HeJj4kgQnhlHCNqeRHb051Qz+HLnm9TAFqaH rMBfsPd6PFxz8khu00Y37CVOAF23k2hyqUG4B6sYUdesxJ04iHES3lOHGEwwBvhsTq40wolKcWO63 4pNIWqzuOEFM3hVo3J4dl1KFN/9JRbO4sGDVV6cuSN7IQqlT3kReFnJzwGskM+Qx9YhPV9ro6LSFp LRq3dEkJH2Ag/RJKMwz4wcOFU/QI+IoltPRjmzVrFhOWpzB/vNSXCUrhz/cZjMmvU95/fI0uXUQdn bjjz/EnSvLZUjw==; In-Reply-To: <87y1c1f6h7.fsf@ocathain.ie> (emacs-devel@gnu.org) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:315857 Archived-At: > Date: Sat, 03 Feb 2024 23:55:32 +0000 > From: Barra Ó Catháin via "Emacs development discussions." > > 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.