search

kevinsawicki / monokai

Monokai Atom Syntax theme
241 stars 133 forks source link

Three small tweaks... #42

Closed svanharmelen closed 9 years ago

svanharmelen commented 9 years ago

Three small tweaks to make the look and feel of this theme consistent with the Monokai theme available for Sublime.

asantos3 commented 9 years ago

.keyword.operator fixes curly braces and punctuation in go but what does the other two tweaks do?

kevinsawicki commented 9 years ago

@svanharmelen thanks for these, would you mind throwing up a screenshot that summarizes the changes?

svanharmelen commented 9 years ago

Please see the below screenshots for the changes made by this PR...

Previous colors: image

Updated colors: image

Please notice the:

Besides that I personally believe these changes make the theme much more readable and clear, the changes are also in line with how the Monokai theme looks in Sublime.

svanharmelen commented 9 years ago

@kevinsawicki any feedback on this one? Are you OK with the changes or do you need/want to discuss anything first? If so just let me know your concerns... Thx!

kevinsawicki commented 9 years ago

Looks good, thanks for the screenshots :ship:

svanharmelen commented 9 years ago

Nice, thx...

Victorystick commented 9 years ago

Having done work on the brackets in atom/language-go#52, some of these changes are basically hacks to fix the syntax highlighting of the odd scopes that the Go grammar generated at the time.

In atom/language-go#32, two syntax themes which have been tweaked especially for Go are presented as a solution, rather than suggesting to fix the arguably buggy grammar.

As you can see in the example I supplied in the PR, I had the same issue of miscolored brackets in my theme. The issue was addressed by aligning language-go's behavior with other languages, like C and JavaScript.

Similarly, function declarations in those languages give the function name the scope entity.name.function, which results in the expected highlighting. Where as the current Go apparently yields support.function.decl.

I propose changing support.function.decl to entity.name.function, which will make more themes work just as well as this one with Go.

svanharmelen commented 9 years ago

Sounds like a plan... Allow me to update that one and at the same time cleanup some of the tweaks in this package as well, making stuff a little more generic again.

Thanks for pointing it out!

Victorystick commented 9 years ago

@svanharmelen You're fast! Nice of you to fix it!

Regarding the third and final change, that of string escapes, Go targets the constant.escape.format-verb scope where as JavaScript and C use constant.character.escape. Syntax themes such as Atom One Dark/Light only target the latter. Extending the scope to include character (such as constant.character.escape.format-verb) would enable Atom's default syntax to highlight all three tweaks without a problem.

Having made this final change would, for better or worse, make the changes of this PR superfluous. Since I know you care about Go's highlighting as much as I do, I hope you won't take offence.

svanharmelen commented 9 years ago

@Victorystick again sounds good :wink: I would suggest to just let it be constant.character.escape in that case. The format-verb doesn't add much I think...

Are there any more improvements your aware of? Then I'll bundle them for a next release.

Thanks for helping to make language-go more robust and inline with the other languages packages!

Victorystick commented 9 years ago

I agree with you. Keep it simple.

I don't have access to a computer, otherwise I would have made a PR myself. I'll take another look when I do.

Nice working with you, as always! :)