[Top][All Lists]

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

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

From: Tino Calancha
Subject: Re: Trimming strings, /emacs/lisp/emacs-lisp/subr-x.el modification
Date: Sun, 7 May 2017 11:25:41 +0900 (JST)
User-agent: Alpine 2.20 (DEB 67 2015-01-07)

On Sat, 6 May 2017, Eli Zaretskii wrote:

From: Tino Calancha <address@hidden>
Cc: Eli Zaretskii <address@hidden>,  address@hidden,  address@hidden
Date: Sat, 06 May 2017 22:51:26 +0900

Add optional regexp for subr-x.el trimming functions
I would say something like:
Allow user regexp in string trimming functions.

FWIW, I don't see much difference between these two.  And the option
is not really for "users", it's for Lisp programs, right?
The latter try to summary the issue, that is 'what'. The former sounds more like the implementation to fix the issue, i.e., 'how'.

+(defsubst string-trim-left (string &optional regexp)
+  "Trim STRING of leading string matching REGEXP.
+REGEXP defaults to \"[ \\t\\n\\r]+\"."
We don't need a empty line inside such short docstrings.

A matter of personal style, IMO.
Emacs might have a prefered style for this, and promote it.

I feel like too much 'trim', 'left' and 'right' around.  It's distracting.
I suggest something like:
(defsubst string-trim (string &optional regexp-beg regexp-end)
(defsubst string-trim (string &optional regexp-l regexp-r)

I wouldn't make comments like this so late in the review process.
It's just a rename of local variables.  It's really a minor issue.

I find it more legible written as:
 (string-trim-right string regexp-end)

than as:
(string-trim-left (string-trim-right string regexp-end) regexp-beg)

Again, personal style issue, IMO.
Just wondering what is your personal style here.
Emacs might have a prefered style for this as well.  It's nicer to read
uniform style sources.

reply via email to

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