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: no padding is applied for "%x" or "%X" #10465

Closed
martisch opened this issue Apr 15, 2015 · 3 comments
Closed

fmt: no padding is applied for "%x" or "%X" #10465

martisch opened this issue Apr 15, 2015 · 3 comments
Assignees
Milestone

Comments

@martisch
Copy link
Contributor

@martisch martisch commented Apr 15, 2015

go version devel +9402e49 Wed Apr 15 05:03:42 2015 +0000 darwin/amd64

testcases to see no padding is applied to strings and byte slices:

{"%04x", []byte{1, 2, 3}, "000100020003"},
{"%4x", []byte{1, 2, 3}, "  01  02  03"},

--- FAIL: TestSprintf (0.00s)
    fmt_test.go:723: Sprintf("%04x", [1 2 3]) = "010203" want "000100020003"
    fmt_test.go:723: Sprintf("%4x", [1 2 3]) = "010203" want "  01  02  03"

#10430
has clarified it is per element for byte slices. However after the change currently no padding seems to be applied since the loop mentioned there does not apply it. It only adds a single space for each element or "0x" but not a general padding.

Also strings and byte slice then as far as i understood need to be handled with differing padding logic as the strings needs to be padded as a whole not hex by hex.

would this be the intended behavior?

        {"%x", "xyz", "78797a"},
    {"%X", "xyz", "78797A"},
    {"% x", "xyz", "78 79 7a"},
    {"% X", "xyz", "78 79 7A"},
    {"%#x", "xyz", "0x78797a"},
    {"%#X", "xyz", "0X78797A"},
    {"%# x", "xyz", "0x78 0x79 0x7a"},
    {"%# X", "xyz", "0X78 0X79 0X7A"},
    {"%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      "},

    {"%x", []byte("xyz"), "78797a"},
    {"%X", []byte("xyz"), "78797A"},
    {"% x", []byte("xyz"), " 78 79 7a"},
    {"% X", []byte("xyz"), " 78 79 7A"},
    {"%#x", []byte("xyz"), "0x780x790x7a"},
    {"%#X", []byte("xyz"), "0X780X790X7A"},
    {"%# x", []byte("xyz"), " 0x78 0x79 0x7a"},
    {"%# X", []byte("xyz"), " 0X78 0X79 0X7A"},
    {"%4X", []byte("xyz"), "  78  79  7A"},
    {"% 4X", []byte("xyz"), "  78  79  7A"},
    {"%#4X", []byte("xyz"), "0X780X790X7A"},
    {"%# 4X", []byte("xyz"), " 0X78  0X79  0X7A"},
    {"%-4X", []byte("xyz"), "78  79  7A  "},
    {"% -4X", []byte("xyz"), " 78  79  7A "},
    {"%-#6X", []byte("xyz"), "0X78  0X79  0X7A  "},
    {"%-# 6X", []byte("xyz"), " 0X78  0X79  0X7A "}, // because Sprintf("%-# 6X", 120) = " 0X78 "

Its a bit unclear now to me how and if both of these statements should be applied to the examples:

  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"

P.S.
I will try to supply a patch to add padding back based on my nearly ready prepared patch to not use a buffer but adding the padding directly.

@martisch martisch changed the title fmt: no padding for "%x" or "%X" fmt: no padding is applied for "%x" or "%X" Apr 15, 2015
@robpike robpike self-assigned this Apr 15, 2015
@martisch
Copy link
Contributor Author

@martisch martisch commented Apr 15, 2015

some issues i encountered while reintroducing the padding with the changes of #10430 in mind:

if padding is applied on a per element basis precision should likely too:

However there are existing test cases that apply it to the whole slice:
{"%.5x", []byte("abcdefghijklmnopqrstuvwxyz"), 6162636465},

I think there are at least 3 options that can be implemented to format byte slices while correcting the padding:

  1. format byte slices no different than other integer slices and apply all formatting on an element basis:
    But then "% x" should be "[ 1 2 3]" not "1 2 3"
    Then the line " String and slice of bytes: " in documentation for %x and %X should be only referring to strings.

Also should change the doc "' ' (space) leave a space for elided sign in numbers (% d);
put spaces between bytes printing strings or slices in hex (% x, % X)"
to not refer to byte slices. Since in an absolute reading of "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." "% x" would never be applied to the whole byte slice and therefore cant introduce space between elements.

If there should still be spaces between elements then specifying "% x" would have the (likely unintended) side effect of introducing spaces between elements and adding a space for each byte element in place of sign at the same time.

  1. format byte slices in the old behavior like strings.

  2. format byte slices in the old behavior but apply ' ' only to the slice but #, padding and precision only to each element. Seems to be the version that would be favored by the test cases added in #10430. Then an open question is still if 1 without padding should be "1" or "01". In the first case then the documentation " String and slice of bytes: ... two characters per byte" needs to be changed.

@robpike
Copy link
Contributor

@robpike robpike commented May 6, 2015

There are some things that need to be thought about here and there are also conflicts in the documentation. For example, it says that "% x" puts spaces between the elements, which implies the first element should not have a space before it, and I believe that is correct and desired behavior, while on the other hand it says the format applies to all elements, which presumably applies to the first.

This is not an urgent issue and resolving it may break some things, so I am deferring it until 1.6.

@robpike robpike added this to the Go1.6 milestone May 6, 2015
@robpike
Copy link
Contributor

@robpike robpike commented Sep 29, 2015

The documentation says:

However, when printing a byte slice with a string-like verb (%s %q %x %X),
it is treated identically to a string, as a single item.

The behavior is consistent with that sentence: The entire item is padded, while the 0 and space flags do apply elementwise, as one would expect.

@robpike robpike closed this Sep 29, 2015
@golang golang locked and limited conversation to collaborators Sep 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
3 participants
You can’t perform that action at this time.