bug#62426: [PATCH] eshell: Add 'rgrep' builtin.

[Antero Mejr]

Jim Porter <jporterbugs@gmail.com> writes:

> I just see one issue with the patch: the NEWS entry and docstring are a bit
> misleading. I know these are just copying from the existing Eshell functions,
> but this isn't really accurate: "Use Emacs grep facility instead of calling
> external rgrep." It *always* calls the external rgrep; it's just that 
> sometimes
> the output goes into a compilation buffer.

I don't think that's always the case. In the "eshell-grep" function it
checks if external grep is available, and if not then it uses a slow
elisp-only implementation. Then it checks to see if the output is being
redirected, and so on.

IMO that entire process constitutes the "emacs grep facility" as
described in the docstring.

> Instead, how about something like this? "Call the external rgrep program,
> opening its output in a compilation buffer when possible." (Plus similar 
> changes
> to the other existing functions.) I think that makes it clearer what's 
> actually
> happening.

I think the "Emacs grep facility" description is technically accurate
and concise enough for the eshell/*grep stub functions.

> Anyway, sorry for the back and forth on this. This is a part of Eshell that I
> haven't looked at in much detail before, and I wanted to get a better sense of
> what our options were here.

Thanks for the review and discussion.