emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philipp
Subject: Re: [PATCH] lisp/files.el: Add `file-name-set-extension`
Date: Fri, 4 Jun 2021 15:08:05 +0200


> Am 31.05.2021 um 03:16 schrieb Colin Woodbury <colin@fosskers.ca>:
> 
> Hi Andreas (et al.),
> 
> After asking around, it seems like letting `string-trim` fail naturally is 
> indeed the canonical thing to do here, especially since with 
> `toggle-debug-on-error` we're able to get the full call-stack if we need it. 
> I've altered the patch (see attached) accordingly. Thanks for your patience.
> 
> Cheers,
> Colin
> 
> On Wed, 26 May 2021, at 10:39, Andreas Schwab wrote:
>> That's exactly what should happen.
>> 
>> Andreas.
>> 
>> -- 
>> Andreas Schwab, schwab@linux-m68k.org
>> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
>> "And now for something completely different."
>> 
> 
> <file-name-set-extension.patch>


Some general comments:

- Please add unit tests, especially testing corner cases such as empty strings, 
directory names, filenames starting with a dot, etc.
- I'd rename the function to something that doesn't contain "set".  "set" gives 
the impression that the function modifies the filename in place, which it 
doesn't do.  Likewise, change the docstring to clarify that a new string is 
returned.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]