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

Improve the support for :lang() #89

Merged
merged 8 commits into from
Sep 25, 2016
Merged

Improve the support for :lang() #89

merged 8 commits into from
Sep 25, 2016

Conversation

hediyi
Copy link
Contributor

@hediyi hediyi commented Sep 22, 2016

The scope "meta.language-ranges.css" seems like unnecessary complexity, is it ok to remove it?

About entity.other.attribute-name.pseudo-class.ui-state.css,

* the name is not accurate in that it equates UI with input controls
* some items (e.g., :required) are not about state

therefore it cannot be justified to be a separate entry.
@hediyi
Copy link
Contributor Author

hediyi commented Sep 24, 2016

@50Wliu ready to 🔥

'end': '\\)'
'endCaptures':
'0':
'name': 'punctuation.section.function.css'
'patterns': [
{
'include': '#selector'
'match': '(?i)(?<=[(,\\s])[a-z]+(-[a-z0-9]*+)*+(?=[),\\s])'
Copy link
Contributor

@winstliu winstliu Sep 24, 2016

Choose a reason for hiding this comment

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

I would prefer the explicit [A-Za-z] rather than doing it through (?i) here and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be better 😆

link|target|visited # location
| active|focus|hover # user action
| default|disabled|enabled|read-only|read-write # input control state
| checked|indeterminate # input value state
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we lose the extra .ui-state class here. Do you think that's important?

Copy link
Contributor Author

@hediyi hediyi Sep 25, 2016

Choose a reason for hiding this comment

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

As I said in the commit msg of ee52174, this name "ui-state" itself doesn't make sense, that's why I want to remove it. And I think it should be safe even for themes that are relying on this class coz the related properties will still get the "pseudo-class" class.

@hediyi
Copy link
Contributor Author

hediyi commented Sep 25, 2016

Do you think "meta.language-ranges.css" is necessary here?

@winstliu
Copy link
Contributor

Thought I commented about that. I guess not.

No, I don't think it's necessary. This isn't really something that can be autocompleted.

@winstliu winstliu merged commit 55a44b2 into atom:master Sep 25, 2016
@hediyi hediyi deleted the improve-lang branch September 28, 2016 00:16
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.

2 participants