emacs-devel
[Top][All Lists]
Advanced

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

Re: Trimming strings, /emacs/lisp/emacs-lisp/subr-x.el modification


From: Jean-Christophe Helary
Subject: Re: Trimming strings, /emacs/lisp/emacs-lisp/subr-x.el modification
Date: Wed, 3 May 2017 07:48:07 +0900

> On May 3, 2017, at 2:16, Eli Zaretskii <address@hidden> wrote:
>> 2) I declared optional "trim-left" and "trim-right" arguments respectively 
>> for the string-trim-left and string-trim-right functions, and declared both 
>> optional for the general string-trim function
> 
> Do we really have use cases for string-trim where trim-left and
> trim-right should be different?  If not, let's give it only one
> optional argument, not 2.

I have no clear use case because of my lack of programming experience, but I 
can see how TRIM-LEFT could be used to remove the beginning of a path and 
TRIM-RIGHT could be used to remove a file extension, for ex.

>> +(defsubst string-trim-left (string &optional trim-left)
>> +  "Trim STRING of leading whitespace matching TRIM-LEFT.
> 
> Since TRIM-LEFT could be anything, the "whitespace" part is no longer
> accurate.  Suggest to use "delimiters" instead.

I overlooked that. In fact, since, as you correctly point out, a regex can be 
anything I think "characters" or "the string" is even better. It seems to me 
that "delimiter" fits better the concept of "splitting" than the concept of 
"trimming".

>> +If TRIM-LEFT is non-nil, it should be a regular expression. If nil,
>> +it defaults to `string-trim-default-regex', normally \"[ \t\n\r]+\"."
> 
> This is better worded like this:
> 
>  TRIM-LEFT should be a regular expression, and defaults to
>  `string-trim-default-regex' if omitted or nil.

Thank you. I tried to stick to the wording found in subr.el (split-string)... 
:-) I'll do as you suggest.

>> +(defsubst string-trim (string &optional trim-left trim-right)
>> +  "Trim STRING of leading and trailing whitespace matching TRIM-LEFT and 
>> TRIM-RIGHT.
> 
> Please make the first line shorter than 80 characters.

I overlooked that one. Sorry.

Jean-Christophe 


reply via email to

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