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.
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!
> + (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