Closed kylefleming closed 9 years ago
Awesome! However, two issues I see are:
enum
and package
.I see now - maybe the italics in the screenshot are just part of your particular config. (In mine, datatypes are not italicized.)
@kylefleming , actually I redact everything I said and I do like your approach. Every other way just doesn't make sense. Thank you soooo much for working on this. There are some minor issues, which I will post here and also add line-by-line comments to the source.
@kylefleming I've now spent significant time with this. Thanks again. I have one humble request and one potential flaw to point out. Screenshot below illustrates.
enum Foo
behaves like class
and interface
.I'm personally no longer concerned with the way package
is/isn't handled; the default actually seems aesthetically pleasing across many different color themes.
@kylefleming (sorry to be so chatty) You can ignore my comments regarding the inconsistent parenthesis highlighting. This is really an issue of the particular color theme.
I guess the only issues to discuss are:
enum
to the same / similar rule as class
and interface
.source.pde
to source.processing
. /cc @b-g
@ybakos I'll answer your questions in order:
enum
/package
highlighting: if you try to declare an enum or declare your file belongs to a package, in both cases you get a compiler error. The reason I can see to keep it is if we wanted someone to be able to select the processing highlighter for a pure java file. On the whole, it doesn't hurt to add them back in, so either way is fine with me. I notice in your screenshots you have these declared listed. Do those files compile?storage.type
and c/c++ uses variable.other
. Given that the color scheme can style each identifier however it wants, your theme probably styles the 2 identifiers exactly the same while the one I'm using (Monokai on sublime text 2) probably italicizes one of them. We can change it to match what was there before if we want, for this issue too I think either way is fine.setup()
) which isn't possible in java.punctuation.definition.parameters
. It's easy to take out and probably should be taken out, or we should highlight the end one too. The easier route is to take it out though..java-processing
to .processing
. This again is for consistency with other languages. (and side note I see I forgot to change the C ones.) There may be side of effects of this change, but I'm not away of any.Thanks @kylefleming . I think in this PR we should keep it focused to the issue at hand and ignore my comments about the Java classes, etc.
I think it's wise to:
enum
stuff (again, ignore what I said about package
).source.processing
change in other locations.Care to create another PR with those two changes, and close this one? (@b-g will probably chime in soon).
@ybakos Thanks for the code review. I added a new commit to this pull request with these changes:
source.pde
. I figure that the config files of people's machines are probably already using source.pde for things like "always open this file as" and stuff like that. Better not to break it.enum
and package
stuff back in. Since the processing language file declares it can open java files, it's probably best to make sure it styles them the same as the java highlighter would.Merged!
@kylefleming Many thanks for taking care of it! Finally working syntax highlighting! I added you to https://github.com/b-g/processing-sublime#acknowledgements
@ybakos Thanks for feedback, code review and spotting!
@kylefleming Awesome, man. Thank you!
I'm glad it all worked out, happy to have contributed!
@b-g @ybakos Give this version a try. I did a screenshot compare of some of my pde files and in each case the highlighting was improved and no highlighting was lost from what I can tell. Hopefully you find the same results!
Here's what I did for reference:
Here's a comparison to the screenshots I took in #20:
(and for completeness here's it compared again to the current processing highlighting)