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.