Skip to content

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

Open
vgadoury wants to merge 2 commits into
securisec:masterfrom
vgadoury:patch-1
Open

Fix issue #13 of files() and type_list() not working#14
vgadoury wants to merge 2 commits into
securisec:masterfrom
vgadoury:patch-1

Conversation

@vgadoury

@vgadoury vgadoury commented Sep 28, 2022

Copy link
Copy Markdown

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("").

Fix issue securisec#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)`. 

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

vgadoury commented Sep 28, 2022

Copy link
Copy Markdown
Author

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().

Change the ignored value for regex_pattern and path from `""` to `None`. Allow to create a Ripgrepy without regex_pattern or path.

This code is now valid:
```
Ripgrepy().type_list().run()
Ripgrepy(path="./ripgrepy/").files().run()
Ripgrepy("Ripgrepy").count_matches().run() 
```
@securisec

securisec commented Sep 28, 2022

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Owner

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

@vgadoury

Copy link
Copy Markdown
Author

@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! :-)

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