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

From: Antero Mejr <[email-masked]>
Date: Tue, 11 Apr 2023 02:12:52 +0000

Jim Porter <[email-masked]> 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.