sqs / goreturns

A gofmt/goimports-like tool for Go programmers that fills in Go return statements with zero values to match the func return types
Other
530 stars 55 forks source link

Diff option -d always returns nothing #28

Closed chrisolsen closed 7 years ago

chrisolsen commented 8 years ago

This issue is in regards to vscode no longer adding/removing imports on save. An additional option was added to override the use of goreturns -d and fallback to goreturns -w, which gets things working for me again.

The initial issue: https://github.com/Microsoft/vscode-go/issues/520

I am using:

sqs commented 8 years ago

Thanks for filing. Is that option an effective workaround? What fixes/changes need to be made to goreturns?

dmitshur commented 8 years ago

@chrisolsen, do you have diff command installed on your Windows 10 Home?

-d flag relies on it to perform the diff. See here. If you don't have it, it would fail to work.

dmitshur commented 8 years ago

If my theory above about diff being the issue here, then...

What fixes/changes need to be made to goreturns?

Easy: It should check that diff binary exists, and exit with error if -d flag is used but diff isn't available. Especially since it ignores the exit code from running the command. This wouldn't fix the problem, but at least it'd make it visible.

Hard: Rewrite diff in pure Go and avoid an external dependency. :)

chrisolsen commented 8 years ago

I do have diff.

C:\Users\chris>diff -v
diff - GNU diffutils version 2.7

C:\Users\chris>vim test.go  // see test file

C:\Users\chris>goreturns -d test.go
diff test.go gofmt/test.go    // only output

test.go

package testme

func main() {
    fmt.Println("testing...")  // should add `import "fmt"`
}
dmitshur commented 8 years ago

If it's printing "diff test.go gofmt/test.go", that means it got to this line.

So diff did run. I'd suggest adding some debug printfs to the code to see what's really going on. It could be that your diff command produces unexpected output when given -u flag, or something else.

Does goimports exhibit the same behavior?

chrisolsen commented 8 years ago

goimports does return the same output.

marpaia commented 8 years ago

For what it's worth, I also came across this issue through my usage of VS Code as referenced in https://github.com/Microsoft/vscode-go/issues/520.

ramya-rao-a commented 7 years ago

@shurcooL Any new ideas or updates on this one? The Go extension for VS Code depends on the diff output from goreturns.

dmitshur commented 7 years ago

@ramya-rao-a I don't see evidence in this issue that there's a problem in code. I suspect it was some specific environment issue on that person's computer. According to https://github.com/sqs/goreturns/issues/28#issuecomment-254862010, they said it happens with goimports too.

I tested both goimports and goreturns, -d option works as expected for me on macOS.

I would recommend closing this issue and suggesting to anyone who actually runs into this problem to make a new issue with more information and reproduce steps.

chrisolsen commented 7 years ago

I guess I should chime in :) Early this week I had to re-install Windows and noticed that the problem was no longer causing issues. I assumed a fix was made on your guy's side.

dmitshur commented 7 years ago

Great! I think that confirms it was an environment issue. The only changed merged into goreturns since this issue was opened on Oct 18, 2016 was a new feature. See https://github.com/sqs/goreturns/commits/master.

ramya-rao-a commented 7 years ago

It is interesting that 3 to 4 people would have had the same environment issue :) But yes, it doesnt look like the problem is with goreturns.

@chrisolsen can you close this issue then?