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

cmd/gofmt: gofmt -r should preserve attached comments through a rewrite #7417

Open
griesemer opened this issue Feb 26, 2014 · 4 comments
Open

cmd/gofmt: gofmt -r should preserve attached comments through a rewrite #7417

griesemer opened this issue Feb 26, 2014 · 4 comments
Assignees
Labels
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Feb 26, 2014

$ cat test.go
package p

var x = (foo /* foo */ + bar /* bar */)

$ gofmt -r 'foo->bla' test.go 
package p

var x = (bla + bar /* bar */)

Instead it should print:

package p

var x = (bla /* foo */ + bar /* bar */)
@griesemer griesemer added new labels Feb 26, 2014
@griesemer griesemer self-assigned this Feb 26, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed release-none labels Apr 10, 2015
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 20, 2015

This just bit me trying to use "gofmt -r" while working on issue #10055:

-       case ld.R_PLT0: // add ip, pc, #0xXX00000
+       case obj.R_PLT0:
mdempsky added a commit that referenced this issue Apr 20, 2015
The majority of this CL was prepared via scripted invocations of
`gofmt -w -r "$SYM -> obj.$SYM" cmd/internal/ld/*.go` and `gofmt -w -r
"ld.$SYM -> obj.$SYM" cmd/?l/*.go`.

Because of issue #7417, that was followed by repeatedly running an AWK
script to identify lines that differed other than whitespace changes
or "ld." or "obj." prefixes and manually restoring comments.

Finally, the redundant constants from cmd/internal/ld/link.go were
removed, and "goimports -w" was used to cleanup import lines.

Passes rsc.io/toolstash/buildall, even when modified to also build cmd.

Fixes #10055.

Change-Id: Icd5dbe819a3b6520ce883748e60017dc8e9a2e85
Reviewed-on: https://go-review.googlesource.com/9112
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dgryski
Copy link
Contributor

@dgryski dgryski commented Feb 17, 2018

Another test case. I was refactoring an example and the replacement happened on the line just before the // Output: comment, causing the entire output comment block to be removed.

$ cat p_test.go
package main

import (
	"bytes"
	"fmt"
)

func ExampleFoo() {
	var a bytes.Buffer
	fmt.Fprintf(&a, "a123")
	fmt.Println(string(a.Bytes()))

	// Output:
	// a123
}

$ gofmt -d -r 'string(b.Bytes()) -> b.String()' p_test.go
diff -u p_test.go.orig p_test.go
--- p_test.go.orig	2018-02-17 09:52:32.000000000 -0800
+++ p_test.go	2018-02-17 09:52:32.000000000 -0800
@@ -8,8 +8,6 @@
 func ExampleFoo() {
 	var a bytes.Buffer
 	fmt.Fprintf(&a, "a123")
-	fmt.Println(string(a.Bytes()))
+	fmt.Println(a.String())

-	// Output:
-	// a123
 }
@andybons andybons added the NeedsFix label Mar 8, 2018
@rillig
Copy link
Contributor

@rillig rillig commented Jul 14, 2019

Another test case:

package main

// comment
@rillig
Copy link
Contributor

@rillig rillig commented Dec 2, 2019

And another test case, taken from my gobco project:

package main

//go:generate go run build/gen.go

Reproducible with go1.14 bf3ee57 from today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.