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

[Eli Zaretskii]

> From: Jean-Christophe Helary <address@hidden>
> Date: Tue, 2 May 2017 18:34:17 +0900
> 
> Following the discussion about how trimming in subr-x.el did not allow for 
> non-default regex, I have this small modification to the 3 trimming functions 
> found therein.
> 
> Basically:
> 1) I extracted the default regex and assigned it to string-trim-default-regex
> 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

Thanks.

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.

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

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

> +(defsubst string-trim-right (string &optional trim-right)
> +  "Trim STRING of trailing whitespace matching TRIM-RIGHT.
> +
> +If TRIM-RIGHT is non-nil, it should be a regular expression. If nil,
> +it defaults to `string-trim-default-regex', normally \"[ \t\n\r]+\"."

Likewise.

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