code review 7064055: fmt: Remove dead code and make comments and variables c...

fmt: Remove dead code and make comments and variables consistent.

[u, 2013-01-08 15:51:56]

Hello golang-dev@googlegroups.com,

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

[minux1, 2013-01-08 21:08:45]

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go
File src/pkg/fmt/format.go (right):

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
src/pkg/fmt/format.go:110: // Append b to f.buf, padded on left (w > 0) or right
(w < 0 or f.minus).
the style should really be:
// pad appends b to f.buf, ....

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
src/pkg/fmt/format.go:158: // integer; interprets prec but not wid.  Once
formatted, result is sent to
why this change?

[dave_cheney.net, 2013-01-08 23:40:11]

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go
File src/pkg/fmt/format.go (right):

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
src/pkg/fmt/format.go:111: // Clear flags afterwards.
Hmm, i don't like this, it implies that Clear is a method, what about 

   and clears flags afterwards.

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
src/pkg/fmt/format.go:128: // Clear flags afterwards.
See above, 

// padString appends s to buf ... and clears flags afterwards.

https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
src/pkg/fmt/format.go:158: // integer; interprets prec but not wid.  Once
formatted, result is sent to
On 2013/01/08 21:08:45, minux wrote:
> why this change?

looks like the previous line was > 80 chars.

[minux1, 2013-01-08 23:47:58]

On Wednesday, January 9, 2013, dfc wrote:
>
> https://codereview.appspot.**com/7064055/diff/5001/src/pkg/**
>
fmt/format.go#newcode158<https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcode158>
> src/pkg/fmt/format.go:158: // integer; interprets prec but not wid.
> Once formatted, result is sent to
> On 2013/01/08 21:08:45, minux wrote:
>
>> why this change?
>>
>
> looks like the previous line was > 80 chars.
>
this is not critical and i think maintaining a clear and minimum diff
history outweights 80-column display. Generally i will re-wrap the line
only when there is other things to fix on that line.

[u, 2013-01-09 03:15:12]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, dave@cheney.net (cc:
golang-dev@googlegroups.com),

Please take another look.

[u, 2013-01-09 03:22:12]

On 2013/01/08 23:40:11, dfc wrote:
>
https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcod...
> src/pkg/fmt/format.go:111: // Clear flags afterwards.
> Hmm, i don't like this, it implies that Clear is a method, what about 
> 
>    and clears flags afterwards.

I've made the change, but I'm not familiar enough with the fmt package to know
if "Clear flags afterwards" was meant as "Flags should be cleared afterwards, by
the calling function" or if it was meant as "pad clears the flags afterwards".
Do you know which one of these is accurate?

In the code I can't see any clearing of flags done by either pad, padString or
any of the methods that they invoke.

[minux1, 2013-01-09 10:00:48]

On Wed, Jan 9, 2013 at 11:22 AM, <r.eklind.87@gmail.com> wrote:

> On 2013/01/08 23:40:11, dfc wrote:
> https://codereview.appspot.**com/7064055/diff/5001/src/pkg/**
>
fmt/format.go#newcode111<https://codereview.appspot.com/7064055/diff/5001/src/pkg/fmt/format.go#newcode111>
>
>> src/pkg/fmt/format.go:111: // Clear flags afterwards.
>> Hmm, i don't like this, it implies that Clear is a method, what about
>
>      and clears flags afterwards.
>
> In the code I can't see any clearing of flags done by either pad,
> padString or any of the methods that they invoke.
>
Some comments in the fmt package is not very accurate, we need some time
to completely redo those comments.

[rsc, 2013-01-18 20:14:10]

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/format.go
File src/pkg/fmt/format.go (right):

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/format.go#newco...
src/pkg/fmt/format.go:75: // computePadding computes left and right padding
widths (only one will be
unwrap this line

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/format.go#newco...
src/pkg/fmt/format.go:112: // and clears flags afterwards.
delete this line

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/format.go#newco...
src/pkg/fmt/format.go:129: // f.minus) and clears flags afterwards.
delete 'and clears flags afterward's. unwrap line

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

https://codereview.appspot.com/7064055/diff/10001/src/pkg/fmt/scan.go#newcode315
src/pkg/fmt/scan.go:315: // SkipSpace provides Scan() methods the ability to
skip space and newline
While were here drop all the () in this comment.

[u, 2013-01-20 09:41:10]

Hello minux.ma@gmail.com, dave@cheney.net, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2013-01-22 22:12:19]

LGTM

[rsc, 2013-01-22 22:12:50]

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

fmt: Remove dead code and make comments and variables consistent.

R=minux.ma, dave, rsc
CC=golang-dev
https://codereview.appspot.com/7064055

Committer: Russ Cox <rsc@golang.org>

[u, 2013-02-22 08:10:57]

*** Abandoned ***