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

SymbolKind for val should not be constant #39

Closed
olafurpg opened this issue Nov 19, 2017 · 7 comments
Closed

SymbolKind for val should not be constant #39

olafurpg opened this issue Nov 19, 2017 · 7 comments

Comments

@olafurpg
Copy link
Member

User is val (icon is kinda ugly in vscode with https://marketplace.visualstudio.com/items?itemName=patrys.vscode-code-outline)
screen shot 2017-11-19 at 17 54 00

User is var
screen shot 2017-11-19 at 17 54 06

I feel that constant is not appropriate for vals since vals can be very stateful objects, constants are IMO literals like "string" and 2

@laughedelic
Copy link
Member

For literals there are SymbolKinds like String, Number, Boolean, Array.

I think that SymbolKinds don't map well on the entities in Scala, but we should try to use more of them just for visual distinction. Here's how val vs. var looks in Atom:

screen shot 2017-11-19 at 18 19 32

I think it kind of makes sense and would be wrong to mark both val and var with the same kind/icon. Icons design is, of course, something editor-specific and could be improved, but the set of symbol kinds is a part of LSP and is harder to change, I think.

@laughedelic
Copy link
Member

Also, here is the full list of symbols/icons in the two editors:

screen shot 2017-11-14 at 01 08 14 screen shot 2017-11-14 at 18 58 06

@ShaneDelmore
Copy link
Collaborator

I would have thought Constant meant single assignment, but it sounds like @olafurpg has the intuition that Constant means literal. They seem to already have icons for String, Number, and Boolean which would cover a good deal of @olafurpg interpretation of Constant (wouldn’t it?) but I don’t see anything else available that covers a single assignment value. Of these admittedly not wonderful options I prefer Constant still, but is it worth proposing another symbol kind to LSP? As for icons, that feels unrelated, we can always contribute better icons.

@ShaneDelmore
Copy link
Collaborator

For what it’s worth, I like the atom icons, visually one looks like it changes over time, and the other looks like it is assigned once to my eye.

@olafurpg
Copy link
Member Author

I guess I'm just unhappy with the icon for outline types in the vscode plugin for "Constant", in atom it looks more sensible. I agree we should have a different symbol kind of vars and vals.

@laughedelic
Copy link
Member

Check patrys/vscode-code-outline#4 and microsoft/vscode#28592: the imcons have to be iproved in the VSCode itself. I tried to search a bit related issues in the vscode repo and I got an impression that it's not considered an big problem.. 🙁
Also related: microsoft/vscode#2628 (didn't read completely, but it's about having more kinds) and microsoft/vscode#23927 (about modifiers for symbol kinds)

@olafurpg
Copy link
Member Author

I can live with it 😸

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

No branches or pull requests

3 participants