add tags.scm queries #15
Conversation
There was a problem hiding this comment.
Hey @the-mikedavis 👋 Thanks for taking care of this! 🎉
There's one more case that we might consider for function definitions, which would be something like
let foo = fn(x) { x + 1 }i.e.
(let
pattern: (identifier)
value: (anonymous_function))
There's a whole road we could go down w.r.t. patterns and assignment with anonymous functions (what if the lambda is assigned a name in a destructuring of a tuple?) but returns rapidly diminish.
This PR makes me think that maybe the pub visibility qualifier might want to be its own node under type_definition or function definitions?
Yeah, I've debated doing that before. Notably, tree=sitter=rust uses a (visibility_modifier) node for it's common pub keyword and we could do something similar, though we also have to create a modifier node for "opaque".
Opened an issue for this here: #17.
| @@ -0,0 +1,54 @@ | |||
| ; Modules | |||
| (module) @name @reference.module | |||
There was a problem hiding this comment.
This will be a bit weird for GitHub, etc, since the (module) nodes' contents will look like "gleam/json" (etc), which is different from it's other references in this file (just "json"). That said, what you have here is technically correct 👍
There was a problem hiding this comment.
Hmm yeah maybe we should be splitting the module tokens in an import? The basic code navigation is a bit fuzzy so I think some awkwardness with imports is ok. Elixir has some troubles too when it comes to aliases.
| @@ -0,0 +1,54 @@ | |||
| ; Modules | |||
| (module) @name @reference.module | |||
| (import alias: (identifier) @name) @reference.module | |||
There was a problem hiding this comment.
This one's a bit hand-wavy too since the alias is just what the module will be called in this file, but this seems close enough that we should keep it. 👍
| (public_type_definition | ||
| (type_constructors | ||
| (type_constructor | ||
| name: (type_identifier) @name))) @definition.type | ||
| (type_definition | ||
| (type_constructors | ||
| (type_constructor | ||
| name: (type_identifier) @name))) @definition.type |
There was a problem hiding this comment.
Technically a data constructor (here mislabeled as "type_constructor"; the actual type constructors are above as "type_name") isn't a type definition. That said, I'm open to keeping this if it has nice properties when integrating into GitHub or other systems.
There was a problem hiding this comment.
Hmm yeah I wasn't sure about this. I think this ends up working out well because you can jump between usages of data constructors and their definitions. The suffix in definition.<kind> or reference.<kind> doesn't have to be module|function|type, we could distinguish these as data_constructor. OTOH it works out nicely to just label all remaining type_identifiers as reference.type. I'm not sure what the best move is 🙃
There was a problem hiding this comment.
I think for now we'll keep it as-is 👍
|
Ah interesting. I saw a I think it's ok for these tags to only work on blatantly obvious function/module/type constructs. That is probably something I would want to cover when writing stack-graphs queries for precise navigation |
connects #14
I'm not sure I got them all, plus I'd like to add some tests (see tree-sitter/tree-sitter#1547). This PR makes me think that maybe the
pubvisibility qualifier might want to be its own node undertype_definitionor function definitions? I think that might simplify the queries but I don't know if that's a good enough reason to refactor nodes. (Also, I use tree-sitter nodes for motion in my editor (see helix-editor/helix#1495) and I think jumping to thepubtoken could occassionally be valuable, wydt?)