Languages: CSS: Added CSS3 support #434

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@Defman21
Contributor

Defman21 commented Jul 18, 2015

Please review at as far as you can 馃槂. If you find something wrong - let me know, I'll fix that If I can.

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

Forgot to say. This fixes #343

Contributor

Defman21 commented Jul 18, 2015

Forgot to say. This fixes #343

@Naatan

View changes

src/codeintel/lib/codeintel2/lang_css.py
@@ -77,6 +78,7 @@
lang = "CSS"
log = logging.getLogger("codeintel.css")
+log.setLevel(logging.DEBUG)

This comment has been minimized.

@Naatan

Naatan Jul 18, 2015

Member

Turn off please :)

@Naatan

Naatan Jul 18, 2015

Member

Turn off please :)

This comment has been minimized.

@Defman21

Defman21 Jul 18, 2015

Contributor

Everything has been fixed.

@Defman21

Defman21 Jul 18, 2015

Contributor

Everything has been fixed.

@Naatan

View changes

src/codeintel/lib/codeintel2/lang_css.py
@@ -233,7 +235,7 @@ def ignore_styles(self):
return (ScintillaConstants.SCE_CSS_DEFAULT,
ScintillaConstants.SCE_CSS_COMMENT)
-DebugStatus = False
+DebugStatus = True

This comment has been minimized.

@Naatan

Naatan Jul 18, 2015

Member

Same here

@Naatan

Naatan Jul 18, 2015

Member

Same here

@Naatan

View changes

src/codeintel/lib/codeintel2/lang_css.py
@@ -465,7 +467,8 @@ def CSS_PSEUDO_CLASS_NAMES(self):
@LazyClassAttribute
def CSS_AT_RULE_NAMES(self):
# at rules
- return sorted(["import", "media", "charset", "font-face", "page", "namespace"],
+ css3_rule_names = ["keyframes"]
+ return sorted(["import", "media", "charset", "font-face", "page", "namespace"] + css3_rule_names,

This comment has been minimized.

@Naatan

Naatan Jul 18, 2015

Member

Why don;t you just modify the list?

@Naatan

Naatan Jul 18, 2015

Member

Why don;t you just modify the list?

@Naatan

View changes

src/languages/koCSSLanguage.py
@@ -52,6 +53,14 @@ class koCSSCommonLanguage(KoLanguageBase):
searchURL = "http://www.google.com/search?q=site%3Ahttp%3A%2F%2Fwww.w3schools.com%2Fcss+%W"
sample = """
+

This comment has been minimized.

@Naatan

Naatan Jul 18, 2015

Member

I dont think this should be part of your PR? Same for the bit below.

For one thing /fonts wont exist. And these modifications seem pointless.

@Naatan

Naatan Jul 18, 2015

Member

I dont think this should be part of your PR? Same for the bit below.

For one thing /fonts wont exist. And these modifications seem pointless.

This comment has been minimized.

@Defman21

Defman21 Jul 18, 2015

Contributor

I thought it would be better to mention all most-used CSS keywords/properties/etc. in the sample to make the scheme CSS syntax modifying more understandable

@Defman21

Defman21 Jul 18, 2015

Contributor

I thought it would be better to mention all most-used CSS keywords/properties/etc. in the sample to make the scheme CSS syntax modifying more understandable

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 18, 2015

Member

This should not be part of your PR (or any PR):

util/docker/id_rsa.pub 100644 鈫 100755

All you are doing is adding some gradient completions and a keyframe keyword, but the PR is huge due to all the debug code. I'm not against adding this in but I'd like you to just PR the functional changes, not all the debug code changes.

I was under the impression that you were adding a lot more though, are you sure this is the right PR?

Member

Naatan commented Jul 18, 2015

This should not be part of your PR (or any PR):

util/docker/id_rsa.pub 100644 鈫 100755

All you are doing is adding some gradient completions and a keyframe keyword, but the PR is huge due to all the debug code. I'm not against adding this in but I'd like you to just PR the functional changes, not all the debug code changes.

I was under the impression that you were adding a lot more though, are you sure this is the right PR?

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

Yes, it's the right pr and github just don't show you the changes I did in constants_css3.py

Contributor

Defman21 commented Jul 18, 2015

Yes, it's the right pr and github just don't show you the changes I did in constants_css3.py

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 18, 2015

Member

I highly doubt github just isnt showing changes, more likely this isnt the right PR.

Member

Naatan commented Jul 18, 2015

I highly doubt github just isnt showing changes, more likely this isnt the right PR.

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

screen

Contributor

Defman21 commented Jul 18, 2015

screen

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 18, 2015

Member

Oh wow, thats ridiculous. Wtf github. I'll have to review this manually then. Gonna do so on Monday.

Member

Naatan commented Jul 18, 2015

Oh wow, thats ridiculous. Wtf github. I'll have to review this manually then. Gonna do so on Monday.

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

Just too much changes that GitHub cannot render in a proper way?

Contributor

Defman21 commented Jul 18, 2015

Just too much changes that GitHub cannot render in a proper way?

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 18, 2015

Member

Probably, but it doesn't even offer you an explanation. Bad UX.

Member

Naatan commented Jul 18, 2015

Probably, but it doesn't even offer you an explanation. Bad UX.

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

Should I merge my last commits into one?

Contributor

Defman21 commented Jul 18, 2015

Should I merge my last commits into one?

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 18, 2015

Member

If you dont mind, that would be much appreciated.

Member

Naatan commented Jul 18, 2015

If you dont mind, that would be much appreciated.

Languages: CSS: Added CSS3 properties support
Languages: CSS: disabled debugging, moved 'keyframes' to the actual list of keywords instead of creating a variable for it

Undo changing permissions on id_rsa.pub

Languages: CSS: undo changing the sample
@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 18, 2015

Contributor

Done 馃槂

Contributor

Defman21 commented Jul 18, 2015

Done 馃槂

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 20, 2015

Member

Thanks Defman :) I reverted your debug statements (that should not be part of this PR) but other than that everything is merged. Job well done!

Member

Naatan commented Jul 20, 2015

Thanks Defman :) I reverted your debug statements (that should not be part of this PR) but other than that everything is merged. Job well done!

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 20, 2015

Member

I guess github doesnt detect manual merges, here it is: a8377f1

Member

Naatan commented Jul 20, 2015

I guess github doesnt detect manual merges, here it is: a8377f1

@Naatan Naatan closed this Jul 20, 2015

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 20, 2015

Contributor

Great news! Thanks Nathan :)

Contributor

Defman21 commented Jul 20, 2015

Great news! Thanks Nathan :)

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 20, 2015

Member

By the way, when you type eg: animation: codeintel gives a tooltip explaining what it does, this does not seem to be completely up to date and thus there is an inconsistency between autocompletions and the tooltips. Imo this is fairly minor and having the autocompletions with an inconsistency outweighs having no autocompletions but no inconsistency.

Member

Naatan commented Jul 20, 2015

By the way, when you type eg: animation: codeintel gives a tooltip explaining what it does, this does not seem to be completely up to date and thus there is an inconsistency between autocompletions and the tooltips. Imo this is fairly minor and having the autocompletions with an inconsistency outweighs having no autocompletions but no inconsistency.

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Jul 20, 2015

Contributor

Anything I can do with it? As I don't understand completely what are you talking about, too much "hard" English words for me :(

Contributor

Defman21 commented Jul 20, 2015

Anything I can do with it? As I don't understand completely what are you talking about, too much "hard" English words for me :(

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Jul 20, 2015

Member

Seems to be an issue in the codeintel logic, logged a bug here: #437

Member

Naatan commented Jul 20, 2015

Seems to be an issue in the codeintel logic, logged a bug here: #437

@Defman21 Defman21 deleted the Defman21:css3-pr branch Jul 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment