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

Moving to one var block missed one variable #212

Closed
Jacalz opened this issue Mar 20, 2022 · 7 comments · Fixed by #214
Closed

Moving to one var block missed one variable #212

Jacalz opened this issue Mar 20, 2022 · 7 comments · Fixed by #214

Comments

@Jacalz
Copy link

Jacalz commented Mar 20, 2022

I was running this project on Fyne, see fyne-io/fyne#2855 for more info. The formatting is, as the PR description says, doing a great job and I am very impressed with it.

However, I noticed that there was one part where it seems to have failed to put all variables inside one block. It managed to get three out of four variables as it seems to somehow have missed the first one.

All necessary information should hopefully be found here: fyne-io/fyne#2855 (comment)

@mvdan
Copy link
Owner

mvdan commented Mar 20, 2022

Thanks for filing this! My guess is that gofumpt saw the comment on the first variable and decided not to group it because of that. But in cases like this one, the comment is for the entire block, so it should be fine.

@cespare
Copy link

cespare commented Mar 26, 2022

I ran gofumpt over a large codebase (thinking of using it at work) and this was the only real issue I noticed.

Oiyoo added a commit to Oiyoo/gofumpt that referenced this issue Mar 30, 2022
Oiyoo added a commit to Oiyoo/gofumpt that referenced this issue Apr 3, 2022
@mvdan mvdan closed this as completed in #214 Apr 3, 2022
mvdan pushed a commit that referenced this issue Apr 3, 2022
@mvdan
Copy link
Owner

mvdan commented Apr 3, 2022

Should be fixed now, thanks to @Oiyoo - please test master and let us know.

@cespare
Copy link

cespare commented Apr 18, 2022

Thanks for the fix! Looks good to me.

@mvdan do you have a particular schedule for releasing? Any chance of a 0.3.2 to include this fix soon?

@mvdan
Copy link
Owner

mvdan commented Apr 19, 2022

I'll probably release in a week or two. It should be possible to pin a newer commit if you need master before then.

@mvdan
Copy link
Owner

mvdan commented Jul 27, 2022

I promise I'm not dead nor have I forgotten about doing a release :) I recognize a release is way overdue, and it's my priority now. Will do that once Go 1.19 is out and I can merge #239, which will significantly help when debugging issues downstream.

@mvdan
Copy link
Owner

mvdan commented Sep 27, 2022

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

Successfully merging a pull request may close this issue.

3 participants