Re: master 87e422f: Beef up the Emacs string utility set a bit

["Mattias Engdegård" <mattiase@acm.org>]

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

Thanks for the good work! I suggest that 'string-limit' be replaced with two separate functions, 'string-prefix' and 'string-suffix', because these are two different operations and it makes no sense to multiplex them onto a single function with an argument saying which one we are going to use.

Suggested patch attached. The names can be discussed; for example, 'string-left' and 'string-right' would do just as well.

If someone prefers saturating semantics for negative length arguments, then that's an option too. Ie, in case we want

 (string-prefix "abc" -2)

to yield the empty string instead of an error. I think an error is more useful.

The new functions are now inline (defsubst) in keeping with the spirit of subr-x.el. (We really should inline these better!)


[-- Attachment #2: 0001-Replace-string-limit-with-string-prefix-and-string-s.patch --]
[-- Type: application/octet-stream, Size: 6399 bytes --]

From 799aa76f2aa2fbbb2d1264a9bfdb6208edfbff65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 22 Dec 2020 11:23:39 +0100
Subject: [PATCH] Replace string-limit with string-prefix and string-suffix

Have separate functions for distinct operations instead of
multiplexing them both onto a single function.

* doc/lispref/strings.texi (Creating Strings):
* etc/NEWS (https):
* lisp/emacs-lisp/shortdoc.el (string):
Update documentation and announcement.
* lisp/emacs-lisp/subr-x.el (string-limit): Remove.
(string-prefix, string-suffix): New.
* test/lisp/emacs-lisp/subr-x-tests.el (subr-string-limit): Remove.
(subr-string-prefix, subr-string-suffix): New tests.
---
 doc/lispref/strings.texi             | 12 ++++++++----
 etc/NEWS                             |  2 +-
 lisp/emacs-lisp/shortdoc.el          | 10 ++++++----
 lisp/emacs-lisp/subr-x.el            | 29 ++++++++++++++++------------
 test/lisp/emacs-lisp/subr-x-tests.el | 22 ++++++++++++++-------
 5 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index ef848ac510..b71279d5d6 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -405,12 +405,16 @@ Creating Strings
 will not be shortened.
 @end defun
 
-@defun string-limit string length &optional end
+@defun string-prefix string length
 If @var{string} is shorter than @var{length}, @var{string} is returned
 as is.  Otherwise, return a substring of @var{string} consisting of
-the first @var{length} characters.  If the optional @var{end}
-parameter is given, return a string of the @var{length} last
-characters instead.
+the first @var{length} characters.
+@end defun
+
+@defun string-suffix string length
+If @var{string} is shorter than @var{length}, @var{string} is returned
+as is.  Otherwise, return a substring of @var{string} consisting of
+the last @var{length} characters.
 @end defun
 
 @defun string-lines string &optional omit-nulls
diff --git a/etc/NEWS b/etc/NEWS
index 46b8435a14..6ad95f7ca4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1442,7 +1442,7 @@ that makes it a valid button.
 
 +++
 *** A number of new string manipulation functions have been added.
-'string-clean-whitespace', 'string-fill', 'string-limit',
+'string-clean-whitespace', 'string-fill', 'string-prefix', 'string-suffx',
 'string-lines', 'string-pad', 'string-chop-newline' and 'string-slice'.
 
 +++
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index e9e1be1d55..0603046bf6 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -143,10 +143,12 @@ string
   (substring
    :eval (substring "foobar" 0 3)
    :eval (substring "foobar" 3))
-  (string-limit
-   :eval (string-limit "foobar" 3)
-   :eval (string-limit "foobar" 3 t)
-   :eval (string-limit "foobar" 10))
+  (string-prefix
+   :eval (string-prefix "abcde" 3)
+   :eval (string-prefix "abcde" 10))
+  (string-suffix
+   :eval (string-suffix "abcde" 3)
+   :eval (string-suffix "abcde" 10))
   (split-string
    :eval (split-string "foo bar")
    :eval (split-string "|foo|bar|" "|")
diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 09c4649817..b775989a88 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -286,20 +286,25 @@ string-fill
       (fill-region (point-min) (point-max)))
     (buffer-string)))
 
-(defun string-limit (string length &optional end)
-  "Return (up to) a LENGTH substring of STRING.
-If STRING is shorter than or equal to LENGTH, the entire string
-is returned unchanged.
-
-If STRING is longer than LENGTH, return a substring consisting of
-the first LENGTH characters of STRING.  If END is non-nil, return
-the last LENGTH characters instead."
+(defsubst string-prefix (string length)
+  "Return the first LENGTH characters of STRING.
+If LENGTH is greater than the length of STRING, return STRING."
+  (declare (side-effect-free t) (pure t))
   (unless (natnump length)
     (signal 'wrong-type-argument (list 'natnump length)))
-  (cond
-   ((<= (length string) length) string)
-   (end (substring string (- (length string) length)))
-   (t (substring string 0 length))))
+  (if (< length (length string))
+      (substring string 0 length)
+    string))
+
+(defsubst string-suffix (string length)
+  "Return the last LENGTH characters of STRING.
+If LENGTH is greater than the length of STRING, return STRING."
+  (declare (side-effect-free t) (pure t))
+  (unless (natnump length)
+    (signal 'wrong-type-argument (list 'natnump length)))
+  (if (< length (length string))
+        (substring string (- (length string) length))
+      string))
 
 (defun string-lines (string &optional omit-nulls)
   "Split STRING into a list of lines.
diff --git a/test/lisp/emacs-lisp/subr-x-tests.el b/test/lisp/emacs-lisp/subr-x-tests.el
index 854d61ed28..88b763fa40 100644
--- a/test/lisp/emacs-lisp/subr-x-tests.el
+++ b/test/lisp/emacs-lisp/subr-x-tests.el
@@ -592,13 +592,21 @@ subr-string-fill
   (should (equal (string-fill "foo bar zot" 5) "foo\nbar\nzot"))
   (should (equal (string-fill "foo bar zot" 7) "foo bar\nzot")))
 
-(ert-deftest subr-string-limit ()
-  (should (equal (string-limit "foo" 10) "foo"))
-  (should (equal (string-limit "foo" 2) "fo"))
-  (should (equal (string-limit "foo" 2 t) "oo"))
-  (should (equal (string-limit "abc" 10 t) "abc"))
-  (should (equal (string-limit "foo" 0) ""))
-  (should-error (string-limit "foo" -1)))
+(ert-deftest subr-string-prefix ()
+  (should (equal (string-prefix "abc" 4) "abc"))
+  (should (equal (string-prefix "abc" 3) "abc"))
+  (should (equal (string-prefix "abc" 2) "ab"))
+  (should (equal (string-prefix "abc" 1) "a"))
+  (should (equal (string-prefix "abc" 0) ""))
+  (should-error (string-prefix "abc" -1)))
+
+(ert-deftest subr-string-suffix ()
+  (should (equal (string-suffix "abc" 4) "abc"))
+  (should (equal (string-suffix "abc" 3) "abc"))
+  (should (equal (string-suffix "abc" 2) "bc"))
+  (should (equal (string-suffix "abc" 1) "c"))
+  (should (equal (string-suffix "abc" 0) ""))
+  (should-error (string-suffix "abc" -1)))
 
 (ert-deftest subr-string-lines ()
   (should (equal (string-lines "foo") '("foo")))
-- 
2.21.1 (Apple Git-122.3)