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

Reduce binary size #8

Merged
merged 7 commits into from
Jul 2, 2024
Merged

Reduce binary size #8

merged 7 commits into from
Jul 2, 2024

Conversation

etiennebacher
Copy link
Owner

No description provided.

@@ -73,7 +76,7 @@ impl UnicodePosition {

#[derive(Clone)]
pub struct SgRoot {
inner: AstGrep<StrDoc<SupportLang>>,
inner: AstGrep<StrDoc<tree_sitter_facade_sg::Language>>,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it should be StrDoc<R> here. StrDoc accepts a type parameter satisfying ast_grep_core::Language. The R language implemented here should be the one to use.

@@ -82,9 +85,9 @@ pub struct SgRoot {
impl SgRoot {
fn new(src: &str) -> Self {
let position = UnicodePosition::new(src);
let lang = SupportLang::R;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, R.ast_grep(src) should be sufficient here.

/// normalize pattern code before matching
/// e.g. remove expression_statement, or prefer parsing {} to object over block
fn pre_process_pattern<'q>(&self, query: &'q str) -> Cow<'q, str> {
Cow::Borrowed(query)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also need to pre_process_pattern if $ is not a valid identifier in R.

/// * which character is used for meta variable.
/// * if we need to use other char in meta var for parser at runtime
/// * pre process the Pattern code.
pub trait Language: Clone {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use ast_grep_core::Language here?

@etiennebacher
Copy link
Owner Author

I think my last commit should work, thanks so much for your help @HerringtonDarkholme, I really appreciate!

Copy link

@HerringtonDarkholme HerringtonDarkholme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Nice!

@etiennebacher etiennebacher merged commit ccd315f into main Jul 2, 2024
6 checks passed
@etiennebacher etiennebacher deleted the reduce-binary-size branch July 2, 2024 21:21
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 this pull request may close these issues.

2 participants