fmt: padding flag is not accepted with minus flag #61784
Comments
|
CC @robpike @martisch via https://dev.golang.org/owners At a quick glance, if the defaults don't understand both of these flags at the same time, I would be somewhat surprised if they could both be passed on to the custom formatter. In a sense it's part of the printf string format now that using both together only gets you one. But I'll let Rob and/or Martin chime in. |
mknyszek
added
the
NeedsInvestigation
|
This is not straightforward to do because, at least in the existing code, '0' and '-' cannot be set at the same time because of how padding works. In fact, this snippet tells the story: I believe the work it would take to make this behave as requested is disproportionate to the general utility, and would like to not do it, marking it "Unfortunate". |
|
From me looking at the code the change necessary seems to be trivial because the code is so well written and organized: diff --git a/src/fmt/format.go b/src/fmt/format.go
index 617f78f15e..55cfcf67a0 100644
--- a/src/fmt/format.go
+++ b/src/fmt/format.go
@@ -75,7 +75,8 @@ func (f *fmt) writePadding(n int) {
}
// Decide which byte the padding should be filled with.
padByte := byte(' ')
- if f.zero {
+ // Zero padding is allowed only to the left.
+ if f.zero && !f.minus {
padByte = byte('0')
}
// Fill padding with padByte.
@@ -223,7 +224,7 @@ func (f *fmt) fmtInteger(u uint64, base int, isSigned bool, verb rune, digits st
f.zero = oldZero
return
}
- } else if f.zero && f.widPresent {
+ } else if f.zero && !f.minus && f.widPresent { // Zero padding is allowed only to the left.
prec = f.wid
if negative || f.plus || f.space {
prec-- // leave room for sign
@@ -580,7 +581,8 @@ func (f *fmt) fmtFloat(v float64, size int, verb rune, prec int) {
if f.plus || num[0] != '+' {
// If we're zero padding to the left we want the sign before the leading zeros.
// Achieve this by writing the sign out and then padding the unsigned number.
- if f.zero && f.widPresent && f.wid > len(num) {
+ // Zero padding is allowed only to the left.
+ if f.zero && !f.minus && f.widPresent && f.wid > len(num) {
f.buf.writeByte(num[0])
f.writePadding(f.wid - len(num))
f.buf.write(num[1:])
diff --git a/src/fmt/print.go b/src/fmt/print.go
index 9c3bd3efec..2997f78c61 100644
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -1050,12 +1050,11 @@ formatLoop:
case '#':
p.fmt.sharp = true
case '0':
- p.fmt.zero = !p.fmt.minus // Only allow zero padding to the left.
+ p.fmt.zero = true
case '+':
p.fmt.plus = true
case '-':
p.fmt.minus = true
- p.fmt.zero = false // Do not pad with zeros to the right.
case ' ':
p.fmt.space = true
default:So just moving the check from the point where it is set to where it is used. Am I missing something? Tests pass. |
|
OK, I buy that. Nicely done. Please send a CL. |
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
|
Done #61836. |
|
Change https://go.dev/cl/516975 mentions this issue: |
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
Existing implementation did not allow setting both padding and minus flags at the same time because standard formatting does not allow that. But custom Formatter interface implementations might have use of it. This change moves the check from the place flags are parsed to where they are used in standard formatting. Fixes golang#61784
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
I am trying to use a custom
Formatmethod on my struct to control how it is formatted usingfmtstandard package. I am formatting stack traces for an error struct I have in my errors package. I wanted to support both0and-flags at the same time with custom meaning for stack tracing formatting and error unwrapping.So I tried
%0+vformat string.What did you expect to see?
That true is returned from both
s.Flag('0')ands.Flag('-').What did you see instead?
Only
s.Flag('-')returns true.Discussion
So this is similar to #51419. And I think decision there is fine. But the issue is that it seems disabling padding flag when also minus flag is provided is done too early in the process so that even custom
Formatmethod does not get both flags, but for the custom method providing both flags at the same time might have sense.So I think both flags should be parsed out and passed on to
Format, but then standard implementation for string, byte slices and byte arrays should ignore padding flag if minus flag is provided. Like I could decide to do the same insideFormatmethod.The text was updated successfully, but these errors were encountered: