projectile icon indicating copy to clipboard operation
projectile copied to clipboard

Error using ripgrep and fish: Expected a variable name after this $.

Open rynoV opened this issue 3 years ago • 5 comments

I had to change this line:

https://github.com/bbatsov/projectile/blob/a4f86f981c84a546530d5904253fa266431ef806/projectile.el#L4108

to

  let args mapcar (lambda (val) (concat "--glob !" (shell-quote-argument val)))

for projectile-ripgrep to work with fish shell, otherwise I get the error in the title because the glob patterns aren't properly escaped. This works for me, but I'm not confident enough with emacs/elisp to be sure this isn't going to break things for other people.

rynoV avatar May 19 '22 04:05 rynoV

The shell-quote-argument looks like a good habit, it's also what the counsel-projectile variant of this function does: https://github.com/ericdanan/counsel-projectile/blob/40d1e1d4bb70acb00fddd6f4df9778bf2c52734b/counsel-projectile.el#L921

I think this issue has been introduced in the PR https://github.com/bbatsov/projectile/pull/1762, where the default directory ignores have been changed from glob-patterns (e.g. ".git") to a regex (e.g. "^\.git$"). Clearly those values have to be escaped if we want to pass them to ripgrep. However ripgrep doesn't expect regexs but globs and the global directory ignores are broken, at least for me.

@rynoV can you maybe check if it's actually looking through those directories that are supposed to be globally ignored?

As a workaround I'm manually setting the globally ignored directories to the old value:

(setq projectile-globally-ignored-directories
  '(".idea"
    ".vscode"
    ".ensime_cache"
    ".eunit"
    ".git"
    ".hg"
    ".fslckout"
    "_FOSSIL_"
    ".bzr"
    "_darcs"
    ".tox"
    ".svn"
    ".stack-work"
    ".ccls-cache"
    ".cache"
    ".clangd"))

The documentation on projectile_globally_ignored_directories does claim to support regexes:

Regular expressions can be used.

Strings that don't start with * are only ignored at the top level
of the project.  Strings that start with * are ignored everywhere
in the project, as if there was no *.  So note that * when used as
a prefix is not a wildcard; it is an indicator that the directory
should be ignored at all levels, not just root.

Examples: \"tmp\" ignores only ./tmp at the top level of the
project, but not ./src/tmp. \"*tmp\" will ignore both ./tmp and
./src/tmp, but not ./not-a-tmp or ./src/not-a-tmp.

I'm not sure what the right solution is here. As far as I can see ripgrep simply doesn't support regex ignores. It looks like @maxtrussell did not really need the additional functionality of regexes, so maybe it would work to:

  • Revert the value of projectile_globally_ignored_directories to the old one and don't claim to support regexes
  • Add conversions from glob-patterns to regexes in appropriate places (is that what file-expand-wildcards does?)

yoricksijsling avatar Jun 29 '22 15:06 yoricksijsling

Revert the value of projectile_globally_ignored_directories to the old one and don't claim to support regexes

My concern, and the reason I didn't go for this in the first place, is that projectile_globally_ignored_directories ends up being combined with user-defined project ignore files. Because the docs say regex is supported, I bet that's exactly what users are putting into those files.

Meaning if we removed regex support it could silently break all the users with project ignore files.

Maybe we could define a list of these global ignores in glob form, pass them through a function to convert them to regex for use with project ignores, and send the original, glob form, list to ripgrep?

EDIT: On a second reading I see that's pretty much exactly what Yorick suggested

maxtrussell avatar Jun 29 '22 16:06 maxtrussell

can you maybe check if it's actually looking through those directories that are supposed to be globally ignored?

I just tested this out and the folders in projectile-globally-ignored-directories seem to be ignored properly. The variable's value:

("^\\.idea$" "^\\.vscode$" "^\\.ensime_cache$" "^\\.eunit$" "^\\.git$" "^\\.hg$" "^\\.fslckout$" "^_FOSSIL_$" "^\\.bzr$" "^_darcs$" "^\\.pijul$" "^\\.tox$" "^\\.svn$" "^\\.stack-work$" "^\\.ccls-cache$" "^\\.cache$" "^\\.clangd$")

And this is with the change I originally described.

I've been using projectile-ripgrep pretty heavily and it's worked well after my change

Ripgrep version:

$ rg --version
ripgrep 13.0.0
-SIMD -AVX (compiled)
+SIMD +AVX (runtime)

This is an example of the command being run by projectile-ripgrep:

rg  --fixed-strings --glob !TAGS --glob !\^\\.idea\$ --glob !\^\\.vscode\$ --glob !\^\\.ensime_cache\$ --glob !\^\\.eunit\$ --glob !\^\\.git\$ --glob !\^\\.hg\$ --glob !\^\\.fslckout\$ --glob !\^_FOSSIL_\$ --glob !\^\\.bzr\$ --glob !\^_darcs\$ --glob !\^\\.pijul\$ --glob !\^\\.tox\$ --glob !\^\\.svn\$ --glob !\^\\.stack-work\$ --glob !\^\\.ccls-cache\$ --glob !\^\\.cache\$ --glob !\^\\.clangd\$ --no-heading --line-number --with-filename --color=always --ignore-case -- testing .

rynoV avatar Jun 30 '22 03:06 rynoV

Ah thanks for checking @rynoV, I see now that I have a customization to add the --hidden flag for ripgrep, because by default it doesn't include any of these hidden files.

;; Default is "rg --with-filename --no-heading --line-number --color never %s"
;; I want to include hidden directories and files
(setq counsel-rg-base-command "rg --hidden --with-filename --no-heading --line-number --color never %s")

Within some directory that has a .git folder we can see that --hidden adds additional matches, and the glob-patterns can be used to filter those out. I've tried to add regex patterns instead of those globs ('!^\.git$', '!^\\.git$', '!\^\\.git\$') but it doesn't work.

$ rg --no-heading test | wc -l
70
$ rg --no-heading --hidden test | wc -l
83
$ rg --no-heading --hidden --glob '!.git' test | wc -l
70

Because the docs say regex is supported, I bet that's exactly what users are putting into those files.

Yes that seems like a valid concern, especially since (I think?) they did work as regexes under other circumstances. The choice to implement it this way was entirely reasonable. It's unfortunate that we have to choose between supporting regexes and having the default settings for ripgrep.

EDIT: On a second reading I see that's pretty much exactly what Yorick suggested

Yep! :smile:

yoricksijsling avatar Jun 30 '22 11:06 yoricksijsling

I see now that I have a customization to add the --hidden flag for ripgrep

Oh yeah I don't have that, and I just tested one of the non-hidden folders (_darcs) and it wasn't ignored, so I guess it wasn't working like I thought it was. Looks like all my change is doing right now is allowing the rg command to run without an error in fish shell.

rynoV avatar Jul 01 '22 00:07 rynoV