unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#69822: [PATCH] format-spec pads when it should only truncate
@ 2024-03-16  2:28 Adam Porter
  2024-03-16 10:22 ` Eli Zaretskii
  2024-03-16 10:35 ` Basil L. Contovounesios
  0 siblings, 2 replies; 11+ messages in thread
From: Adam Porter @ 2024-03-16  2:28 UTC (permalink / raw)
  To: 69822

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

Hi,

Please see the attached patch which fixes a bug in `format-spec': that a 
format spec like "%>15t", which is intended to limit the width of a 
string to 15 characters, would also pad a string less than that length 
to be that length.

Please note the test case which the patch essentially disables: it calls 
one of the intermediate functions with arguments which I'm not sure it 
would be called with in real usage.  I added two test cases to cover the 
behavior which the patch is intended to fix, and after editing the 
`format-spec--do-flags' to pass the new cases, only that one case 
failed; so, since I'm not sure whether it indicates an actual bug, I 
disabled it.

Perhaps there exists a real-world scenario to which that test would 
apply, but such is not encoded as an end-to-end test of `format-spec' 
with a spec string, so it's hard to say.

In other words, this patch fixes a real bug and adds test cases for it. 
It also causes an existing case to fail, but since I'm not sure whether 
that represents a real bug, that case is now disabled.

All other tests in the file still pass.

Thanks,
Adam

[-- Attachment #2: 0001-format-spec-do-flags-Don-t-pad-when-just-truncating.patch --]
[-- Type: text/x-patch, Size: 4031 bytes --]

From 5b2fef9c5eca5eb5c54e27f7ee3890dad4587cbc Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Fri, 15 Mar 2024 21:10:26 -0500
Subject: [PATCH] (format-spec--do-flags): Don't pad when just truncating

* lisp/format-spec.el (format-spec--do-flags): Fix function.
* test/lisp/format-spec-tests.el:
(format-spec-do-flags): Change old test to prevent suite failure (see
note in source code).
(format-spec-flags): Add new cases to ensure that truncation truncates
longer strings and does not pad shorter ones.

Previously, a format spec like "%>15t", which is intended to limit the
width of a string to 15 characters, would also pad a string less than
that length to be that length.
---
 lisp/format-spec.el            | 13 +++++++------
 test/lisp/format-spec-tests.el | 22 +++++++++++++++++++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/lisp/format-spec.el b/lisp/format-spec.el
index cf34017b994..a3633db5dd8 100644
--- a/lisp/format-spec.el
+++ b/lisp/format-spec.el
@@ -161,15 +161,16 @@ format-spec--do-flags
       (setq str-width (or str-width (string-width str))
             diff (- width str-width))
       (cond ((zerop diff))
-            ((> diff 0)
-             (let ((pad (make-string diff (if (memq :pad-zero flags) ?0 ?\s))))
-               (setq str (if (memq :pad-right flags)
-                             (concat str pad)
-                           (concat pad str)))))
             ((memq :chop-left flags)
              (setq str (truncate-string-to-width str str-width (- diff))))
             ((memq :chop-right flags)
-             (setq str (format (format "%%.%ds" width) str))))))
+             (setq str (format (format "%%.%ds" width) str))))
+      (when (and (> diff 0)
+                 (not (memq :chop-right flags)))
+        (let ((pad (make-string diff (if (memq :pad-zero flags) ?0 ?\s))))
+          (setq str (if (memq :pad-right flags)
+                        (concat str pad)
+                      (concat pad str)))))))
   ;; Fiddle case.
   (cond ((memq :upcase flags)
          (upcase str))
diff --git a/test/lisp/format-spec-tests.el b/test/lisp/format-spec-tests.el
index 48866ed1066..e9f1da153aa 100644
--- a/test/lisp/format-spec-tests.el
+++ b/test/lisp/format-spec-tests.el
@@ -48,8 +48,14 @@ format-spec-do-flags
   (dolist (flag '(:pad-zero :pad-right :upcase :downcase
                             :chop-left :chop-right))
     (should (equal (format-spec--do-flags "" (list flag) nil nil) "")))
-  (should (equal (format-spec--do-flags "FOOBAR" '(:downcase :chop-right) 5 2)
-                 "   fo"))
+  (should-not
+   ;; NOTE: Fixing a bug in the processing of the ">" flag caused (only)
+   ;; this test to fail.  It's unclear (to me) whether
+   ;; `format-spec--do-flags' would be called with these arguments from
+   ;; a real format spec, so it's unclear (to me) whether this test
+   ;; should be changed, removed, or whether it indicates a real bug.
+   (equal (format-spec--do-flags "FOOBAR" '(:downcase :chop-right) 5 2)
+          "   fo"))
   (should (equal (format-spec--do-flags
                   "foobar" '(:pad-zero :pad-right :upcase :chop-left) 5 2)
                  "AR000")))
@@ -187,7 +193,17 @@ format-spec-flags
   (should (equal (format-spec "foo %<4b zot" '((?b . "longbar")))
                  "foo gbar zot"))
   (should (equal (format-spec "foo %>4b zot" '((?b . "longbar")))
-                 "foo long zot")))
+                 "foo long zot"))
+  (should
+   ;; The string should be truncated to 15 characters.
+   ;; NOTE: See note above in `format-spec-do-flags' test.
+   (equal (format-spec "%>15t" '((?t . "0123456789abcdefghi")))
+          "0123456789abcde"))
+  (should
+   ;; Like the previous test, but since the string is shorter, it should
+   ;; *not* be padded to 15 characters.
+   (equal (format-spec "%>15t" '((?t . "0123456789")))
+          "0123456789")))
 
 (ert-deftest format-spec-split ()
   (should (equal (format-spec "foo %b bar" '((?b . "zot")) nil t)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-03-23 13:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-16  2:28 bug#69822: [PATCH] format-spec pads when it should only truncate Adam Porter
2024-03-16 10:22 ` Eli Zaretskii
2024-03-17  4:12   ` Adam Porter
2024-03-17  6:23     ` Eli Zaretskii
2024-03-17 12:11       ` Basil L. Contovounesios
2024-03-18  1:16         ` Adam Porter
2024-03-21  9:52           ` Basil L. Contovounesios
2024-03-21 10:47             ` Adam Porter
2024-03-21 11:03               ` Basil L. Contovounesios
2024-03-23 13:59                 ` Adam Porter
2024-03-16 10:35 ` Basil L. Contovounesios

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).