gofmt: accept program fragments on standard input
This makes it possible to grab a block of code
in an editor and pipe it through gofmt, instead of
having to pipe in the entire file.
On Mon, Sep 12, 2011 at 14:52, <gri@golang.org> wrote: > http://codereview.appspot.com/4973074/diff/4001/src/cmd/gofmt/gofmt.go#newcode281 > src/cmd/gofmt/gofmt.go:281: psrc := ...
13 years, 7 months ago
(2011-09-12 19:12:13 UTC)
#3
On Mon, Sep 12, 2011 at 14:52, <gri@golang.org> wrote:
>
http://codereview.appspot.com/4973074/diff/4001/src/cmd/gofmt/gofmt.go#newcod...
> src/cmd/gofmt/gofmt.go:281: psrc := append([]byte("package p;"), src...)
> make "package p;" a constant and use it below (as an aside, here you are
> using "p;" below you are writing "p\n")
They're different. The psrc version is meant to do its
thing without affecting the line numbers, hence the
semicolon. The part being cut off is known to be
gofmt-formatted, hence the newline. Here they
end up with the same length, and below it turns out
that they do too, but mostly just a coincidence.
I added comments explaining the forms.
>
http://codereview.appspot.com/4973074/diff/4001/src/cmd/gofmt/gofmt.go#newcod...
> src/cmd/gofmt/gofmt.go:301: file, err = parser.ParseFile(fset, filename,
> fsrc, parserMode)
> The code here that follows is essentially the same pattern as above
> (just that you have both a prefix (package p; func _() {) and a suffix
> to insert. Seems like one should be able to factor this out and have
> logic only once (with an empty suffix for the former)?
Possibly. The adjust function is different because it has to
strip leading indent. You could generalize it, but since there
are only two cases I think it would end up being basically
if case1 { } vs if case2 { } and might not be any clearer.
I added comments in the second adjust function to make
it clear how it's different compared to the first one.
>
http://codereview.appspot.com/4973074/diff/4001/src/cmd/gofmt/gofmt.go#newcod...
> src/cmd/gofmt/gofmt.go:335: indent, _, after := cutSpace(orig)
> I think here s/indent/before/
Much nicer. I was able to drop the if too.
Russ
great. i looked at doing this a while ago and failed to come up with ...
13 years, 7 months ago
(2011-09-12 21:00:25 UTC)
#7
great. i looked at doing this a while ago and failed to come
up with a nice way of doing it.
On 12 September 2011 19:11, <rsc@golang.org> wrote:
> Reviewers: gri,
>
> Message:
> Hello gri (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg
>
>
> Description:
> gofmt: accept program fragments on standard input
>
> This makes it possible to grab a block of code
> in an editor and pipe it through gofmt, instead of
> having to pipe in the entire file.
>
> Please review this at http://codereview.appspot.com/4973074/
>
> Affected files:
> M src/cmd/gofmt/doc.go
> M src/cmd/gofmt/gofmt.go
> M src/cmd/gofmt/gofmt_test.go
> A src/cmd/gofmt/testdata/stdin1.golden
> A src/cmd/gofmt/testdata/stdin1.golden.gofmt
> A src/cmd/gofmt/testdata/stdin1.input
> A src/cmd/gofmt/testdata/stdin1.input.gofmt
> A src/cmd/gofmt/testdata/stdin2.golden
> A src/cmd/gofmt/testdata/stdin2.golden.gofmt
> A src/cmd/gofmt/testdata/stdin2.input
> A src/cmd/gofmt/testdata/stdin2.input.gofmt
> A src/cmd/gofmt/testdata/stdin3.golden
> A src/cmd/gofmt/testdata/stdin3.golden.gofmt
> A src/cmd/gofmt/testdata/stdin3.input
> A src/cmd/gofmt/testdata/stdin3.input.gofmt
> A src/cmd/gofmt/testdata/stdin4.golden
> A src/cmd/gofmt/testdata/stdin4.golden.gofmt
> A src/cmd/gofmt/testdata/stdin4.input
> A src/cmd/gofmt/testdata/stdin4.input.gofmt
>
>
>
Why were the *.gofmt files added to vcs? Aren't they just temporary files written out ...
10 years, 8 months ago
(2014-08-21 06:43:40 UTC)
#8
Message was sent while issue was closed.
Why were the *.gofmt files added to vcs? Aren't they just temporary files
written out when
output of gofmt on *.input doesn't match the corresponding *.golden (i.e., tests
failing)?
It is confusing to see 4 files per test case (.input, .input.gofmt, .golden,
.golden.gofmt), and
took me a while to figure out when I wanted to add a new test case.
I think the *.gofmt files should be removed, unless there's a good reason for
them
to exist, or if I misunderstood their purpose.
On 2011/09/12 21:00:25, rog wrote:
> great. i looked at doing this a while ago and failed to come
> up with a nice way of doing it.
>
> On 12 September 2011 19:11, <mailto:rsc@golang.org> wrote:
> > Reviewers: gri,
> >
> > Message:
> > Hello gri (cc: mailto:golang-dev@googlegroups.com),
> >
> > I'd like you to review this change to
> > https://go.googlecode.com/hg
> >
> >
> > Description:
> > gofmt: accept program fragments on standard input
> >
> > This makes it possible to grab a block of code
> > in an editor and pipe it through gofmt, instead of
> > having to pipe in the entire file.
> >
> > Please review this at http://codereview.appspot.com/4973074/
> >
> > Affected files:
> > M src/cmd/gofmt/doc.go
> > M src/cmd/gofmt/gofmt.go
> > M src/cmd/gofmt/gofmt_test.go
> > A src/cmd/gofmt/testdata/stdin1.golden
> > A src/cmd/gofmt/testdata/stdin1.golden.gofmt
> > A src/cmd/gofmt/testdata/stdin1.input
> > A src/cmd/gofmt/testdata/stdin1.input.gofmt
> > A src/cmd/gofmt/testdata/stdin2.golden
> > A src/cmd/gofmt/testdata/stdin2.golden.gofmt
> > A src/cmd/gofmt/testdata/stdin2.input
> > A src/cmd/gofmt/testdata/stdin2.input.gofmt
> > A src/cmd/gofmt/testdata/stdin3.golden
> > A src/cmd/gofmt/testdata/stdin3.golden.gofmt
> > A src/cmd/gofmt/testdata/stdin3.input
> > A src/cmd/gofmt/testdata/stdin3.input.gofmt
> > A src/cmd/gofmt/testdata/stdin4.golden
> > A src/cmd/gofmt/testdata/stdin4.golden.gofmt
> > A src/cmd/gofmt/testdata/stdin4.input
> > A src/cmd/gofmt/testdata/stdin4.input.gofmt
> >
> >
> >
Dmitri; you are absolutely correct - thanks for pointing this out, I missed it: https://codereview.appspot.com/131030044 ...
10 years, 8 months ago
(2014-08-21 21:29:49 UTC)
#9
Dmitri;
you are absolutely correct - thanks for pointing this out, I missed it:
https://codereview.appspot.com/131030044
- gri
On Wed, Aug 20, 2014 at 11:43 PM, <shurcooL@gmail.com> wrote:
> Why were the *.gofmt files added to vcs? Aren't they just temporary
> files written out when
> output of gofmt on *.input doesn't match the corresponding *.golden
> (i.e., tests failing)?
>
> It is confusing to see 4 files per test case (.input, .input.gofmt,
> .golden, .golden.gofmt), and
> took me a while to figure out when I wanted to add a new test case.
>
> I think the *.gofmt files should be removed, unless there's a good
> reason for them
> to exist, or if I misunderstood their purpose.
>
>
> On 2011/09/12 21:00:25, rog wrote:
>
>> great. i looked at doing this a while ago and failed to come
>> up with a nice way of doing it.
>>
>
> On 12 September 2011 19:11, <mailto:rsc@golang.org> wrote:
>> > Reviewers: gri,
>> >
>> > Message:
>> > Hello gri (cc: mailto:golang-dev@googlegroups.com),
>> >
>> > I'd like you to review this change to
>> > https://go.googlecode.com/hg
>> >
>> >
>> > Description:
>> > gofmt: accept program fragments on standard input
>> >
>> > This makes it possible to grab a block of code
>> > in an editor and pipe it through gofmt, instead of
>> > having to pipe in the entire file.
>> >
>> > Please review this at http://codereview.appspot.com/4973074/
>> >
>> > Affected files:
>> > M src/cmd/gofmt/doc.go
>> > M src/cmd/gofmt/gofmt.go
>> > M src/cmd/gofmt/gofmt_test.go
>> > A src/cmd/gofmt/testdata/stdin1.golden
>> > A src/cmd/gofmt/testdata/stdin1.golden.gofmt
>> > A src/cmd/gofmt/testdata/stdin1.input
>> > A src/cmd/gofmt/testdata/stdin1.input.gofmt
>> > A src/cmd/gofmt/testdata/stdin2.golden
>> > A src/cmd/gofmt/testdata/stdin2.golden.gofmt
>> > A src/cmd/gofmt/testdata/stdin2.input
>> > A src/cmd/gofmt/testdata/stdin2.input.gofmt
>> > A src/cmd/gofmt/testdata/stdin3.golden
>> > A src/cmd/gofmt/testdata/stdin3.golden.gofmt
>> > A src/cmd/gofmt/testdata/stdin3.input
>> > A src/cmd/gofmt/testdata/stdin3.input.gofmt
>> > A src/cmd/gofmt/testdata/stdin4.golden
>> > A src/cmd/gofmt/testdata/stdin4.golden.gofmt
>> > A src/cmd/gofmt/testdata/stdin4.input
>> > A src/cmd/gofmt/testdata/stdin4.input.gofmt
>> >
>> >
>> >
>>
>
>
> https://codereview.appspot.com/4973074/
>
I'm very happy to be useful! Thank you for fixing it now. On 2014/08/21 21:29:49, ...
10 years, 8 months ago
(2014-08-21 22:23:39 UTC)
#10
Message was sent while issue was closed.
I'm very happy to be useful! Thank you for fixing it now.
On 2014/08/21 21:29:49, gri wrote:
> Dmitri;
>
> you are absolutely correct - thanks for pointing this out, I missed it:
>
> https://codereview.appspot.com/131030044
>
> - gri
>
>
> On Wed, Aug 20, 2014 at 11:43 PM, <mailto:shurcooL@gmail.com> wrote:
>
> > Why were the *.gofmt files added to vcs? Aren't they just temporary
> > files written out when
> > output of gofmt on *.input doesn't match the corresponding *.golden
> > (i.e., tests failing)?
> >
> > It is confusing to see 4 files per test case (.input, .input.gofmt,
> > .golden, .golden.gofmt), and
> > took me a while to figure out when I wanted to add a new test case.
> >
> > I think the *.gofmt files should be removed, unless there's a good
> > reason for them
> > to exist, or if I misunderstood their purpose.
> >
> >
> > On 2011/09/12 21:00:25, rog wrote:
> >
> >> great. i looked at doing this a while ago and failed to come
> >> up with a nice way of doing it.
> >>
> >
> > On 12 September 2011 19:11, <mailto:rsc@golang.org> wrote:
> >> > Reviewers: gri,
> >> >
> >> > Message:
> >> > Hello gri (cc: mailto:golang-dev@googlegroups.com),
> >> >
> >> > I'd like you to review this change to
> >> > https://go.googlecode.com/hg
> >> >
> >> >
> >> > Description:
> >> > gofmt: accept program fragments on standard input
> >> >
> >> > This makes it possible to grab a block of code
> >> > in an editor and pipe it through gofmt, instead of
> >> > having to pipe in the entire file.
> >> >
> >> > Please review this at http://codereview.appspot.com/4973074/
> >> >
> >> > Affected files:
> >> > M src/cmd/gofmt/doc.go
> >> > M src/cmd/gofmt/gofmt.go
> >> > M src/cmd/gofmt/gofmt_test.go
> >> > A src/cmd/gofmt/testdata/stdin1.golden
> >> > A src/cmd/gofmt/testdata/stdin1.golden.gofmt
> >> > A src/cmd/gofmt/testdata/stdin1.input
> >> > A src/cmd/gofmt/testdata/stdin1.input.gofmt
> >> > A src/cmd/gofmt/testdata/stdin2.golden
> >> > A src/cmd/gofmt/testdata/stdin2.golden.gofmt
> >> > A src/cmd/gofmt/testdata/stdin2.input
> >> > A src/cmd/gofmt/testdata/stdin2.input.gofmt
> >> > A src/cmd/gofmt/testdata/stdin3.golden
> >> > A src/cmd/gofmt/testdata/stdin3.golden.gofmt
> >> > A src/cmd/gofmt/testdata/stdin3.input
> >> > A src/cmd/gofmt/testdata/stdin3.input.gofmt
> >> > A src/cmd/gofmt/testdata/stdin4.golden
> >> > A src/cmd/gofmt/testdata/stdin4.golden.gofmt
> >> > A src/cmd/gofmt/testdata/stdin4.input
> >> > A src/cmd/gofmt/testdata/stdin4.input.gofmt
> >> >
> >> >
> >> >
> >>
> >
> >
> > https://codereview.appspot.com/4973074/
> >
Issue 4973074: code review 4973074: gofmt: accept program fragments on standard input
(Closed)
Created 13 years, 7 months ago by rsc
Modified 10 years, 8 months ago
Reviewers: rog, shurcooL
Base URL:
Comments: 8
