Skip to content

fmt: fix incorrect format of whole-number floats when using %#v #26383

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fmt: fix incorrect format of whole-number floats when using %#v #26383

wants to merge 1 commit into from

Conversation

dave
Copy link
Contributor

@dave dave commented Jul 14, 2018 •
edited
Loading

This fixes the unwanted behaviour where printing a zero float with the
#v fmt verb outputs "0" - e.g. missing the trailing decimal. This means
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes #26363

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jul 14, 2018
@dave dave mentioned this pull request Jul 14, 2018
Closed
@gopherbot gopherbot force-pushed the master branch 18 times, most recently from 9092511 to 95c3348 Compare July 19, 2018 18:17
@gopherbot gopherbot force-pushed the master branch 5 times, most recently from 0090c13 to 8fbbf63 Compare July 28, 2018 01:16
@gopherbot
Copy link
Contributor

Message from Gerrit User 5015:

Patch Set 1:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 2:

Patch Set 1:

(3 comments)

Done!

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 2:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 3: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 5:

remember that you need to update the PR title and description to update the commit message. that's #25359

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 6: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 7: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 8: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 9: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 10: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 11: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 12: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 13: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 13:

(5 comments)

Patch Set 5:

remember that you need to update the PR title and description to update the commit message. that's #25359

OK, I think I got it working. Sorry I haven't used Gerrit in five years.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 13: Run-TryBot+1 Code-Review+1

Will leave it to Rob to +2.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 13:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=b8b1f160

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 13:

Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/b8b1f160/linux-amd64_2aea81da.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 13: TryBot-Result-1

5 of 19 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/b8b1f160/linux-amd64_2aea81da.log
Failed on freebsd-amd64-11_1: https://storage.googleapis.com/go-build-log/b8b1f160/freebsd-amd64-11_1_27d0e934.log
Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/b8b1f160/nacl-amd64p32_ad12b5bf.log
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/b8b1f160/openbsd-amd64-62_b2cad448.log
Failed on nacl-386: https://storage.googleapis.com/go-build-log/b8b1f160/nacl-386_f3394c7b.log

Consult https://build.golang.org/ to see whether they are new failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 13:

it seems like some compiler tests broke, as some compiler error messages got a bit better. please update the tests so that they pass again.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 13:

Patch Set 13:

it seems like some compiler tests broke, as some compiler error messages got a bit better. please update the tests so that they pass again.

It looks like the failures aren't connected to this CL though... Is there a retry button somewhere?

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 13:

It looks like the failures aren't connected to this CL though... Is there a retry button somewhere?

Yes, they are connected. The compiler now prints better errors in some cases including floats, so you need to update the tests to change the expected error strings. This is what I meant above.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 13:

Patch Set 13:

It looks like the failures aren't connected to this CL though... Is there a retry button somewhere?

Yes, they are connected. The compiler now prints better errors in some cases including floats, so you need to update the tests to change the expected error strings. This is what I meant above.

OK cool I'll take a look...

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@dave
fmt: fix incorrect format of whole-number floats when using %#v
adfb061 
This fixes the unwanted behaviour where printing a zero float with the
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes golang#26363
@gopherbot
Copy link
Contributor

Message from Gerrit User 12415:

Patch Set 14:

Patch Set 13:

It looks like the failures aren't connected to this CL though... Is there a retry button somewhere?

Yes, they are connected. The compiler now prints better errors in some cases including floats, so you need to update the tests to change the expected error strings. This is what I meant above.

OK it took me a little while to work out exactly what was going on there. Tests pass now, but are we OK with those new error messages?

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 13550:

Patch Set 14: Run-TryBot+1 Code-Review+1

The new errors seem fine to me, even if they're a bit verbose. Adding Robert and Matthew for a second opinion.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 14:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=28fd2186

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 14: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5015:

Patch Set 14: Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/123956.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/123956 has been merged.

@gopherbot gopherbot closed this Aug 28, 2018
gopherbot pushed a commit that referenced this pull request Aug 28, 2018
@dave @robpike
fmt: fix incorrect format of whole-number floats when using %#v
7c7cecc 
This fixes the unwanted behaviour where printing a zero float with the
#v fmt verb outputs "0" - e.g. missing the trailing decimal. This means
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".

Fixes #26363

Change-Id: Ic5c060522459cd5ce077675d47c848b22ddc34fa
GitHub-Last-Rev: adfb061
GitHub-Pull-Request: #26383
Reviewed-on: https://go-review.googlesource.com/123956
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

proposal: fmt: append decimals to whole-number floats when using '%#v'
3 participants
@dave @gopherbot @googlebot