Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Correct scope for var keyword #34

Merged
merged 1 commit into from
Sep 4, 2015
Merged

Conversation

pchaigno
Copy link
Contributor

As reported in #33, the current scope for var is incorrect. This PR changes it to storage.type.source.cs.

@MaximSokolov
Copy link
Contributor

It's weird. I didn't see source scope for symbols in other languages. source scope is usually "root scope" like on the screenshot.
image
I created an issue #35

Also add var please
I think it should be storage.type.source.cs -> storage.type.var.cs.

@MaximSokolov MaximSokolov mentioned this pull request Aug 26, 2015
1 task
@winstliu
Copy link
Contributor

I agree. Those source scopes seem weird.

@pchaigno
Copy link
Contributor Author

I didn't see source scope for language symbols in other languages.

I only added it to stay coherent with the other scopes. I find it weird too.

I think it should be storage.type.source.cs -> storage.type.var.cs.

Why var? The examples in the TextMate naming conventions are always storage.type.language-name.

@MaximSokolov
Copy link
Contributor

I only added it to stay coherent with the other scopes.

I see

The examples in the TextMate naming conventions are always storage.type.language-name

More subscopes let us more possibility of customization. Yes, there is no storage.type.var in the TextMate but also there is:

...You should however append as much information to the sub-type you choose...

TextMate docs are very unclear. I created an issue before writing clear docs for Atom. My bad.

@pchaigno
Copy link
Contributor Author

More subscopes let us more possibility of customization.

Okay, makes sense.

@pchaigno
Copy link
Contributor Author

pchaigno commented Sep 3, 2015

@50Wliu I think this is ready :)

@@ -263,6 +263,10 @@
'name': 'keyword.other.source.cs'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Var needs to be removed in the match above this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks!

winstliu pushed a commit that referenced this pull request Sep 4, 2015
Correct scope for var keyword
@winstliu winstliu merged commit eed77b7 into atom:master Sep 4, 2015
@pchaigno pchaigno deleted the scope-var branch September 4, 2015 20:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants