Re: [PATCH] lisp/files.el: Add `file-name-set-extension`

["Colin Woodbury" <colin@fosskers.ca>]

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

That regex was borrowed directly from `string-trim`, to which I added a `.`. You're right that the main motivation was to allow the caller to pass either `.foo` or `foo` as the extension and have either case work (many languages do this). Leaving the other characters in the regex seemed like a sanitary thing to do, just in case they pass something bogus. Although as the code is, there's nothing preventing them from still giving bogus characters before the file, or after the extension, making the resulting filepath still meaningless. 

So we could consider:

1. Simplifying the regex to just include dots (and perhaps whitespace) (i.e. trust the caller), or;
2. Expanding the logic to sanitize both ends of both arguments, (i.e. don't trust the caller), or;
3. Adding error-throwing logic if malformed files or extensions are given (i.e. warn the user).

I'm happy to alter the patch for any of those scenarios, whichever is preferable. Thanks!

On Tue, 25 May 2021, at 13:25, Stefan Monnier wrote:
> > +  (when (and filename extension)
> > +    (let* ((patt "[ \t\n\r.]+") ; Inspired by `string-trim'.
> > +           (filename (string-trim-right filename patt))
> > +           (extension (string-trim-left extension patt)))
> > +      (unless (or (string-empty-p filename)
> > +                  (string-empty-p extension))
> > +        (concat (file-name-sans-extension filename) "." extension)))))
> 
> Why do you trim [ \t\n\r.]+ from the end of the filename and the
> beginning of the extension?
> 
> I can see why you'd want to remove a single "." at the beginning of
> `extension`, so as to allow extension to come with or without a leading
> ".", but the rest seems rather surprising because such characters in my
> experience almost never show up in such a circumstance (which means
> that if they do, it's either on purpose and we should preserve it, or
> it's an error "upstream" and we should try and help expose the error
> rather than silently try to cover it).
> 
> 
>         Stefan
> 
> 

[-- Attachment #2: Type: text/html, Size: 2968 bytes --]