bug#64210: Allow ff-other-file-alist to specify which file to create

bug#64210: Allow ff-other-file-alist to specify which file to create

[Aaron Zeng]

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

Tags: patch

Tags: patch


This patch adds an optional third element to the lists that make
ff-other-file-alist.  If this third element is present, it is used as
the extension for a new file, when none of the other options were
found.  Previously, this was forced to be the first extension in the
list, which unnecessarily coupled search order and file-to-create.

The specific use case I have in mind is for the following scenario in
OCaml source files: it is common to have the following three files for
a module "Foo": foo.ml, foo.mli, and foo_intf.ml.  I would like to
make ff-find-other-file cycle between these files if all of them
exist, or to create a new file if they do not.  However, it is also
normal for only foo.ml and foo.mli to exist, and foo_intf.ml should
not be created if it is not necessary.

At the moment, if the user is looking at foo.mli, there is no way to
program ff-other-file-alist to say, "go to foo_intf.ml if it exists,
otherwise foo.ml, but if neither exist, create foo.ml".  This patch
makes it possible to specify what file to create, if it is not the
first file searched for.

The manual has not yet been updated for this change, since I wanted to
get some feedback on the patch itself first.  Happy to make the
changes in the appropriate places if the patch looks reasonable.

In GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.12, Xaw scroll bars)
 of 2023-02-09 built on igm-qws-u12685a
Repository revision: 739b5d0e52d83ec567bd61a5a49ac0e93e0eb469
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: CentOS Linux 7 (Core)

Configured using:
 'configure --with-x-toolkit=lucid --without-gpm --without-gconf
 --without-selinux --without-imagemagick --with-modules --with-gif=no
 --with-cairo --with-rsvg
 --prefix=/j/office/app/emacs/builds/28.2-20230209-135825'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-ff-other-file-alist-functions-to-be-lambdas.patch --]
[-- Type: text/patch, Size: 1782 bytes --]

From 6aba92d369183035cd86cc56bea5104dfc1816d4 Mon Sep 17 00:00:00 2001
From: "Aaron L. Zeng" <azeng@janestreet.com>
Date: Mon, 12 Jun 2023 18:43:38 -0400
Subject: [PATCH 1/2] Allow ff-other-file-alist functions to be lambdas

* lisp/find-file.el (ff-other-file-alist, ff-find-the-other-file):
allow functions in the alist to be lambdas, by just checking
`functionp'.  Also make a minor docstring correction to
`ff-other-file-alist'.
---
 lisp/find-file.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/find-file.el b/lisp/find-file.el
index 05459c3643d..e4c86ecdfee 100644
--- a/lisp/find-file.el
+++ b/lisp/find-file.el
@@ -215,10 +215,10 @@ matching extension or name (e.g., `.cc' yields `.hh').
 
 This alist should be set by the major mode.
 
-Note: if an element of the alist names a FUNCTION as its cdr, that
-function must return a non-nil list of file-names.  It cannot
-return nil, nor can it signal in any way a failure to find a suitable
-list of file names."
+Note: if an element of the alist names a FUNCTION as its cadr,
+that function must return a non-nil list of file-names.  It
+cannot return nil, nor can it signal in any way a failure to find
+a suitable list of file names."
   :type '(choice (repeat (list regexp (choice (repeat string) function)))
 		 symbol))
 
@@ -469,7 +469,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
 
         ;; if we have a function to generate new names,
         ;; invoke it with the name of the current file
-        (if (and (atom action) (fboundp action))
+        (if (functionp action)
             (setq suffixes (funcall action (ff-buffer-file-name))
                   match (cons (car match) (list suffixes))
                   stub nil
-- 
2.39.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Allow-ff-other-file-alist-to-specify-the-extension-f.patch --]
[-- Type: text/x-patch, Size: 4569 bytes --]

From 13d1881e03ea838c5448da6b57678c7ba4c48704 Mon Sep 17 00:00:00 2001
From: "Aaron L. Zeng" <azeng@janestreet.com>
Date: Tue, 13 Jun 2023 17:07:40 -0400
Subject: [PATCH 2/2] Allow ff-other-file-alist to specify the extension for a
 new file

* lisp/find-file.el (ff-other-file-alist, ff-find-the-other-file): Add
an optional third element to the lists that make ff-other-file-alist.
If this third element is present, it is used as the extension for a
new file, when none of the other options were found.  Previously, this
was forced to be the first extension in the list, which unnecessarily
coupled search order and file-to-create.
---
 lisp/find-file.el | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/lisp/find-file.el b/lisp/find-file.el
index e4c86ecdfee..807f46e637c 100644
--- a/lisp/find-file.el
+++ b/lisp/find-file.el
@@ -192,7 +192,7 @@ filename that EXTRACT returned."
 The value could be an alist or a symbol whose value is an alist.
 Each element of the alist has the form
 
-   (REGEXP (EXTENSION...))
+   (REGEXP (EXTENSION...) [CREATE-EXTENSION])
 
 where REGEXP is the regular expression matching a file's extension,
 and EXTENSIONs is the list of literal file-name extensions to search
@@ -202,7 +202,7 @@ through each directory specified in `ff-search-directories'.
 
 Alist elements can also be of the form
 
-   (REGEXP FUNCTION)
+   (REGEXP FUNCTION [CREATE-EXTENSION])
 
 where FUNCTION is a function of one argument, the current file's name,
 that returns the list of possible names of the corresponding files, with
@@ -210,8 +210,11 @@ or without leading directories.  Note the difference: FUNCTION returns
 the list of file names, not their extensions.  This is for the case when
 REGEXP is not enough to determine the file name of the other file.
 
-If a file is not found, a new one is created with the first
-matching extension or name (e.g., `.cc' yields `.hh').
+If a file is not found, a new one is created with the first matching
+extension or name (e.g., `.cc' yields `.hh').  If [CREATE-EXTENSION]
+is present, it is used instead of the first extension.  If
+[CREATE-EXTENSION] is a function, it is called with a single argument,
+the current file's name, and the name it returns is used instead.
 
 This alist should be set by the major mode.
 
@@ -219,7 +222,13 @@ Note: if an element of the alist names a FUNCTION as its cadr,
 that function must return a non-nil list of file-names.  It
 cannot return nil, nor can it signal in any way a failure to find
 a suitable list of file names."
-  :type '(choice (repeat (list regexp (choice (repeat string) function)))
+  :type '(choice (repeat (list regexp
+                               (choice :tag "Extensions to try"
+                                       (repeat string)
+                                       function)
+                               (choice :tag "Extension to create"
+                                       (list :inline t :tag "string" string)
+                                       (list :inline t :tag "function" function))))
 		 symbol))
 
 (defcustom ff-search-directories 'cc-search-directories
@@ -465,7 +474,11 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
         ;; otherwise, suffixes contains what we need
         (setq suffixes (car (cdr match))
               action (car (cdr match))
-              found nil)
+              found nil
+              default-name (caddr match))
+
+        (when (functionp default-name)
+          (setq default-name (funcall default-name (ff-buffer-file-name))))
 
         ;; if we have a function to generate new names,
         ;; invoke it with the name of the current file
@@ -473,7 +486,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
             (setq suffixes (funcall action (ff-buffer-file-name))
                   match (cons (car match) (list suffixes))
                   stub nil
-                  default-name (car suffixes))
+                  default-name (or default-name (car suffixes)))
 
           ;; otherwise build our filename stub
           (cond
@@ -493,7 +506,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
 
           ;; if we find nothing, we should try to get a file like this one
           (setq default-name
-                (concat stub (car (car (cdr match))))))
+                (concat stub (or default-name (car (car (cdr match)))))))
 
         ;; do the real work - find the file
         (setq found
-- 
2.39.3

bug#64210: Allow ff-other-file-alist to specify which file to create

[Stefan Kangas]

Aaron Zeng <azeng@janestreet.com> writes:

> This patch adds an optional third element to the lists that make
> ff-other-file-alist.  If this third element is present, it is used as
> the extension for a new file, when none of the other options were
> found.  Previously, this was forced to be the first extension in the
> list, which unnecessarily coupled search order and file-to-create.
>
> The specific use case I have in mind is for the following scenario in
> OCaml source files: it is common to have the following three files for
> a module "Foo": foo.ml, foo.mli, and foo_intf.ml.  I would like to
> make ff-find-other-file cycle between these files if all of them
> exist, or to create a new file if they do not.  However, it is also
> normal for only foo.ml and foo.mli to exist, and foo_intf.ml should
> not be created if it is not necessary.
>
> At the moment, if the user is looking at foo.mli, there is no way to
> program ff-other-file-alist to say, "go to foo_intf.ml if it exists,
> otherwise foo.ml, but if neither exist, create foo.ml".  This patch
> makes it possible to specify what file to create, if it is not the
> first file searched for.

Makes sense to me.  Please find my review below, which is basically only
about minor, stylistic things.

> The manual has not yet been updated for this change, since I wanted to
> get some feedback on the patch itself first.  Happy to make the
> changes in the appropriate places if the patch looks reasonable.

Yes, the manual needs updating, and this should be called out in NEWS,
too.

>>From 6aba92d369183035cd86cc56bea5104dfc1816d4 Mon Sep 17 00:00:00 2001
> From: "Aaron L. Zeng" <azeng@janestreet.com>
> Date: Mon, 12 Jun 2023 18:43:38 -0400
> Subject: [PATCH 1/2] Allow ff-other-file-alist functions to be lambdas
>
> * lisp/find-file.el (ff-other-file-alist, ff-find-the-other-file):
> allow functions in the alist to be lambdas, by just checking
> `functionp'.  Also make a minor docstring correction to
> `ff-other-file-alist'.

Please include the bug number here.  (Bug#NNNNN)

> ---
>  lisp/find-file.el | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/find-file.el b/lisp/find-file.el
> index 05459c3643d..e4c86ecdfee 100644
> --- a/lisp/find-file.el
> +++ b/lisp/find-file.el
> @@ -215,10 +215,10 @@ matching extension or name (e.g., `.cc' yields `.hh').
>
>  This alist should be set by the major mode.
>
> -Note: if an element of the alist names a FUNCTION as its cdr, that
> -function must return a non-nil list of file-names.  It cannot
> -return nil, nor can it signal in any way a failure to find a suitable
> -list of file names."
> +Note: if an element of the alist names a FUNCTION as its cadr,
> +that function must return a non-nil list of file-names.  It
> +cannot return nil, nor can it signal in any way a failure to find
> +a suitable list of file names."
>    :type '(choice (repeat (list regexp (choice (repeat string) function)))
>  		 symbol))
>
> @@ -469,7 +469,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
>
>          ;; if we have a function to generate new names,
>          ;; invoke it with the name of the current file
> -        (if (and (atom action) (fboundp action))
> +        (if (functionp action)
>              (setq suffixes (funcall action (ff-buffer-file-name))
>                    match (cons (car match) (list suffixes))
>                    stub nil
> --
> 2.39.3
>
>
>>From 13d1881e03ea838c5448da6b57678c7ba4c48704 Mon Sep 17 00:00:00 2001
> From: "Aaron L. Zeng" <azeng@janestreet.com>
> Date: Tue, 13 Jun 2023 17:07:40 -0400
> Subject: [PATCH 2/2] Allow ff-other-file-alist to specify the extension for a
>  new file
>
> * lisp/find-file.el (ff-other-file-alist, ff-find-the-other-file): Add
> an optional third element to the lists that make ff-other-file-alist.
> If this third element is present, it is used as the extension for a
> new file, when none of the other options were found.  Previously, this
> was forced to be the first extension in the list, which unnecessarily
> coupled search order and file-to-create.

Please include the bug number here.

> ---
>  lisp/find-file.el | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/lisp/find-file.el b/lisp/find-file.el
> index e4c86ecdfee..807f46e637c 100644
> --- a/lisp/find-file.el
> +++ b/lisp/find-file.el
> @@ -192,7 +192,7 @@ filename that EXTRACT returned."
>  The value could be an alist or a symbol whose value is an alist.
>  Each element of the alist has the form
>
> -   (REGEXP (EXTENSION...))
> +   (REGEXP (EXTENSION...) [CREATE-EXTENSION])
>
>  where REGEXP is the regular expression matching a file's extension,
>  and EXTENSIONs is the list of literal file-name extensions to search
> @@ -202,7 +202,7 @@ through each directory specified in `ff-search-directories'.
>
>  Alist elements can also be of the form
>
> -   (REGEXP FUNCTION)
> +   (REGEXP FUNCTION [CREATE-EXTENSION])
>
>  where FUNCTION is a function of one argument, the current file's name,
>  that returns the list of possible names of the corresponding files, with
> @@ -210,8 +210,11 @@ or without leading directories.  Note the difference: FUNCTION returns
>  the list of file names, not their extensions.  This is for the case when
>  REGEXP is not enough to determine the file name of the other file.
>
> -If a file is not found, a new one is created with the first
> -matching extension or name (e.g., `.cc' yields `.hh').
> +If a file is not found, a new one is created with the first matching
> +extension or name (e.g., `.cc' yields `.hh').  If [CREATE-EXTENSION]
> +is present, it is used instead of the first extension.  If

This should read:

    If CREATE-EXTENSION is non-nil, ...

> +[CREATE-EXTENSION] is a function, it is called with a single argument,
> +the current file's name, and the name it returns is used instead.

CREATE-EXTENSION should not have brackets here.  See for example
`font-lock-defaults' for how we do this.

>  This alist should be set by the major mode.
>
> @@ -219,7 +222,13 @@ Note: if an element of the alist names a FUNCTION as its cadr,
>  that function must return a non-nil list of file-names.  It
>  cannot return nil, nor can it signal in any way a failure to find
>  a suitable list of file names."
> -  :type '(choice (repeat (list regexp (choice (repeat string) function)))
> +  :type '(choice (repeat (list regexp
> +                               (choice :tag "Extensions to try"
> +                                       (repeat string)
> +                                       function)
> +                               (choice :tag "Extension to create"
> +                                       (list :inline t :tag "string" string)
> +                                       (list :inline t :tag "function" function))))
>  		 symbol))
>
>  (defcustom ff-search-directories 'cc-search-directories
> @@ -465,7 +474,11 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
>          ;; otherwise, suffixes contains what we need
>          (setq suffixes (car (cdr match))
>                action (car (cdr match))
> -              found nil)
> +              found nil
> +              default-name (caddr match))
> +
> +        (when (functionp default-name)
> +          (setq default-name (funcall default-name (ff-buffer-file-name))))
>
>          ;; if we have a function to generate new names,
>          ;; invoke it with the name of the current file
> @@ -473,7 +486,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
>              (setq suffixes (funcall action (ff-buffer-file-name))
>                    match (cons (car match) (list suffixes))
>                    stub nil
> -                  default-name (car suffixes))
> +                  default-name (or default-name (car suffixes)))
>
>            ;; otherwise build our filename stub
>            (cond
> @@ -493,7 +506,7 @@ If optional IN-OTHER-WINDOW is non-nil, find the file in another window."
>
>            ;; if we find nothing, we should try to get a file like this one
>            (setq default-name
> -                (concat stub (car (car (cdr match))))))
> +                (concat stub (or default-name (car (car (cdr match)))))))
>
>          ;; do the real work - find the file
>          (setq found

Not sure how hard it would be to add tests for this, so I'll leave that
up to you.