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

[Tino Calancha <tino.calancha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3376 bytes --]



On Sat, 6 May 2017, Jean-Christophe Helary wrote:

>>> Basically everything in subr-x is either a macro or a defsubst,
>>>
>>> As such, most uses of subr-x are done with (eval-when-compile (require
>>> 'subr-x)).  Won't the use of a global variable break these?
>>
>> (defconst string-trim-default-regex "[ \t\n\r]+")
>>  "The default value of the trimmed string for `string-trim'."
>>
>> I have no idea what the effects would be. What would you suggest?

Hi Jean-Christophe,

Mark is right.  Your patch won't work in the following example:

Write a file '/tmp/test.el'
--8<-----------------------------cut here---------------start------------->8---
(eval-when-compile (require 'subr-x))
(defun test ()
   (message
    (string-trim "  Hi!  ")))
--8<-----------------------------cut here---------------end--------------->8---
M-! emacs -batch -eval '(byte-compile-file "/tmp/test.el")' && \
     emacs -batch -l /tmp/test.elc -eval "(test)" RET

;; Signals error: Symbol’s value as variable is void: string-trim-default-regex


Instead, something as follows would work:
--8<-----------------------------cut here---------------start------------->8---
commit dc4d7fa64c5cf4f3c3fa04182da974f2d0bc6499
Author: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
Date:   Sat May 6 13:24:03 2017 +0900

     Allow trim strings of whitespaces with a general regexp

     * lisp/emacs-lisp/subr-x.el (string-trim-left, string-trim-right):
     Add optional arg REGEXP.
     (string-trim): Add optional args TRIM-LEFT and TRIM-RIGHT.

diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 440213eb38..5148e8b724 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -179,21 +179,29 @@ string-join

  (define-obsolete-function-alias 'string-reverse 'reverse "25.1")

-(defsubst string-trim-left (string)
-  "Remove leading whitespace from STRING."
-  (if (string-match "\\`[ \t\n\r]+" string)
+(defsubst string-trim-left (string &optional regexp)
+  "Trim STRING of leading whitespace matching REGEXP.
+
+REGEXP defaults to \"[ \t\n\r]+\"."
+  (if (string-match (concat "\\`" (or regexp "[ \t\n\r]+")) string)
        (replace-match "" t t string)
      string))

-(defsubst string-trim-right (string)
-  "Remove trailing whitespace from STRING."
-  (if (string-match "[ \t\n\r]+\\'" string)
+(defsubst string-trim-right (string &optional regexp)
+  "Trim STRING of trailing whitespace matching REGEXP.
+
+REGEXP defaults to \"[ \t\n\r]+\"."
+  (if (string-match (concat (or regexp "[ \t\n\r]+") "\\'") string)
        (replace-match "" t t string)
      string))

-(defsubst string-trim (string)
-  "Remove leading and trailing whitespace from STRING."
-  (string-trim-left (string-trim-right string)))
+(defsubst string-trim (string &optional trim-left trim-right)
+  "Trim STRING of leading and trailing whitespace matching TRIM-LEFT and TRIM-RIGHT.
+
+Both, TRIM-RIGHT and TRIM-LEFT default to \"[ \t\n\r]+\"."
+  (string-trim-left
+   (string-trim-right string trim-right)
+   trim-left))

  (defsubst string-blank-p (string)
    "Check whether STRING is either empty or only whitespace."
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
  of 2017-05-05
Repository revision: 927dcbd2e6e0e53fcfb09296716e11c002ab1518


Cheers,
Tino