code review 154820043: fmt: part 2 of the great flag rebuild: make %+v work in...

fmt: part 2 of the great flag rebuild: make %+v work in formatters
Apply a similar transformation to %+v that we did to %#v, making it
a top-level setting separate from the + flag itself. This fixes the
appearance of flags in Formatters and cleans up the code too,
probably making it a little faster.

Fixes issue 8835.

[r, 2014-10-03 18:52:31]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[iant, 2014-10-03 20:08:38]

LGTM

https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go
File src/fmt/format.go (right):

https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go#newcode45
src/fmt/format.go:45: // For the formats %+v %#v, we set these flags and
In the first CL, it took me a while to figure out that "these flags" refers only
to plusV and sharpV (hindered by the fact that the first CL didn't hvae plusV
yet).  Perhaps "we set the plusV and sharpV flags".  Or add a blank line before
the command and after sharpV to make it clear.

[r, 2014-10-03 20:23:39]

*** Submitted as https://code.google.com/p/go/source/detail?r=bc0fe81c7252 ***

fmt: part 2 of the great flag rebuild: make %+v work in formatters
Apply a similar transformation to %+v that we did to %#v, making it
a top-level setting separate from the + flag itself. This fixes the
appearance of flags in Formatters and cleans up the code too,
probably making it a little faster.

Fixes issue 8835.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/154820043

https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go
File src/fmt/format.go (right):

https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go#newcode45
src/fmt/format.go:45: // For the formats %+v %#v, we set these flags and
On 2014/10/03 20:08:37, iant wrote:
> In the first CL, it took me a while to figure out that "these flags" refers
only
> to plusV and sharpV (hindered by the fact that the first CL didn't hvae plusV
> yet).  Perhaps "we set the plusV and sharpV flags".  Or add a blank line
before
> the command and after sharpV to make it clear.

Done.

[mtj1, 2014-10-03 20:36:55]

Is this something to implement in Big.Int?

On Fri, Oct 3, 2014 at 1:23 PM, <r@golang.org> wrote:

> *** Submitted as
> https://code.google.com/p/go/source/detail?r=bc0fe81c7252 ***
>
> fmt: part 2 of the great flag rebuild: make %+v work in formatters
> Apply a similar transformation to %+v that we did to %#v, making it
> a top-level setting separate from the + flag itself. This fixes the
> appearance of flags in Formatters and cleans up the code too,
> probably making it a little faster.
>
> Fixes issue 8835.
>
> LGTM=iant
> R=golang-codereviews, iant
> CC=golang-codereviews
> https://codereview.appspot.com/154820043
>
>
>
> https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go
> File src/fmt/format.go (right):
>
> https://codereview.appspot.com/154820043/diff/20001/src/
> fmt/format.go#newcode45
> src/fmt/format.go:45: // For the formats %+v %#v, we set these flags and
> On 2014/10/03 20:08:37, iant wrote:
>
>> In the first CL, it took me a while to figure out that "these flags"
>>
> refers only
>
>> to plusV and sharpV (hindered by the fact that the first CL didn't
>>
> hvae plusV
>
>> yet).  Perhaps "we set the plusV and sharpV flags".  Or add a blank
>>
> line before
>
>> the command and after sharpV to make it clear.
>>
>
> Done.
>
>
> https://codereview.appspot.com/154820043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>



-- 
*Michael T. Jones | Chief Technology Advocate  | mtj@google.com
<mtj@google.com> |  +1 650-335-5765*

[r, 2014-10-03 20:47:04]

No, it's just a bug fix in how flags are handled inside the fmt
package. Before this change the + and # flags weren't visible when
attached to the v verb when the item was an array of Formatters and
some other similar corner cases. The code is just cleaned up to remove
the bug categorically.

-rob


On Fri, Oct 3, 2014 at 1:36 PM, Michael Jones <mtj@google.com> wrote:
> Is this something to implement in Big.Int?
>
> On Fri, Oct 3, 2014 at 1:23 PM, <r@golang.org> wrote:
>>
>> *** Submitted as
>> https://code.google.com/p/go/source/detail?r=bc0fe81c7252 ***
>>
>> fmt: part 2 of the great flag rebuild: make %+v work in formatters
>> Apply a similar transformation to %+v that we did to %#v, making it
>> a top-level setting separate from the + flag itself. This fixes the
>> appearance of flags in Formatters and cleans up the code too,
>> probably making it a little faster.
>>
>> Fixes issue 8835.
>>
>> LGTM=iant
>> R=golang-codereviews, iant
>> CC=golang-codereviews
>> https://codereview.appspot.com/154820043
>>
>>
>>
>> https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go
>> File src/fmt/format.go (right):
>>
>>
>>
https://codereview.appspot.com/154820043/diff/20001/src/fmt/format.go#newcode45
>> src/fmt/format.go:45: // For the formats %+v %#v, we set these flags and
>> On 2014/10/03 20:08:37, iant wrote:
>>>
>>> In the first CL, it took me a while to figure out that "these flags"
>>
>> refers only
>>>
>>> to plusV and sharpV (hindered by the fact that the first CL didn't
>>
>> hvae plusV
>>>
>>> yet).  Perhaps "we set the plusV and sharpV flags".  Or add a blank
>>
>> line before
>>>
>>> the command and after sharpV to make it clear.
>>
>>
>> Done.
>>
>>
>> https://codereview.appspot.com/154820043/
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "golang-codereviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-codereviews+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --
> Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
> 650-335-5765