From: Adam Porter <adam@alphapapa.net>
To: 69822@debbugs.gnu.org
Subject: bug#69822: [PATCH] format-spec pads when it should only truncate
Date: Fri, 15 Mar 2024 21:28:15 -0500 [thread overview]
Message-ID: <cc0e3544-d0df-405b-8646-1159a21d9274@alphapapa.net> (raw)
[-- 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
next reply other threads:[~2024-03-16 2:28 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-16 2:28 Adam Porter [this message]
2024-03-16 10:22 ` bug#69822: [PATCH] format-spec pads when it should only truncate 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cc0e3544-d0df-405b-8646-1159a21d9274@alphapapa.net \
--to=adam@alphapapa.net \
--cc=69822@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).