ripgrepy icon indicating copy to clipboard operation
ripgrepy copied to clipboard

Fix issue #13 of files() and type_list() not working

Open vgadoury opened this issue 3 years ago • 4 comments

Fix issue #13 and emptying the pattern when using files(), and by preventing adding empty the patterns and path to the command. This also fixes the options type_list() and regexp(pattern), that were also broken.

I did it this way because it seems to be what was already expected in the code for type_list() and regexp(pattern).

This PR would prevent creating an object with the arguments to "search for an empty pattern" (i.e. before this change, Ripgrepy("", ".").run() valid and equivalent to rg "" ".", which is equivalent to rg --regexp "" ".".). This basically matches everything, which I guess could be useful (combined with other options) to list the full content of non-ignored searched files in the same format as a pattern-search. But I would doubt that someone currently depends on this "feature" from Ripgrepy... Note thatt it would still be possible to search for this explicitly with regexp("").

vgadoury avatar Sep 28 '22 22:09 vgadoury

Concerning the change in behaviour for Ripgrepy("", path), I guess a more backward-compatible fix would be to set None as default values to pattern and path in init, and set self.regex_pattern and self.path to None in the methods where they are set to "". This way any current code should not be broken (since currently is was not possible to create a Ripgrepy without at least 2 position arguments, and creating a Ripgrepy with either a regex_pattern=None or a path=None was unusable and causing errors along the road).

This solution has the bonus of making it possible to write Ripgrepy().type_list().run() or Ripgrepy("mypath").files().run().

vgadoury avatar Sep 28 '22 22:09 vgadoury

Thank for for the fantastic explanation and an awesome pull request for both #14 and #15 . I believe that I prefer the library to stay more focused on the actual regex features of ripgrepy instead of supporting ls like behavior of files or non useful (for regex context) behavior of type-list. It might be better to deprecate these methods instead

securisec avatar Sep 28 '22 23:09 securisec

Saying that, I will consider the changes and keep the pull requests open for now. Thanks for the contribution!

securisec avatar Sep 28 '22 23:09 securisec

@securisec I agree with you on the principle. When coding the fix I also thought that these files-listing options "do something else" and thus might be better somewhere else. But at the same time, you can still use then in addition to the other options.

If I may, I'll explain my use case: one of my prefered usage of ripgrep is its less-know power for finding files by name or pattern while still getting its speed and ignored-files support. I was about to reprogram something similar in python with globs and all, when I realized that riggrep was already doing it better than me (and than glob in python). So that's how I found ripgrepy as the first result while searching for "using ripgrep from python". ;-)

Examples of usage:

rg --files --glob "**/*Point.*"
rg --files -uu --glob "**/.vscode/settings.json"

This is sooo much faster the other (non indexed) tools I've tried on Windows, and from quick tests I would say still faster than find on linux.

And another big plus is just reusing the same tool and same syntax (especially useful for the --files-with-matches). And the --files option is quite useful to debug your other options (like paths and ignore-files and globbing), to make sure you actually have files to search in (could be quite useful if you're stuck in a remote python debug shell for example).

So, yeah, In my opinion the "files listing" options or ripgrep are still usages that take advantage of its regex power! :-)

vgadoury avatar Sep 29 '22 00:09 vgadoury