Re: master 4f1a5e4: Add `file-name-set-extension'

[Colin Woodbury <colin@fosskers.ca>]

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


Hi gentlemen, Lars brought up some excellent points, so I did a survey
of other languages to see what they expect/permit:

| Language | Forgives dot? | Scrutinizes ext? | Handles directories? |
|----------+---------------+------------------+----------------------|
| Rust     | No            | No               | No                   |
| Python   | "Yes"         | "Yes"            | No                   |
| Haskell  | Yes           | No               | No                   |

Rust allows everything, including setting the extension of a directory,
the result of which then returns `false` for `is_dir()`.

Python throws an exception if the passed extension doesn't begin with a
dot, but otherwise allows anything else in the filename or extension.

Haskell allows the "dot or not" trick that I had adopted in my patches,
but otherwise doesn't scrutinize the contents of the filename or extension.

I also looked at Golang, but it was just raw string manipulation with no
extra helper functions.

And given that my FS seems to accept spaces in both filenames and their
extensions without issue, I've landed on the following logic:

- DO allow the passing of an optionally dotted extension, like Haskell.
- DO only strip a single dot.
- DO check if the filename is empty or shaped like a directory name.
- DON'T otherwise care about the contents of the filename or extension.

I've attached a revised patch that accounts for these. And for Michael,
I made sure to add the texinfo docs and NEWS entry.

Cheers!
Colin


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: file-name-with-extension.patch --]
[-- Type: text/x-patch, Size: 4154 bytes --]

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 2033177fbb..8096c9e861 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2129,6 +2129,24 @@ the period that delimits the extension, and if @var{filename} has no
 extension, the value is @code{""}.
 @end defun
 
+@defun file-name-with-extension filename extension
+This function returns @var{filename} with its extension set to
+@var{extension}. A single leading dot in the @var{extension} will be
+stripped if there is one. For exmaple,
+
+@example
+(file-name-with-extension "file" "el")
+     @result{} "file.el"
+(file-name-with-extension "file" ".el")
+     @result{} "file.el"
+(file-name-with-extension "file.c" "el")
+     @result{} "file.el"
+@end example
+
+Note that this function will error if the @var{filename} or
+@var{extension} are empty, or if the @var{filename} is shaped like a
+directory (i.e. if @code{directory-name-p} returns @code{t}).
+
 @defun file-name-sans-extension filename
 This function returns @var{filename} minus its extension, if any.  The
 version/backup part, if present, is only removed if the file has an
diff --git a/etc/NEWS b/etc/NEWS
index 60226f0a3e..9838693a65 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2964,6 +2964,11 @@ been added, and takes a callback to handle the return status.
 ---
 ** 'ascii' is now a coding system alias for 'us-ascii'.
 
++++
+** New function 'file-name-with-extension'.
+This function allows a canonical way to set/replace the extension of a
+filename string.
+
 +++
 ** New function 'file-backup-file-names'.
 This function returns the list of file names of all the backup files
diff --git a/lisp/files.el b/lisp/files.el
index 2450daf5bf..a5ac1821b2 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4892,6 +4892,22 @@ extension, the value is \"\"."
         (if period
             "")))))
 
+(defun file-name-with-extension (filename extension)
+  "Set the EXTENSION of a FILENAME.
+
+Trims a leading dot from the EXTENSION so that either `foo' or
+`.foo' can be given.
+
+Errors if the filename or extension are empty, or if the given
+filename has the format of a directory.
+
+See also `file-name-sans-extension'."
+  (let ((extn (string-trim-left extension "[.]")))
+    (cond ((string-empty-p filename) (error "Empty filename: %s" filename))
+          ((string-empty-p extn) (error "Malformed extension: %s" extension))
+          ((directory-name-p filename) (error "Filename is a directory: %s" filename))
+          (t (concat (file-name-sans-extension filename) "." extn)))))
+
 (defun file-name-base (&optional filename)
   "Return the base name of the FILENAME: no directory, no extension."
   (declare (advertised-calling-convention (filename) "27.1"))
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index dc96dff639..257cbc2d32 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1478,5 +1478,23 @@ The door of all subtleties!
                                (buffer-substring (point-min) (point-max))
                                nil nil)))))
 
+(ert-deftest files-tests-file-name-with-extension-good ()
+  "Test that `file-name-with-extension' succeeds with reasonable input."
+  (should (string= (file-name-with-extension "Jack" "css") "Jack.css"))
+  (should (string= (file-name-with-extension "Jack" ".css") "Jack.css"))
+  (should (string= (file-name-with-extension "Jack.scss" "css") "Jack.css"))
+  (should (string= (file-name-with-extension "/path/to/Jack.md" "org") "/path/to/Jack.org")))
+
+(ert-deftest files-tests-file-name-with-extension-bad ()
+  "Test that `file-name-with-extension' fails on malformed input."
+  (should-error (file-name-with-extension nil nil))
+  (should-error (file-name-with-extension "Jack" nil))
+  (should-error (file-name-with-extension nil "css"))
+  (should-error (file-name-with-extension "" ""))
+  (should-error (file-name-with-extension "" "css"))
+  (should-error (file-name-with-extension "Jack" ""))
+  (should-error (file-name-with-extension "Jack" "."))
+  (should-error (file-name-with-extension "/is/a/directory/" "css")))
+
 (provide 'files-tests)
 ;;; files-tests.el ends here