Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: empty byte slice prints one value with format "% 02x" #10430

Closed
robpike opened this issue Apr 12, 2015 · 8 comments
Closed

fmt: empty byte slice prints one value with format "% 02x" #10430

robpike opened this issue Apr 12, 2015 · 8 comments
Assignees
Milestone

Comments

@robpike
Copy link
Contributor

@robpike robpike commented Apr 12, 2015

https://play.golang.org/p/S51xT7K4Au

package main

import "fmt"

func main() {
fmt.Printf("% 02x\n", []byte{0})
fmt.Printf("% 02x\n", []byte{})
}

prints 00.

@robpike robpike self-assigned this Apr 12, 2015
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Apr 12, 2015

So?

https://play.golang.org/p/35pyRrGLXN

package main

import "fmt"

func main() {
    fmt.Printf("% 08x\n", []byte{1})
}

prints:

00000001

Are those leading zeros wrong too?

The user asked for it to be zero-padded.

@paulhankin
Copy link

@paulhankin paulhankin commented Apr 13, 2015

I think while this is surprising, it isn't a bug as it works as documented in https://golang.org/pkg/fmt/:
"""
For most values, width is the minimum number of runes to output, padding the formatted form with spaces if necessary.

0 pad with leading zeros rather than spaces;
"""

"% 08x" applied to an argument of type []byte doesn't mean "format the bytes in the argument with 8 hex characters per byte, separated by spaces."

It means "format the bytes in the argument with 2 hex characters per byte, separated by spaces, and then pad the result with 0s on the left if the string has length less than 8."

Thus, fmt.Printf("% 04x", []byte{}) first produces "" which is padded to "0000".
And fmt.Printf("% 04x", []byte{2}) first produces "02" which is padded to "0002".

Examples: https://play.golang.org/p/lvP2PBEdJZ

@robpike
Copy link
Contributor Author

@robpike robpike commented Apr 13, 2015

"For compound operands such as slices and structs, the format applies to the elements of each operand, recursively, not to the operand as a whole. Thus %q will quote each element of a slice of strings, and %6.2f will control formatting for each element of a floating-point array."

So for an empty slice it should print nothing. Fix coming.

@robpike robpike closed this in 5705832 Apr 13, 2015
@mikioh mikioh added this to the Go1.5 milestone Apr 14, 2015
@rajender
Copy link
Contributor

@rajender rajender commented Apr 14, 2015

As brad asked, isn't this change effectively make width not applicable for string and byte slices? Should we mention that in docs?

It seems, the previous behavior was adhering to the following statement in the docs.

"For most values, width is the minimum number of runes to output, padding the formatted form with spaces if necessary."

raj@rajender:~/go/src> cat ~/a.go
package main

import "fmt"

func main() {
fmt.Printf("% 20x\n", []byte{1})
fmt.Printf("% 20x\n", []byte{1, 1})
fmt.Printf("% 20x\n", []byte{1, 1, 1})
fmt.Printf("% 20x\n", []byte{1, 1, 1, 1})
}
raj@rajender:~/go/src> /go1.4/bin/go version
go version go1.4.2 linux/amd64
raj@rajender:
/go/src> ~/go1.4/bin/go run /a.go
01
01 01
01 01 01
01 01 01 01
raj@rajender:
/go/src> /go/bin/go version
go version devel +2f98bac Tue Apr 14 07:22:44 2015 +0000 linux/amd64
raj@rajender:
/go/src> ~/go/bin/go run ~/a.go
01
01 01
01 01 01
01 01 01 01

raj@rajender:~/go/src> cat ~/a.go
package main

import "fmt"

func main() {
fmt.Printf("% 05s\n", []byte{})
}
raj@rajender:~/go/src> /go/bin/go version
go version devel +2f98bac Tue Apr 14 07:22:44 2015 +0000 linux/amd64
raj@rajender:
/go/src> ~/go/bin/go run ~/a.go
00000

@martisch
Copy link
Contributor

@martisch martisch commented Apr 15, 2015

hi,

the statement in the patch notes that the loop applies the padding is false.

See a testcase like:

{"% 03x", []byte{1, 2, 3}, "001 002 003"},

it works for 02x because two characters per byte for 1 is already 01.

i was working on the todo in the function that changed and had make some test cases for the old padding behavior. They are failing now which is correct as rob explained (since they should be element wise) but their output also reveal that no padding is applied anymore regardless if it should per element or not.

{"%20X", []byte("xyz"), "              78797A"},
{"% 20X", []byte("xyz"), "            78 79 7A"},
{"%#20X", []byte("xyz"), "            0X78797A"},
{"%# 20X", []byte("xyz"), "      0X78 0X79 0X7A"},
{"%-20X", []byte("xyz"), "78797A              "},
{"% -20X", []byte("xyz"), "78 79 7A            "},
{"%-#20X", []byte("xyz"), "0X78797A            "},
{"%-# 20X", []byte("xyz"), "0X78 0X79 0X7A      "},

--- FAIL: TestSprintf (0.00s)
    fmt_test.go:729: Sprintf("%20X", [120 121 122]) = "78797A" want "              78797A"
    fmt_test.go:729: Sprintf("% 20X", [120 121 122]) = "78 79 7A" want "            78 79 7A"
    fmt_test.go:729: Sprintf("%#20X", [120 121 122]) = "0X78797A" want "            0X78797A"
    fmt_test.go:729: Sprintf("%# 20X", [120 121 122]) = "0X78 0X79 0X7A" want "      0X78 0X79 0X7A"
    fmt_test.go:729: Sprintf("%-20X", [120 121 122]) = "78797A" want "78797A              "
    fmt_test.go:729: Sprintf("% -20X", [120 121 122]) = "78 79 7A" want "78 79 7A            "
    fmt_test.go:729: Sprintf("%-#20X", [120 121 122]) = "0X78797A" want "0X78797A            "
    fmt_test.go:729: Sprintf("%-# 20X", [120 121 122]) = "0X78 0X79 0X7A" want "0X78 0X79 0X7A      "

If there should be be per element padding we need to amend the loop to add 0 and white space padding and i guess we also need to treat strings and byte slices differently in respect to padding in the loop or an extra function. Created a new issue.

As for strings something more similar to the old padding application might need to be restored:

    {"%20X","xyz", "              78797A"},
    {"% 20X","xyz", "            78 79 7A"},
    {"%#20X","xyz", "            0X78797A"},
    {"%# 20X","xyz", "      0X78 0X79 0X7A"},
    {"%-20X","xyz", "78797A              "},
    {"% -20X","xyz", "78 79 7A            "},
    {"%-#20X","xyz", "0X78797A            "},
    {"%-# 20x","xyz", "0X78 0X79 0X7A      "},
@martisch
Copy link
Contributor

@martisch martisch commented Apr 15, 2015

adding to the above - not sure if this is another inconsistency if we need to format per element by the rule "For compound operands such as slices and structs, the format applies to the elements of each operand, recursively, not to the operand as a whole.":

currently:
Sprintf("% 02x", [1]) = "01"

but if the format is applied per element should the ' ' mean we need to make a space for the sign?
Sprintf("% 02x", [1]) = " 01"

like currently with normal two digit numbers:
"% 02X", and uint16('x') yields Sprintf("% 02X", 120) = " 78"

and the same question can be asked for the # option.

To me these two statements cant hold at the same time for slices of bytes:

  1. "For compound operands such as slices and structs, the format applies to the elements of each operand, recursively, not to the operand as a whole."
  2. " String and slice of bytes:
    %x base 16, lower-case, two characters per byte
    %X base 16, upper-case, two characters per byte"

If 1) then {"%x", []byte{1, 2, 3}, "[1 2 3]"},
If 2) then {"%x", []byte{1, 2, 3}, "010203"},

@robpike
Copy link
Contributor Author

@robpike robpike commented Jun 27, 2015

Reverting this change. An empty slice or empty string printed with padding should show the padding. I will adjust the documentation to make this clear.

@robpike robpike reopened this Jun 27, 2015
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 27, 2015

CL https://golang.org/cl/11600 mentions this issue.

@robpike robpike closed this in a76c1a5 Jun 29, 2015
@golang golang locked and limited conversation to collaborators Jun 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.