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

Read query from file #143

Closed
bheylin opened this issue Oct 3, 2024 · 5 comments · Fixed by #144
Closed

Read query from file #143

bheylin opened this issue Oct 3, 2024 · 5 comments · Fixed by #144

Comments

@bheylin
Copy link
Contributor

bheylin commented Oct 3, 2024

I would like to be able to read a tree-sitter query from file so I can develop the query in a text editor and optionally save the for a longer time.

So given the query file find_mods_nested_in_test_mods.scm:

; saved in `find_mods_nested_in_test_mods.scm`
(mod_item 
  (visibility_modifier) 
  name: (identifier) @mod.name 
  body: 
    (declaration_list 
      (mod_item name: (identifier)) 
    )
)
(#eq? @mod.name "test")

I would like to be able to give that file to the --rust-query option:

srgn --rust-query find_mods_nested_in_test_mods.scm


I`m open to implementing this feature myself, but want to discuss it first.

Copy link

issuedigger bot commented Oct 3, 2024

The most similar issues to this one are:

  1. Breaking but unimportant changes #137 , with a similarity score of 0.91.
  2. 0.13.0 build failure #106 , with a similarity score of 0.91.
  3. Fix cloning static query on every query call #76 , with a similarity score of 0.91.

@alexpovel
Copy link
Owner

Hi @bheylin , thanks for your request. Apologies for @issuedigger , it's a tool I'm experimenting with and safe to ignore.

Your proposal looks good, it's a useful feature. Your contribution would be very welcome!

I was thinking, should --rust-query be multi-modal and first check if its argument is an existing file (path), if not fall through and see if it's a parsable query (it only does the latter currently)? Or should there be a new argument such as --rust-query-file, so things remain unambiguous?

Valid & existing file paths are nothing like tree-sitter queries, so I reckon multi-modal is safe and sane.

@bheylin
Copy link
Contributor Author

bheylin commented Oct 3, 2024

Hi @alexpovel changing --rust-query and related options to multi-modal also sounds reasonable to me.
I would simply use std::Path::is_file on the option's value and branch from there.

I'll get a PR together when I have a moment over the coming days.

@alexpovel
Copy link
Owner

Great, thanks! I think CONTRIBUTING.md should contain everything you'll need to get started, but let me know.

One thought: the docs of std::Path::is_file mention:

When the goal is simply to read from (or write to) the source, the most reliable way to test the source can be read (or written to) is to open it. Only using is_file can break workflows like diff <( prog_a ) on a Unix-like system for example. See fs::File::open or fs::OpenOptions::open for more information.

which I tend to agree with. So perhaps just opening directly is a more suitable approach.

srgn also uses logging, which we should use to keep users informed of what decision was taken at every step.

The main function of interest should be:

srgn/src/main.rs

Lines 800 to 843 in 4234925

fn get_language_scopers(args: &cli::Cli) -> Vec<Box<dyn LanguageScoper>> {
// We have `LanguageScoper: Scoper`, but we cannot upcast
// (https://github.com/rust-lang/rust/issues/65991), so hack around the limitation
// by providing both.
let mut scopers: Vec<Box<dyn LanguageScoper>> = Vec::new();
macro_rules! handle_language_scope {
($lang:ident, $lang_query:ident, $query_type:ident, $lang_type:ident) => {
if let Some(lang_scope) = &args.languages_scopes.$lang {
if !scopers.is_empty() {
let mut cmd = cli::Cli::command();
cmd.error(
clap::error::ErrorKind::ArgumentConflict,
"Can only use one language at a time.",
)
.exit();
}
assert!(scopers.is_empty());
for query in &lang_scope.$lang {
let query = $query_type::Prepared(query.clone());
scopers.push(Box::new($lang_type::new(query.clone())));
}
for query in &lang_scope.$lang_query {
let query = $query_type::Custom(query.clone());
scopers.push(Box::new($lang_type::new(query.clone())));
}
assert!(!scopers.is_empty(), "Language specified, but no scope."); // Internal bug
};
};
}
handle_language_scope!(c, c_query, CQuery, C);
handle_language_scope!(csharp, csharp_query, CSharpQuery, CSharp);
handle_language_scope!(hcl, hcl_query, HclQuery, Hcl);
handle_language_scope!(go, go_query, GoQuery, Go);
handle_language_scope!(python, python_query, PythonQuery, Python);
handle_language_scope!(rust, rust_query, RustQuery, Rust);
handle_language_scope!(typescript, typescript_query, TypeScriptQuery, TypeScript);
scopers
}

which contains a really nasty macro, sorry... I'm open to any refactors there.

@bheylin
Copy link
Contributor Author

bheylin commented Oct 3, 2024

One thought: the docs of std::Path::is_file mention:

When the goal is simply to read from (or write to) the source, the most reliable way to test the source can be read (or written to) is to open it. Only using is_file can break workflows like diff <( prog_a ) on a Unix-like system for example. See fs::File::open or fs::OpenOptions::open for more information.

which I tend to agree with. So perhaps just opening directly is a more suitable approach.

Yea that's a fair point, I'll update the proof-of-concept code.

srgn also uses logging, which we should use to keep users informed of what decision was taken at every step.

👍

@alexpovel alexpovel linked a pull request Oct 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants