GNU bug report logs - #39384
[PATCH] gnu: Add emacs-rg.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39384 in the body.
You can then email your comments to 39384 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add emacs-rg.
Date: Sat, 01 Feb 2020 14:28:37 -0600

[Message part 1 (text/plain, inline)]

Patch file is attached to package 
https://github.com/dajva/rg.el.git.

--
Joseph LaFreniere

[0001-gnu-Add-emacs-rg.patch (text/x-patch, attachment)]

Message #8 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sat, 01 Feb 2020 23:09:19 +0100

Hello,

"LaFreniere, Joseph" <joseph <at> lafreniere.xyz> writes:

> Patch file is attached to package https://github.com/dajva/rg.el.git.

Thank you! Some comments follow.

> +       (sha256
> +        (base32
> +         "0k7x5z7mh9flwih35cqy8chs54rack3nswdcpw5wcpgv6xim227y"))))

Nitpick: I think the trend is to align `base32' with the string.

> +    (build-system emacs-build-system)
> +    (propagated-inputs
> +     `(("emacs-s" ,emacs-s)
> +       ("emacs-wgrep" ,emacs-wgrep)
> +       ("ripgrep" ,ripgrep)))
> +    (home-page "https://rgel.readthedocs.io/en/latest/")
> +    (synopsis "A search tool based on @code{ripgrep}")

You may want to lint your package. In particular, the synopsis should be
akin to "Search tool based ..."

> +    (description
> +     "An Emacs search package based on the @code{ripgrep} command line

The description must start with a full sentence, e.g., "rg.el" is an
Emacs search package...

> +tool. It allows you to interactively create searches, doing automatic searches

Texinfo requires two spaces after the full stop.

> +based on the editing context, refining and modifying search results and much
> +more. It is also highly configurable to be able to fit different users’

Ditto. Besides, the quote after "users" looks suspicious. You should use
a regular quote.

Could you send an updated patch?

Regards,

-- 
Nicolas Goaziou

Message #11 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Cc: 39384 <at> debbugs.gnu.org
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sun, 02 Feb 2020 08:21:52 -0600

[Message part 1 (text/plain, inline)]

Thank you for the fast feedback!

Nicolas Goaziou <mail <at> nicolasgoaziou.fr> writes:
> Nitpick: I think the trend is to align `base32' with the string.

> You may want to lint your package. In particular, the synopsis 
> should be
> akin to "Search tool based ..."

> The description must start with a full sentence, e.g., "rg.el" 
> is an
> Emacs search package...

> Texinfo requires two spaces after the full stop.

> Ditto. Besides, the quote after "users" looks suspicious. You 
> should use
> a regular quote.

A patch file is attached that addresses all of the above feedback. 
The output of `guix lint emacs-rg` is now clean on my system; 
thank you for making me aware of that utility.

The only part of the package I'm uncertain about is declaring 
ripgrep as a propagated dependency.  ripgrep is not needed for 
this Emacs package to be able to byte-compile successfully, but 
`rg` does not need to be on PATH for the package to be useful at 
all.  So while I imagine the majority of the uses-cases would want 
to have ripgrep installed locally, it's definitely plausible that 
one could only ever want to use emacs-rg via TRAMP in which case 
pulling in ripgrep would be completely unnecessary.

Please let me know what you think.

--
Joseph LaFreniere

[0001-gnu-Add-emacs-rg.patch (text/x-patch, attachment)]

Message #14 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sun, 2 Feb 2020 20:47:57 +0200

[Message part 1 (text/plain, inline)]

On Sun, Feb 02, 2020 at 08:21:52AM -0600, LaFreniere, Joseph wrote:
> Thank you for the fast feedback!
> 
> Nicolas Goaziou <mail <at> nicolasgoaziou.fr> writes:
> > Nitpick: I think the trend is to align `base32' with the string.
> 
> > You may want to lint your package. In particular, the synopsis should be
> > akin to "Search tool based ..."
> 
> > The description must start with a full sentence, e.g., "rg.el" is an
> > Emacs search package...
> 
> > Texinfo requires two spaces after the full stop.
> 
> > Ditto. Besides, the quote after "users" looks suspicious. You should use
> > a regular quote.
> 
> A patch file is attached that addresses all of the above feedback. The
> output of `guix lint emacs-rg` is now clean on my system; thank you for
> making me aware of that utility.
> 
> The only part of the package I'm uncertain about is declaring ripgrep as a
> propagated dependency.  ripgrep is not needed for this Emacs package to be
> able to byte-compile successfully, but `rg` does not need to be on PATH for
> the package to be useful at all.  So while I imagine the majority of the
> uses-cases would want to have ripgrep installed locally, it's definitely
> plausible that one could only ever want to use emacs-rg via TRAMP in which
> case pulling in ripgrep would be completely unnecessary.
> 
> Please let me know what you think.

Is it possible to patch the invocations of `rg` to refer to ripgrep
directly?


-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sun, 02 Feb 2020 21:41:41 -0600

Efraim Flashner <efraim <at> flashner.co.il> writes:
> Is it possible to patch the invocations of `rg` to refer to 
> ripgrep directly?

Thank you for the input, but I don't understand what you mean by 
"referring to ripgrep directly".  Can you elaborate on that 
proposal and how it would help resolve the question of whether to 
include ripgrep as a propagated input?

--
Joseph LaFreniere

Message #20 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Tue, 4 Feb 2020 11:58:51 +0200

[Message part 1 (text/plain, inline)]

On Sun, Feb 02, 2020 at 09:41:41PM -0600, LaFreniere, Joseph wrote:
> 
> Efraim Flashner <efraim <at> flashner.co.il> writes:
> > Is it possible to patch the invocations of `rg` to refer to ripgrep
> > directly?
> 
> Thank you for the input, but I don't understand what you mean by "referring
> to ripgrep directly".  Can you elaborate on that proposal and how it would
> help resolve the question of whether to include ripgrep as a propagated
> input?

We want to have ripgrep as a regular input, so at the point in the code
where it searches through PATH for the rg binary we patch it to refer to
the specific binary. One example of this is in (gnu packages emacs-xyz),
with emacs-nov-el.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Tue, 04 Feb 2020 21:08:11 -0600

Efraim Flashner <efraim <at> flashner.co.il> writes:
> We want to have ripgrep as a regular input, so at the point in 
> the code
> where it searches through PATH for the rg binary we patch it to 
> refer to
> the specific binary. One example of this is in (gnu packages 
> emacs-xyz),
> with emacs-nov-el.

Ah, I see what you mean now.  But wouldn't hard-coding the path to 
ripgrep in that way prevent the package from being able to use 
remote systems' ripgrep binaries when running over TRAMP?

--
Joseph LaFreniere

Message #26 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Wed, 05 Feb 2020 07:13:02 +0000

On February 5, 2020 3:08:11 AM UTC, "LaFreniere, Joseph" <joseph <at> lafreniere.xyz> wrote:
>
>Efraim Flashner <efraim <at> flashner.co.il> writes:
>> We want to have ripgrep as a regular input, so at the point in 
>> the code
>> where it searches through PATH for the rg binary we patch it to 
>> refer to
>> the specific binary. One example of this is in (gnu packages 
>> emacs-xyz),
>> with emacs-nov-el.
>
>Ah, I see what you mean now.  But wouldn't hard-coding the path to 
>ripgrep in that way prevent the package from being able to use 
>remote systems' ripgrep binaries when running over TRAMP?
>

Unfortunately that's not something I know. I remember that for the tramp package itself there were some changes related to PATH to make it work when connecting between a Guix System and a foreign one. Hopefully someone knows the answer.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Message #29 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>,
 Efraim Flashner <efraim <at> flashner.co.il>
Cc: 39384 <at> debbugs.gnu.org, Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Wed, 05 Feb 2020 22:33:26 +0100

[Message part 1 (text/plain, inline)]

"LaFreniere\, Joseph" <joseph <at> lafreniere.xyz> writes:

> Efraim Flashner <efraim <at> flashner.co.il> writes:
>> We want to have ripgrep as a regular input, so at the point in 
>> the code
>> where it searches through PATH for the rg binary we patch it to 
>> refer to
>> the specific binary. One example of this is in (gnu packages 
>> emacs-xyz),
>> with emacs-nov-el.
>
> Ah, I see what you mean now.  But wouldn't hard-coding the path to 
> ripgrep in that way prevent the package from being able to use 
> remote systems' ripgrep binaries when running over TRAMP?

Perhaps we could patch it to do both?  Use the store prefix if it
exists, and fall back to searching in PATH?

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 39384 <at> debbugs.gnu.org, Efraim Flashner <efraim <at> flashner.co.il>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Thu, 06 Feb 2020 18:47:23 -0600

Marius Bakke <mbakke <at> fastmail.com> writes:
> "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz> writes:
>> Ah, I see what you mean now.  But wouldn't hard-coding the path 
>> to
>> ripgrep in that way prevent the package from being able to use
>> remote systems' ripgrep binaries when running over TRAMP?
>
> Perhaps we could patch [emacs-rg] to do both?  Use the store 
> prefix if it
> exists, and fall back to searching in PATH?

What would be the advantage of that over just searching PATH to 
start with?

--
Joseph LaFreniere

Message #35 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org, Marius Bakke <mbakke <at> fastmail.com>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Fri, 7 Feb 2020 12:54:35 +0200

[Message part 1 (text/plain, inline)]

On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph wrote:
> 
> Marius Bakke <mbakke <at> fastmail.com> writes:
> > "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz> writes:
> > > Ah, I see what you mean now.  But wouldn't hard-coding the path to
> > > ripgrep in that way prevent the package from being able to use
> > > remote systems' ripgrep binaries when running over TRAMP?
> > 
> > Perhaps we could patch [emacs-rg] to do both?  Use the store prefix if
> > it
> > exists, and fall back to searching in PATH?
> 
> What would be the advantage of that over just searching PATH to start with?

It will still work even if you don't have ripgrep specifically
installed.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Message #38 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 39384 <at> debbugs.gnu.org, Marius Bakke <mbakke <at> fastmail.com>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sat, 08 Feb 2020 16:23:08 -0600

Efraim Flashner <efraim <at> flashner.co.il> writes:
> On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph 
> wrote:
>>
>> Marius Bakke <mbakke <at> fastmail.com> writes:
>> > "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz> writes:
>> > > Ah, I see what you mean now.  But wouldn't hard-coding the 
>> > > path to
>> > > ripgrep in that way prevent the package from being able to 
>> > > use
>> > > remote systems' ripgrep binaries when running over TRAMP?
>> >
>> > Perhaps we could patch [emacs-rg] to do both?  Use the store 
>> > prefix if
>> > it
>> > exists, and fall back to searching in PATH?
>>
>> What would be the advantage of that over just searching PATH to 
>> start with?
>
> It will still work even if you don't have ripgrep specifically
> installed.

Can you point me to the Guix documentation where the functionality 
you're describing is explained?  I have read through the 
description of package inputs in section 6.2.1 of Guix's manual, 
but I still do not explain what advantage patching the search path 
offers.

My understanding is that if we want to preserve both local and 
remote-via-TRAMP functionality, we can either
- just include ripgrep as a propagated input, or
- include ripgrep as a propagated input _and_ patch the package to 
 look for ripgrep in a hardcoded location (for local) as well as 
 PATH (for TRAMP).

Both options would have ripgrep included as propagated input.  As 
soon as ripgrep is installed in a user's profile, its binary will 
be available on PATH.  If that is correct, then I don't see any 
advantage to patching in a hardcoded path to ripgrep.

--
Joseph LaFreniere

Message #41 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384 <at> debbugs.gnu.org, Marius Bakke <mbakke <at> fastmail.com>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sun, 9 Feb 2020 15:28:41 +0200

[Message part 1 (text/plain, inline)]

On Sat, Feb 08, 2020 at 04:23:08PM -0600, LaFreniere, Joseph wrote:
> 
> Efraim Flashner <efraim <at> flashner.co.il> writes:
> > On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph wrote:
> > > 
> > > Marius Bakke <mbakke <at> fastmail.com> writes:
> > > > "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz> writes:
> > > > > Ah, I see what you mean now.  But wouldn't hard-coding the > >
> > > path to
> > > > > ripgrep in that way prevent the package from being able to > >
> > > use
> > > > > remote systems' ripgrep binaries when running over TRAMP?
> > > >
> > > > Perhaps we could patch [emacs-rg] to do both?  Use the store >
> > > prefix if
> > > > it
> > > > exists, and fall back to searching in PATH?
> > > 
> > > What would be the advantage of that over just searching PATH to
> > > start with?
> > 
> > It will still work even if you don't have ripgrep specifically
> > installed.
> 
> Can you point me to the Guix documentation where the functionality you're
> describing is explained?  I have read through the description of package
> inputs in section 6.2.1 of Guix's manual, but I still do not explain what
> advantage patching the search path offers.

I'm not sure I can find a spot in the manual where it is detailed. It
comes down to the difference between "search for this program in PATH"
and "call this program located at this location". By calling the rg
at it's exact path rg doesn't need to be installed directly.

> My understanding is that if we want to preserve both local and
> remote-via-TRAMP functionality, we can either
> - just include ripgrep as a propagated input, or
> - include ripgrep as a propagated input _and_ patch the package to  look for
> ripgrep in a hardcoded location (for local) as well as  PATH (for TRAMP).

The second option is to include ripgrep as an input and patch the
package to look for it at a hardcoded location (for local) as well as
PATH (for TRAMP).

> Both options would have ripgrep included as propagated input.  As soon as
> ripgrep is installed in a user's profile, its binary will be available on
> PATH.  If that is correct, then I don't see any advantage to patching in a
> hardcoded path to ripgrep.
> 
> --
> Joseph LaFreniere

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 39384 <at> debbugs.gnu.org (full text, mbox):

From: "LaFreniere\, Joseph" <joseph <at> lafreniere.xyz>
To: 39384 <at> debbugs.gnu.org
Cc: Marius Bakke <mbakke <at> fastmail.com>, Efraim Flashner <efraim <at> flashner.co.il>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sat, 22 Feb 2020 17:08:51 -0600

[Message part 1 (text/plain, inline)]

Efraim Flashner <efraim <at> flashner.co.il> writes:
>> Can you point me to the Guix documentation where the 
>> functionality you're
>> describing is explained?  I have read through the description 
>> of package
>> inputs in section 6.2.1 of Guix's manual, but I still do not 
>> explain what
>> advantage patching the search path offers.
>
> I'm not sure I can find a spot in the manual where it is 
> detailed. It
> comes down to the difference between "search for this program in 
> PATH"
> and "call this program located at this location". By calling the 
> rg
> at it's exact path rg doesn't need to be installed directly.

A patch that includes ripgrep as non-propagated inputs is 
attached.

--
Joseph LaFreniere

[0001-gnu-Add-emacs-rg.patch (text/x-patch, attachment)]

Message #49 received at 39384-done <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: "LaFreniere, Joseph" <joseph <at> lafreniere.xyz>
Cc: 39384-done <at> debbugs.gnu.org, Marius Bakke <mbakke <at> fastmail.com>,
 Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Subject: Re: [bug#39384] [PATCH] gnu: Add emacs-rg.
Date: Sun, 23 Feb 2020 13:17:00 +0200

[Message part 1 (text/plain, inline)]

Looks good to me.
Patch pushed. Thanks!

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.