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

Jump to bottom

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
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@griesemer
Copy link
Contributor 

$ 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 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
@mdempsky
Copy link
Member

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
@mdempsky
cmd/internal/ld: prefer constants from cmd/internal/obj
c776592 
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>
@dominikh dominikh mentioned this issue Jan 3, 2018
Open
@dgryski
Copy link
Contributor

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 The path to resolution is known, but the work has not been done. label Mar 8, 2018
@rillig
Copy link
Contributor

rillig commented Jul 14, 2019

Another test case:

package main

// comment

@rillig
Copy link
Contributor

rillig commented Dec 2, 2019 •
edited

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
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@bradfitz @mdempsky @rsc @andybons @dgryski @rillig @griesemer