User Type Highlighting #431
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @da-x (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Iron-E
force-pushed
the
feature/struct-definition-identifiers
branch
2 times, most recently
from
77ffd41 to
a3bb8a5
Compare
|
Thanks @Iron-E, this is amazing work! Largest update to the syntax description in quite awhile. @chris-morgan please help review this? |
There is one specific case when we know that a token is a modpath, but it doesn't precede a '::'. Example: ```rust use foo; ``` We know `foo` must be a module, but under normal circumstances it appears like an Identifier. This commit adds special logic to identify these cases.
Iron-E
force-pushed
the
feature/struct-definition-identifiers
branch
from
a3bb8a5 to
e0babbf
Compare
Whenever attemping to access an enum: ```rust Option::Some Iso::USA ``` `Option` and `Iso` would be highlighted as if they were modules rather than a types. To fix this, the definition of a `rustModPath` was altered to contain `@rustIdentifiers`.
There was some repetition of which comment groups were `contained` in folds, and which weren't. Some even dictated inclusion of nested groups, which seem to be intended for nesting within a `contained` top-level comment. This commit adds a dedicated `cluster` for including the top-level `rustComment`* groups, and replaces usages where applicable.
Allows highlighting of `&` in syntax such as:
```rust
pub struct Location<'outer>
{
pub outer: Option<&'outer Self>,
}
```
`pub`lic `type`defs were not being detected.
`let foo = match bar {};` wasn't highlighting `match`.
Makes `dyn` highlighted in: ```rust &dyn FooTrait ```
Before we did not highlight `as` in a use block, like so:
```rust
use core::fmt::{Display, Result as FmtResult};
```
Now `as` is highlighted properly.
The `pub?` check should have been outside the alternation scope, since both `use` and `mod` can be `pub`.
Both `mod` and `use` can be `pub`, so the check for whether or not `pub` was present should not have been in an alternation specific to `mod`.
Previously this group was needed to detect mod paths. Recent updates to the mod path groups have allowed this to be removed entirely.
Whenever using `return` explicitly, there was an opportunity for either `match` or `await` to be used after `return`. Despite this, I had selected `@rustIdentifiers` as the `nextgroup`— this caught all the keywords and as such `match` and `await` never showed as expected.
|
Sorry for that last round of commits— there were some more issues that I uncovered which needed to be ironed out. Haven't discovered anything else in about a week since though so it's probably close in terms of function. I will say that there are some things which this implementation doesn't try to do, like tell the difference between typedefs, traits, and the like. There really isn't a great way to know for sure whether or not something is just a "type"/struct or if it's something more specific without leveraging a language server (which I believe is also being discussed). So, these changes would probably act as a fallback for when the LSP is not available (or as a bridge until the LSP highlighting is integrated). |
It allows for supertraits with generic bounds that have the following
syntax:
```rust
pub trait CrudOrganization<T> : Into<T> + where WLocation : Wrapper<Location<'name>>, { }
```
Previously, the following `dyn` would have been highlighted as an
identifier:
```rust
pub trait Foo<T> where T : Bar<dyn SomeSizedTrait> {}
```
Now it will be detected as `rustDynKeyword`.
```rust
impl Foo
for Bar
{}
```
The above is valid syntax, but `for` was not being highlighted correctly
in that case.
```rust ($name: ident, $($life: lifetime)*, $T: ident) ``` Now, `$name` and `$life` are highlighted the same. `ident` and `lifetime` are also highlighted as types, rather than identifiers.
|
I have addressed most of the concerns which you noted in the previous commits. There are only a few questions/comments I had which I am hoping you could clarify/discuss with me further:
The reason why I opted for syn match rustModule "\v(r#)?<\l(\l|\d)*(_+(\l|\d)*)*>" contained contains=rustRawModule,@rustScopes displayAs for closures specifically, could you give an example of such breakage? There is a whole section of the sample file dedicated to testing this exact case.
This is one area I disagree. The
Could you note an example for this as well?
I don't necessarily think the use of |
The `>` in `'static>` was being treated literally, so there was mishighlighting of `'static` when inside of a `rustGenericRegion` but also outside of it.
I thought it would be better to use a different name since this macro does not panic at runtime, instead it causes compilation to fail.
|
What's stopping this from getting merged? :) |
Iron-E
force-pushed
the
feature/struct-definition-identifiers
branch
from
1c2592b to
d3a27a5
Compare
|
@chris-morgan or @da-x any updates? It would be good to finally push this through. Even though this PR isn't as good as something like LSP or Treesitter highlighting— it's still an improvement to and has fewer bugs than |
It was mislighting certain things, for example:
```rust
macro_rules! foo {
// Invalid syntax; highlights correctly
($($bar:expr),,*) => {},
// Valid syntax; highlights incorrectly
($($bar:expr) *) => {},
}
```
This is now fixed.
|
@chris-morgan @da-x checking in again— what is the status of this pull? Is there anything blocking it? |
|
@chris-morgan only waiting your ack, being it a big change around highlighting. |
It did not recognize this repeat delimiter, even though it is valid.
|
I feel like the keywords in macro_rules could be actually highlighted maybe (i.e. pub fn self...). Master: |
Closes #21.
Closes #102.
Closes #142.
Closes #205.
Closes #373.
Closes #397.
Closes #428.
Closes #477.
Yesterday I opened a PR and it was closed. Despite this, the reviewer said:
So I did. Now, types are highlighted in most places rather than just struct definitions. I used active community repositories as a benchmark, such as Ripgrep.
Further, if changes to my chosen names are desired, just ask :). I'm sure we can figure something out.
This PR does a few things of direct relation to #142:
rustNoiseto allow highlighting of commas, semicolons, dot operators, and other noise characters.rustBoundsto allow highlighting of:inwhere Foo : Barfrom a struct definition.rustGenericRegion.matchgroup.@rustIdentifiers.rustIdentifierrustTyperustTyperustBuiltinTyperustIdentifierrustConstant:h group-namefor more information.Topics left to discussion:
If you find some place where there wasn't coverage let me know.
master