Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust grammar doesn't produce nice highlighting #502

Closed
trishume opened this issue Jun 23, 2016 · 14 comments
Closed

Rust grammar doesn't produce nice highlighting #502

trishume opened this issue Jun 23, 2016 · 14 comments
Labels
T: enhancement Improvement of existing language features

Comments

@trishume
Copy link
Contributor

The built in Rust grammar highlights code less effectively (IMO) than this grammar despite being much more complex and complete.

The reason is that although Rust.sublime-syntax assigns scopes to many things that the other grammar doesn't (e.g blocks), it doesn't assign scopes to many of the important things that would actually benefit from highlighting and that themes use.

Chief among these is that it doesn't assign a special scope to type names. Even just a rule for matching capitalized identifiers like the tmLanguage has would go a long way. As it stands all types except some standard library types get no annotation, and the standard library types get a non-standard annotation used by very few themes.

To be fair the C++ syntax has this same issue, but for some reason I don't notice it as much. An argument in favour of highlighting capitalized identifiers in Rust but not C++ is that many C++ projects use lower case type names so it would be inconsistent, whereas all Rust projects I know use capitalized type names.

A rule that highlights capitalized identifiers as storage.type.rust would mostly address my complaint, although it would be cool if the grammar could actually parse which identifiers were type names, which might be difficult, but less difficult than it would be for C++.

@wbond

@trishume
Copy link
Contributor Author

Example of a bunch of type names that should be highlighted but aren't:

screen shot 2016-06-23 at 5 52 24 pm

  • Enum constructors Ok(..)
  • Struct literals ScopeStack { ... }
  • Return types Result, ScopeStack, ParseScopeError
  • Static calls (both the Vec and a function call highlight for new)
  • Maybe annotate type and impl names with some scope?

@wbond
Copy link
Member

wbond commented Jun 23, 2016

I did spend some time learning Rust syntax in the process of porting that tmLanguage to .sublime-syntax. That said, I've never actually programmed in Rust, so it does not surprise me if I missed a number of things.

What would be most useful from you as a user of Rust would be examples of English-language rules that we can implement, along with code samples that show a snippet of code we can use to add to the test file. For instance, if identifiers starting with an upper-case letter are always a type, that would be a good rule to add. When it comes down to it, the existing syntax file is a solid base from which to make tweaks from. The syntax test file has a wealth of information to make sure we are preserving information as we move forward.

Over the past few months as I've been rewriting many of the syntaxes, we've been slowly standardizing on some scope names. Many of these are in heavy use, however some are not. Unfortunately the tmLanguage and tmTheme landscape is kind of a disaster when it comes to scope names. Many syntaxes make up their own conventions, and then color scheme authors implement the specific scopes in the language they use most.

My intention is that we are going to continue the rewrite process and start to go back through various syntaxes and standardize the scope names and create official documentation about what scopes should be used where, and what scopes color scheme authors should implement.

@wbond
Copy link
Member

wbond commented Jun 23, 2016

In addition to screenshots, be sure to post the raw code so that it can be copy-pasted for testing purposes.

@wbond wbond added the T: enhancement Improvement of existing language features label Jun 23, 2016
@trishume
Copy link
Contributor Author

Thanks. I might look into the Rust syntax myself and see if I can see an easy way to add the rules I would like.

Here's the example snippet from my screenshot:

impl FromStr for ScopeStack {
    type Err = ParseScopeError;

    /// Parses a scope stack from a whitespace separated list of scopes.
    fn from_str(s: &str) -> Result<ScopeStack, ParseScopeError> {
        let mut scopes = Vec::new();
        for name in s.split_whitespace() {
            scopes.push(try!(Scope::from_str(name)))
        }
        Ok(ScopeStack { scopes: scopes })
    }
}

@wbond
Copy link
Member

wbond commented Jun 24, 2016

Currently ScopeStack after for has the proper scope, it must just be that your color scheme isn't highlighting the generic scope entity.name. Many color schemes only target entity.name.function, entity.name.type and entity.name.class. Check out the Monokai color scheme for an example of one that does target entity.name.

Additionally, Err after type and Ok before ( have scopes and are targeted also.

screen shot 2016-06-24 at 5 57 33 am

The static method calls do need some scopes added, and the instance methods calls need to be tweaked to be consistent with other syntaxes.

:: needs the scope punctuation.accessor added for consistency.

@trishume
Copy link
Contributor Author

Right sorry I had forgotten about that, I had already seen that those had annotations, it just slipped my mind while writing the issue.

The interesting thing about that snippet though is that the Vec and Ok are only highlighted correctly because they are special cased here. Any user defined type gets no highlighting.

@FichteFoll
Copy link
Collaborator

Maybe add a support.type.user scope for this? And rename variable.other.readwrite to variable.user?

@jrappen
Copy link
Contributor

jrappen commented Jan 9, 2017

@trishume maybe you can fix your issues above more easily now that rust-lang/sublime-rust has a Rust.sublime-syntax file for easier comparison.

@wbond
Copy link
Member

wbond commented Jan 9, 2017

@jrappen The Rust Enhanced package recent forked the syntax here, so there won't be a ton of differences.

I'm hoping to do a little work on the Rust syntax soon to bring it more in line with the evolving scope naming docs.

@FichteFoll
Copy link
Collaborator

Reading my comment from last year (that I totally forgot about), variableuser doesn't sound that bad, honestly. It's quite unfortunate that variable.other.readwrite has been out for such a long time now because I really don't like that name.

emidoots added a commit to emidoots/Packages that referenced this issue May 9, 2018
This syntax file is generally better for right now, see:

sublimehq#502
@jwortmann
Copy link
Contributor

I observed some other inconsistencies with the scope naming guidelines:

  1. Is there a reason why Rust uses support.function instead of variable.function for generic function calls? Ref.: https://www.sublimetext.com/docs/3/scope_naming.html#variable
  2. Could as, in, box, get the scope keyword.operator.word instead of just keyword.operator, so that color schemes could highlight them differently from operator symbols? Ref.: https://www.sublimetext.com/docs/3/scope_naming.html#keyword

Would pull requests for these be accepted?

@keith-hall
Copy link
Collaborator

I believe PRs would be accepted, yes - definitely for number 2 anyway. For number one, open a PR and see what discussions arise :)

@FichteFoll
Copy link
Collaborator

I certainly agree with the former. If we're talking about normal function calls of arbitrary identifiers, those should be variable.function.

I'd also like to get highlighting of types for both Rust and Python (which are probably the languages I use the most) based on naming conventions, but I'd like to think that the discussion in #1842 should be resolved first.

@deathaxe
Copy link
Collaborator

deathaxe commented Jan 8, 2022

PR #2305 should improve the situation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: enhancement Improvement of existing language features
Projects
None yet
Development

No branches or pull requests

7 participants