Conversation
resources/add_language.py
Outdated
| ALL_LANGUAGES = re.compile( | ||
| r"""\/{51} | ||
| \/\/ Constants | ||
| \/{51} | ||
| const allLanguages = (?P<all_languages>\[ | ||
| (\s*"[\w\-]+",\n?)+ | ||
| (?P<last_language>\s*"[\w\-]+",) | ||
| \])""", | ||
| ) |
There was a problem hiding this comment.
We should use GritQL for this.
There was a problem hiding this comment.
Good point. I don't really know GritQL well enough, yet. Whereas regex, I know all too well 🤣
There was a problem hiding this comment.
Thankfully this one is very simple:
`const allLanguages = [$langs]` where { $langs += `"foo"`}
There was a problem hiding this comment.
Very nice. Thanks. Might be worthwhile for me to spend a bit of time learning GritQL after this PR then!
There was a problem hiding this comment.
I'm just going to add "At least 10 test cases, including rewrites and metavariables." as you mentioned on the issue description. From your perspective, once I do that (assuming they all pass or are fixed as needed) would this PR be ready to merge? Or have I missed something else?
There was a problem hiding this comment.
I think I'd opt to remove this file, since it will potentially require maintenance and doesn't really make things much faster/easier as-is.
There was a problem hiding this comment.
@Alex-ley-scrub Can you remove this file? It's better to not point people in the wrong direction with a script for something that will probably become outdated quickly.
There was a problem hiding this comment.
yeah no worries, removed this file in: 16ce34d
as a small aside on this gritql pattern, for my general understanding of gritql, there's no built in sort() or sorted() command, right (https://docs.grit.io/language/functions#function-definitions) - i.e. to insert "Kotlin" in the "correct" place in this alphabetically sorted list? But we could do it with a custom JavaScript function
(https://docs.grit.io/language/functions#foreign-functions)?
`const allLanguages = [$langs]` where { $langs += `"Kotlin"`}maybe like this? It almost works but not exactly:
https://app.grit.io/studio?key=CIQtWzXJuUibMJ6krsn9R
engine marzano(0.1)
language js
function add_lang($langs, $new_lang) js {
// Use $var.text to access the binding's value
let output = $langs.text.split(",");
output.push($new_lang.text);
return output.sort();
}
`const allLanguages = [$langs]` where {
$new_langs = add_lang($langs, `"Kotlin"`)
} => `const allLanguages = [ $new_langs ];`|
Please also add at least one binary/CLI test in |
There was a problem hiding this comment.
LGTM
61 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
… add `simple_kotlin()` test fixes biomejs#570
…ariable-grammar.js' fixes biomejs#570
…ch of places to enable metavariable substitution fixes biomejs#570
…ources/language-metavariables/tree-sitter-kotlin/" fixes biomejs#570
…s/edit_grammars.mjs' script fixes biomejs#570
…PatternLanguage::Kotlin` fixes biomejs#570
…uct and add it to all match statements fixes biomejs#570
…es/lsp and crates/wasm-bindings fixes biomejs#570
…n tests as examples of GritQL syntax see https://docs.grit.io/language/syntax
…e some parsing it not happening correctly
…ython-metavariable-grammar with fewer $.grit_metavariable but at 'higher' level fixes biomejs#570
9855620 to
3b99c9c
Compare
a95e62e to
5da1d43
Compare
|
@morgante I finally got the time to debug this properly and figure out why the patterns weren't matching and make progress! But there are a bunch of clippy errors for code I didn't write: https://github.com/getgrit/gritql/actions/runs/12643182239/job/35228763160?pr=573 Do you want me to fix them? Are they new errors with a new compiler version or something? How else are they only just showing up now in my PR? I had done a |
|
Thanks @Alex-ley-scrub for following up here - excited to see this land.
Since they don't show up on main it would be good if you fix them.
Don't worry about that, I always just squash changes once we merge. |
resources/add_language.py
Outdated
| ALL_LANGUAGES = re.compile( | ||
| r"""\/{51} | ||
| \/\/ Constants | ||
| \/{51} | ||
| const allLanguages = (?P<all_languages>\[ | ||
| (\s*"[\w\-]+",\n?)+ | ||
| (?P<last_language>\s*"[\w\-]+",) | ||
| \])""", | ||
| ) |
There was a problem hiding this comment.
@Alex-ley-scrub Can you remove this file? It's better to not point people in the wrong direction with a script for something that will probably become outdated quickly.
|
Actually feel free to ignore the clippy errors, I think they're related to an untracked upstream change. |
|
Thanks for merging 'main' into this branch @morgante looks like all tests pass and all the clippys errors are removed now as well 🎉 🙌 |
/claim #570
fixes #570
todo:
resources/add_language.pyand run it withpython resources/add_language.py --lang kotlinedit_grammars.mjsnode ./resources/edit_grammars.mjs kotlincrates/core/languages/src/kotlin.rscargo test --package marzano-language --lib -- kotlin::tests --show-outputPatternsDirectorystruct in crates/gritmodule/src/patterns_directory.rs add it to all match statementscargo test --package marzano-core --lib -- test::simple_kotlin --exact --show-outputGreptile Summary
This is an auto-generated summary
Hi! Looks like you've reached your API usage limit. You can increase it from your account settings page here: app.greptile.com/settings/usage