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

set globs have incorrect behavior #391

Closed
BurntSushi opened this issue Mar 3, 2017 · 1 comment
Closed

set globs have incorrect behavior #391

BurntSushi opened this issue Mar 3, 2017 · 1 comment
Labels
bug A bug. help wanted Others are encouraged to work on this issue.

Comments

@BurntSushi
Copy link
Owner

Repro:

$ git clone git://github.com/bag-man/dotfiles
$ cd dotfiles
$ rg --no-ignore --hidden --follow --files --glob '!{.git,node_modules,plugged}/**' --glob '*.{js,json,php,md,styl,scss,sass,pug,html,config,py,cpp,c,go,hs}'

The output includes files in the .git directory. Interestingly, removing some of the extensions in the second --glob impacts the output in unexpected ways. Changing the first glob to not use a recursive glob also seems to make it work.

If I look at the debug output, it looks like the generated regex for the second glob is:

(?-u)^(?:/?|.*/).*\\.hs|c|cpp|py|config|html|pug|sass|scss|styl|md|php|json|js$

Which is just plain wrong. The alternation should be in a group.

@BurntSushi BurntSushi added the bug A bug. label Mar 3, 2017
@BurntSushi
Copy link
Owner Author

BurntSushi commented Mar 3, 2017

Yup, the bug is here:

ripgrep/globset/src/glob.rs

Lines 657 to 665 in cf750a1

Token::Alternates(ref patterns) => {
let mut parts = vec![];
for pat in patterns {
let mut altre = String::new();
self.tokens_to_regex(options, &pat, &mut altre);
parts.push(altre);
}
re.push_str(&parts.join("|"));
}

All it needs is a regression test and some parens pushed around the alternation.

@BurntSushi BurntSushi added the help wanted Others are encouraged to work on this issue. label Mar 3, 2017
tiehuis added a commit to tiehuis/ripgrep that referenced this issue Mar 4, 2017
BurntSushi pushed a commit that referenced this issue Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug. help wanted Others are encouraged to work on this issue.
Projects
None yet
Development

No branches or pull requests

1 participant