Implemented tkn! macro for syntax kinds #1257
Conversation
pasa
force-pushed
the
issue_1248
branch
2 times, most recently
from
3b7c996 to
54c0bb8
Compare
| @@ -134,8 +136,8 @@ fn pick_best<'a>(l: SyntaxToken<'a>, r: SyntaxToken<'a>) -> SyntaxToken<'a> { | |||
| return if priority(r) > priority(l) { r } else { l }; | |||
| fn priority(n: SyntaxToken) -> usize { | |||
| match n.kind() { | |||
| WHITESPACE => 0, | |||
| IDENT | SELF_KW | SUPER_KW | CRATE_KW | LIFETIME => 2, | |||
| tkn![' '] => 0, | |||
There was a problem hiding this comment.
For whitespace, I think we should just use the name WHITESPACE, withot macro
| @@ -241,6 +241,102 @@ pub enum SyntaxKind { | |||
| } | |||
| use self::SyntaxKind::*; | |||
|
|
|||
| #[macro_export] | |||
| macro_rules! tkn { | |||
There was a problem hiding this comment.
Let's name this TOKEN: this generates a const, so it makes sense for this to be capitalized
So that one would use TOKEN![,]
There was a problem hiding this comment.
Hm, it'll be to screamy for short tokens though... Let's make it just
T![,]
?
There was a problem hiding this comment.
Maybe T! is too short? TKN![,] ?
There was a problem hiding this comment.
I like TKN less than either of T and TOKEN. Dropping vowels is not very readable to me.
My personal preference would be
T, TOKEN # somewhat equivalent
TOK
TKN
though, I don't have a strong opinion here and changing later would be easy
There was a problem hiding this comment.
TOKEN![,] is good for me, as IDE should be able to auto-complete the remain “KEN” after i type in “TO”.😂
There was a problem hiding this comment.
I like T! most of all too. I've made so.
crates/ra_ide_api/Cargo.toml
Outdated
| @@ -20,6 +20,7 @@ jemallocator = { version = "0.1.9", optional = true } | |||
| jemalloc-ctl = { version = "0.2.0", optional = true } | |||
|
|
|||
| ra_syntax = { path = "../ra_syntax" } | |||
| ra_parser = { path = "../ra_parser" } | |||
There was a problem hiding this comment.
The macro should be re-exported in ra_syntax, such that no direct dependency on ra_parser is required.
There was a problem hiding this comment.
I didn't figure out how to do this last time. It's correct right now
|
bors r+ Thanks! |
Build succeeded |
|
Might be a good idea to try to use this macro more across the codebase |
Implementation of #1248