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

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

[Lars Ingebrigtsen]

Michael.Albinus@gmx.de (Michael Albinus) writes:

>     Add `file-name-set-extension'
>
>     * lisp/files.el (file-name-with-extension): New defun.

[...]

> +(defun file-name-with-extension (filename extension)
> +  "Set the EXTENSION of a FILENAME.
> +Consolidates leading/trailing dots so that either `foo' or `.foo'
> +can be passed as an EXTENSION.
> +
> +See also `file-name-sans-extension'."
> +  (let* ((patt "[ .]+")
> +         (file (string-trim-right filename patt))
> +         (extn (string-trim-left extension patt)))
> +    (cond ((string-empty-p file) (error "Malformed filename: %s" filename))
> +          ((string-empty-p extn) (error "Malformed extension: %s" extension))
> +          ((directory-name-p file) (error "Filename is a directory: %s" filename))
> +          (t (concat (file-name-sans-extension file) "." extn)))))

Is this really a well-defined function?  It strips spaces from the
start of the extension?

(file-name-with-extension "foo.bar" "  zot ")
=> "foo.zot "

That seems pretty peculiar and is not documented.  And also:

(file-name-with-extension "foo.bar.." "  zot ")
=> "foo.zot "

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> +(defun file-name-with-extension (filename extension)
>> +  "Set the EXTENSION of a FILENAME.
>> +Consolidates leading/trailing dots so that either `foo' or `.foo'
>> +can be passed as an EXTENSION.
>> +
>> +See also `file-name-sans-extension'."
>> +  (let* ((patt "[ .]+")
>> +         (file (string-trim-right filename patt))
>> +         (extn (string-trim-left extension patt)))
>> +    (cond ((string-empty-p file) (error "Malformed filename: %s" filename))
>> +          ((string-empty-p extn) (error "Malformed extension: %s" extension))
>> + ((directory-name-p file) (error "Filename is a directory: %s"
>> filename))
>> +          (t (concat (file-name-sans-extension file) "." extn)))))
>
> Is this really a well-defined function?  It strips spaces from the
> start of the extension?
>
> (file-name-with-extension "foo.bar" "  zot ")
> => "foo.zot "
>
> That seems pretty peculiar and is not documented.  And also:
>
> (file-name-with-extension "foo.bar.." "  zot ")
> => "foo.zot "

This was discussed on emacs-devel, see thread starting with
<https://lists.gnu.org/archive/html/emacs-devel/2021-05/msg01162.html>.

Anyway, I've made an error in commit, failing to use Colin as author. So
I have reverted the patch; when this discussion here has finished I'm
ready to reapply this (or a modified version of) this patch.

Best regards, Michael.

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

[Colin Woodbury]

[-- 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

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

[Lars Ingebrigtsen]

Colin Woodbury <colin@fosskers.ca> writes:

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

Great!  I've now applied it to Emacs 28 -- it's a helper function that
I've often wished had existed when wrangling with file conversions.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no