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

playground: goimports can delete what user has typed #6947

Closed
cznic opened this issue Dec 13, 2013 · 4 comments
Closed

playground: goimports can delete what user has typed #6947

cznic opened this issue Dec 13, 2013 · 4 comments

Comments

@cznic
Copy link
Contributor

cznic commented Dec 13, 2013

1.
Imagine yourself coming to play.golang.org for the first time.

2.
Trying to experiment with something from package time, you edit the imports to include
"time": http://play.golang.org/p/CZpUiGyVho

3.
Noticing the "Format" button, you press it.

What is the expected output?
http://play.golang.org/p/-oPFe90KmG


What do you see instead?
http://play.golang.org/p/duRF5gXJEP


Please provide any additional information below.

The intent of goimports is good and everything - but only in the hands of a well
informed hacker who welcomes what goimports do (I don't). For a newbie coming to
playground, it's inexplicable why the "Format" button "eats" her
edits so far.

- There's no information and/or warning about this behavior available to the
playground's user.

- Moreover, "Format" is probably rightfully expected to change only the white
space of a program, not its semantics.

I have missed the change on the dev-list and was thus genuinely surprised the same as
any dev-list non reader could/would possibly be.

Suggestions:

- Remove the goimports functionality.
- Or make it a standalone button.
- Or disable/enable goimports by a check box.
- Avoid the confusion by providing a prominently visible information that
"Format" will remove your unused imports (and insert missing used ones, but
that's a welcome "surprise").

----
I personally avoid any automatic source changing "on save" for good (for me)
reasons. It's just my personal preference and no one is pushing such thing on me, so no
problem; neither for me nor anyone else. However, at the playground we are now forcing
such thing on the playground users in a potentially quite confusing way, which I believe
is a good intended mistake.
@robpike
Copy link
Contributor

robpike commented Dec 13, 2013

Comment 1:

I tend to agree. The goimports functionality is great but invasive. It should be
optional at best.

Labels changed: added repo-playground.

Owner changed to @adg.

Status changed to Accepted.

@adg
Copy link
Contributor

adg commented Dec 17, 2013

Comment 2:

Yeah, this is a fair complaint. Sorry for the confusion/annoyance.
I think it should be a checkbox beside the "Format" button, and the UI should store a
browser cookie to remember the checkbox state.

@adg
Copy link
Contributor

adg commented Dec 17, 2013

Comment 3:

https://golang.org/cl/43220043/
https://golang.org/cl/43240043/
bradfitz/goimports#30

Status changed to Started.

@adg
Copy link
Contributor

adg commented Dec 18, 2013

Comment 4:

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned adg Jun 22, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants