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

[semantic tokens] Use singular for token type names #86281

Closed
alexdima opened this issue Dec 4, 2019 · 5 comments
Closed

[semantic tokens] Use singular for token type names #86281

alexdima opened this issue Dec 4, 2019 · 5 comments
Assignees
Labels
feature-request Request for new features or functionality semantic-tokens Semantic tokens issues
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Dec 4, 2019

Testing #85972

I have written quite a number of parsers, and so far I never used terms like this is "comments", this is "classes", etc.

I find the choice to use plurals in built-in token types a bit surprising.

@aeschli
Copy link
Contributor

aeschli commented Dec 5, 2019

It's more related to styling

I chose plural as we already have the setting

	"editor.tokenColorCustomizations": {
		"comments": "#FF0000",
		"keywords": "#FF0000",
		"strings": "#FF0000",
		"types": "#FF0000",
		"functions": "#FF0000",
		"numbers": "#FF0000",
		"variables": "#FF0000"
	},

and my goal is to bring this together.

@alexdima
Copy link
Member Author

alexdima commented Dec 5, 2019

I see how this was driven, and in the coloring context that makes sense: "I want to color comments in red". But I still think writing a parser and returning plurals is very different than what I have seen/done so far. Could we look at some language ASTs, is anyone using plurals?

@aeschli
Copy link
Contributor

aeschli commented Dec 5, 2019

The token names are not user facing, while the settings are. So while I agree that plurals are unconventional and even a bit weird, are not a real problem.

That said, deprecating the current setting names is also possible.

@aeschli aeschli added feature-request Request for new features or functionality semantic-tokens Semantic tokens issues labels Dec 5, 2019
@aeschli aeschli changed the title Plurals are weird [semantic tokens] Use singular for token type names Dec 5, 2019
@aeschli aeschli added this to the Backlog milestone Dec 5, 2019
@alexdima
Copy link
Member Author

alexdima commented Dec 5, 2019

@aeschli Two final points.

  1. IMHO the tokens that we are proposing impact more than VS Code since I think we want to add them to the LSP, so using plurals because of an existing VS Code configuration option is a bit forcing...

  2. Interesting, the same problem exists in CSS. Given the HTML:

<div class="title">Title 1</div>
<div class="content">Content 1</div>
<div class="title">Title 2</div>
<div class="content">Content 2</div>

The CSS is .title and not the plural .titles even if multiple elements are targeted:

.title { color: red }

@aeschli aeschli modified the milestones: Backlog, November 2019 Dec 6, 2019
@aeschli
Copy link
Contributor

aeschli commented Dec 6, 2019

Making the change as we want to mention the new API in the release notes so that others can try it out.

@aeschli aeschli closed this as completed in 54151bb Dec 6, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality semantic-tokens Semantic tokens issues
Projects
None yet
Development

No branches or pull requests

2 participants