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

* bug#69822: [PATCH] format-spec pads when it should only truncate
  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-16 10:35 ` Basil L. Contovounesios
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-03-16 10:22 UTC (permalink / raw)
  To: Adam Porter; +Cc: 69822

> Date: Fri, 15 Mar 2024 21:28:15 -0500
> From: Adam Porter <adam@alphapapa.net>
> 
> 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.

I don't understand why you consider it a bug.  According to the doc
string (see below), this:

  (format-spec "%>15t" '((?t . "0123456789")))

should behave the same as this:

  (format "%15s" "0123456789")

And in my testing, it does: both produce "     0123456789".

So I don't think I agree that there's a bug here to begin with.  The
doc string of format-spec says:

  The width and truncation modifiers behave like the corresponding
  ones in ‘format’ when applied to %s.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  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-16 10:35 ` Basil L. Contovounesios
  1 sibling, 0 replies; 11+ messages in thread
From: Basil L. Contovounesios @ 2024-03-16 10:35 UTC (permalink / raw)
  To: Adam Porter; +Cc: 69822

Adam Porter [2024-03-15 21:28 -0500] wrote:

> 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.

In this format string, 15 is the 'width' (which controls padding), not
the 'precision' (which controls truncation).

To truncate beyond 15 without padding, specify a precision instead of a
width: "%>.15t".

So I'm not sure there's a bug here, or at least not the one you
describe.

What's not clear in the documentation is what happens when one specifies
< or > without an explicit precision, as in the example you give.  I'm
guessing this was the source of confusion here.  But I think this can be
addressed as a docfix.  Or am I missing something?

Thanks,
-- 
Basil





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-16 10:22 ` Eli Zaretskii
@ 2024-03-17  4:12   ` Adam Porter
  2024-03-17  6:23     ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Adam Porter @ 2024-03-17  4:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Basil L. Contovounesios, 69822

Hi Eli, Basil,

On 3/16/24 05:22, Eli Zaretskii wrote:

> I don't understand why you consider it a bug.  According to the doc 
> string (see below)...
> 
> So I don't think I agree that there's a bug here to begin with.  The 
> doc string of format-spec says:
> 
> The width and truncation modifiers behave like the corresponding ones
> in ‘format’ when applied to %s.

Thanks, now I understand.

Should I close the bug, or do you think the docstring should be changed 
to help clarify this?





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-17  4:12   ` Adam Porter
@ 2024-03-17  6:23     ` Eli Zaretskii
  2024-03-17 12:11       ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2024-03-17  6:23 UTC (permalink / raw)
  To: Adam Porter; +Cc: basil, 69822

> Date: Sat, 16 Mar 2024 23:12:16 -0500
> Cc: 69822@debbugs.gnu.org, "Basil L. Contovounesios" <basil@contovou.net>
> From: Adam Porter <adam@alphapapa.net>
> 
> Hi Eli, Basil,
> 
> On 3/16/24 05:22, Eli Zaretskii wrote:
> 
> > I don't understand why you consider it a bug.  According to the doc 
> > string (see below)...
> > 
> > So I don't think I agree that there's a bug here to begin with.  The 
> > doc string of format-spec says:
> > 
> > The width and truncation modifiers behave like the corresponding ones
> > in ‘format’ when applied to %s.
> 
> Thanks, now I understand.
> 
> Should I close the bug, or do you think the docstring should be changed 
> to help clarify this?

If there's something in the doc string that is unclear, please tell
what that is, and let's by all means try to find ways to clarify that.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-17  6:23     ` Eli Zaretskii
@ 2024-03-17 12:11       ` Basil L. Contovounesios
  2024-03-18  1:16         ` Adam Porter
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2024-03-17 12:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Adam Porter, 69822

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

Eli Zaretskii [2024-03-17 08:23 +0200] wrote:

>> Date: Sat, 16 Mar 2024 23:12:16 -0500
>> Cc: 69822@debbugs.gnu.org, "Basil L. Contovounesios" <basil@contovou.net>
>> From: Adam Porter <adam@alphapapa.net>
>> 
>> Hi Eli, Basil,
>> 
>> On 3/16/24 05:22, Eli Zaretskii wrote:
>> 
>> > I don't understand why you consider it a bug.  According to the doc 
>> > string (see below)...
>> > 
>> > So I don't think I agree that there's a bug here to begin with.  The 
>> > doc string of format-spec says:
>> > 
>> > The width and truncation modifiers behave like the corresponding ones
>> > in ‘format’ when applied to %s.
>> 
>> Thanks, now I understand.
>> 
>> Should I close the bug, or do you think the docstring should be changed 
>> to help clarify this?
>
> If there's something in the doc string that is unclear, please tell
> what that is, and let's by all means try to find ways to clarify that.

How about this for now?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-description-of-format-spec-truncation.patch --]
[-- Type: text/x-diff, Size: 2196 bytes --]

From a4ffad9891b607e432ce246773b1f2d75d3cdeeb Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <basil@contovou.net>
Date: Sun, 17 Mar 2024 13:04:32 +0100
Subject: [PATCH] Clarify description of format-spec truncation

* doc/lispref/strings.texi (Custom Format Strings): Mention that
precision specifier affects both '<' and '>' truncation (bug#69822).
* lisp/format-spec.el (format-spec, format-spec--do-flags): Use same
terminology as 'format', especially when referring to its behavior.
---
 doc/lispref/strings.texi | 2 +-
 lisp/format-spec.el      | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index a364fef3aab..eca69002779 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -1369,7 +1369,7 @@ Custom Format Strings
 
 @item >
 This flag causes the substitution to be truncated on the right to the
-given width, if specified.
+given width and precision, if specified.
 
 @item ^
 This flag converts the substituted text to upper case (@pxref{Case
diff --git a/lisp/format-spec.el b/lisp/format-spec.el
index cf34017b994..73f9fccd793 100644
--- a/lisp/format-spec.el
+++ b/lisp/format-spec.el
@@ -38,7 +38,7 @@ format-spec
                  (?l . \"ls\")))
 
 Each %-spec may contain optional flag, width, and precision
-modifiers, as follows:
+specifiers, as follows:
 
   %<flags><width><precision>character
 
@@ -51,7 +51,7 @@ format-spec
 * ^: Convert to upper case.
 * _: Convert to lower case.
 
-The width and truncation modifiers behave like the corresponding
+The width and precision specifiers behave like the corresponding
 ones in `format' when applied to %s.
 
 For example, \"%<010b\" means \"substitute into the output the
@@ -145,7 +145,7 @@ format-spec--do-flags
   "Return STR formatted according to FLAGS, WIDTH, and TRUNC.
 FLAGS is a list of keywords as returned by
 `format-spec--parse-flags'.  WIDTH and TRUNC are either nil or
-string widths corresponding to `format-spec' modifiers."
+string widths corresponding to `format-spec' specifiers."
   (let (diff str-width)
     ;; Truncate original string first, like `format' does.
     (when trunc
-- 
2.43.0


[-- Attachment #3: Type: text/plain, Size: 248 bytes --]


I don't know why I added 'and precision' to the description of '<' but
not '>' in:

  Fix and extend format-spec (bug#41758)
  0185d76e742 2020-06-18 12:46:21 +0100
  https://git.sv.gnu.org/cgit/emacs.git/commit/?id=0185d76e742

Thanks,
-- 
Basil

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-17 12:11       ` Basil L. Contovounesios
@ 2024-03-18  1:16         ` Adam Porter
  2024-03-21  9:52           ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: Adam Porter @ 2024-03-18  1:16 UTC (permalink / raw)
  To: Basil L. Contovounesios, Eli Zaretskii; +Cc: 69822

Hi Basil, Eli,

On 3/17/24 07:11, Basil L. Contovounesios wrote:

>>> Should I close the bug, or do you think the docstring should be changed
>>> to help clarify this?
>>
>> If there's something in the doc string that is unclear, please tell
>> what that is, and let's by all means try to find ways to clarify that.
> 
> How about this for now?

Thanks, I think that will be helpful.

--Adam





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-18  1:16         ` Adam Porter
@ 2024-03-21  9:52           ` Basil L. Contovounesios
  2024-03-21 10:47             ` Adam Porter
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2024-03-21  9:52 UTC (permalink / raw)
  To: Adam Porter; +Cc: Eli Zaretskii, 69822

Adam Porter [2024-03-17 20:16 -0500] wrote:
> On 3/17/24 07:11, Basil L. Contovounesios wrote:
>>>> Should I close the bug, or do you think the docstring should be changed
>>>> to help clarify this?
>>> If there's something in the doc string that is unclear, please tell
>>> what that is, and let's by all means try to find ways to clarify that.
>> How about this for now?
> Thanks, I think that will be helpful.

Pushed to emacs-29:

Clarify description of format-spec truncation
689f04a2ddf 2024-03-21 10:43:17 +0100
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=689f04a2ddf

Feel free to keep this open if you think there's more to be done here.
Or can the report be closed?

Thanks,
-- 
Basil





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-21  9:52           ` Basil L. Contovounesios
@ 2024-03-21 10:47             ` Adam Porter
  2024-03-21 11:03               ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: Adam Porter @ 2024-03-21 10:47 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: Eli Zaretskii, 69822

Hi Basil,

On 3/21/24 04:52, Basil L. Contovounesios wrote:
> Adam Porter [2024-03-17 20:16 -0500] wrote:
>> On 3/17/24 07:11, Basil L. Contovounesios wrote:
>>>>> Should I close the bug, or do you think the docstring should be changed
>>>>> to help clarify this?
>>>> If there's something in the doc string that is unclear, please tell
>>>> what that is, and let's by all means try to find ways to clarify that.
>>> How about this for now?
>> Thanks, I think that will be helpful.
> 
> Pushed to emacs-29:
> 
> Clarify description of format-spec truncation
> 689f04a2ddf 2024-03-21 10:43:17 +0100
> https://git.sv.gnu.org/cgit/emacs.git/commit/?id=689f04a2ddf
> 
> Feel free to keep this open if you think there's more to be done here.
> Or can the report be closed?

Thank you.  I'm happy for it to be closed.  (Can the reporter do that, 
or does it have to be a maintainer?  I can't remember, and the tracker's 
Web pages don't seem to say for certain.)

--Adam





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-21 10:47             ` Adam Porter
@ 2024-03-21 11:03               ` Basil L. Contovounesios
  2024-03-23 13:59                 ` Adam Porter
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2024-03-21 11:03 UTC (permalink / raw)
  To: Adam Porter; +Cc: Eli Zaretskii, 69822

Adam Porter [2024-03-21 05:47 -0500] wrote:

> I'm happy for it to be closed.  (Can the reporter do that, or does
> it have to be a maintainer?  I can't remember, and the tracker's Web pages don't
> seem to say for certain.)

Anyone can close it.  The easiest way is to change the To/Cc address
from 69822@debbugs.gnu.org to 69822-done@debbugs.gnu.org.

[Details are in the file admin/notes/bugtracker.]

Alternatively you can email or Bcc control@debbugs.gnu.org and start
your message with Debbugs commands.  I tend to rely on the command
debbugs-gnu-make-control-message from the debbugs package.

HTH,
-- 
Basil





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#69822: [PATCH] format-spec pads when it should only truncate
  2024-03-21 11:03               ` Basil L. Contovounesios
@ 2024-03-23 13:59                 ` Adam Porter
  0 siblings, 0 replies; 11+ messages in thread
From: Adam Porter @ 2024-03-23 13:59 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 69822-done

On 3/21/24 06:03, Basil L. Contovounesios wrote:
> Adam Porter [2024-03-21 05:47 -0500] wrote:
> 
>> I'm happy for it to be closed.  (Can the reporter do that, or does
>> it have to be a maintainer?  I can't remember, and the tracker's Web pages don't
>> seem to say for certain.)
> 
> Anyone can close it.  The easiest way is to change the To/Cc address
> from 69822@debbugs.gnu.org to 69822-done@debbugs.gnu.org.

Thanks, Basil.  Closing...





^ permalink raw reply	[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).