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

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

[LaFreniere, Joseph]

[-- Attachment #1: Type: text/plain, Size: 93 bytes --]

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

--
Joseph LaFreniere

[-- Attachment #2: 0001-gnu-Add-emacs-rg.patch --]
[-- Type: text/x-patch, Size: 1898 bytes --]

From d3e76096a6c44764d51ef06aedb171682f92c85e Mon Sep 17 00:00:00 2001
From: Joseph LaFreniere <joseph@lafreniere.xyz>
Date: Sat, 1 Feb 2020 14:23:36 -0600
Subject: [PATCH] gnu: Add emacs-rg.

* gnu/packages/emacs-xyz.scm (emacs-rg): New variable.
---
 gnu/packages/emacs-xyz.scm | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index f95ad26e4b..c8b87ce712 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -2751,6 +2751,35 @@ column by drawing a thin line down the length of the editing window.")
      "This Emacs package allows managing multiple grep buffers.")
     (license license:gpl3+)))
 
+(define-public emacs-rg
+  (package
+    (name "emacs-rg")
+    (version "1.8.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/dajva/rg.el.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "0k7x5z7mh9flwih35cqy8chs54rack3nswdcpw5wcpgv6xim227y"))))
+    (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}")
+    (description
+     "An Emacs search package based on the @code{ripgrep} command line
+tool. It allows you to interactively create searches, doing automatic searches
+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’
+needs.")
+    (license license:gpl3+)))
+
 (define-public emacs-inf-ruby
   (package
     (name "emacs-inf-ruby")
-- 
2.25.0

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

[Nicolas Goaziou]

Hello,

"LaFreniere, Joseph" <joseph@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

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

[LaFreniere, Joseph]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

Thank you for the fast feedback!

Nicolas Goaziou <mail@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-emacs-rg.patch --]
[-- Type: text/x-patch, Size: 2015 bytes --]

From 9a17c333ceee4bb72dcb1ee36aaf45a7d2ce4276 Mon Sep 17 00:00:00 2001
From: Joseph LaFreniere <joseph@lafreniere.xyz>
Date: Sat, 1 Feb 2020 14:23:36 -0600
Subject: [PATCH] gnu: Add emacs-rg.

* gnu/packages/emacs-xyz.scm (emacs-rg): New variable.
---
 gnu/packages/emacs-xyz.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index f95ad26e4b..29928364e0 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -134,6 +134,7 @@
   #:use-module (gnu packages package-management)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pdf)
+  #:use-module (gnu packages rust-apps)
   #:use-module (gnu packages scheme)
   #:use-module (gnu packages speech)
   #:use-module (gnu packages xiph)
@@ -2751,6 +2752,32 @@ column by drawing a thin line down the length of the editing window.")
      "This Emacs package allows managing multiple grep buffers.")
     (license license:gpl3+)))
 
+(define-public emacs-rg
+  (package
+    (name "emacs-rg")
+    (version "1.8.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/dajva/rg.el.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0k7x5z7mh9flwih35cqy8chs54rack3nswdcpw5wcpgv6xim227y"))))
+    (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 "Search tool based on @code{ripgrep}")
+    (description
+     "@code{rg} is an Emacs search package based on the @code{ripgrep} command
+line tool.  It allows one to interactively search based on the editing context
+then refine or modify the search results.")
+    (license license:gpl3+)))
+
 (define-public emacs-inf-ruby
   (package
     (name "emacs-inf-ruby")
-- 
2.25.0

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

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Sun, Feb 02, 2020 at 08:21:52AM -0600, LaFreniere, Joseph wrote:
> Thank you for the fast feedback!
> 
> Nicolas Goaziou <mail@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@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[LaFreniere, Joseph]

Efraim Flashner <efraim@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

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

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

On Sun, Feb 02, 2020 at 09:41:41PM -0600, LaFreniere, Joseph wrote:
> 
> Efraim Flashner <efraim@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@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[LaFreniere, Joseph]

Efraim Flashner <efraim@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

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

[Efraim Flashner]

On February 5, 2020 3:08:11 AM UTC, "LaFreniere, Joseph" <joseph@lafreniere.xyz> wrote:
>
>Efraim Flashner <efraim@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.

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

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

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

> Efraim Flashner <efraim@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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

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

[LaFreniere, Joseph]

Marius Bakke <mbakke@fastmail.com> writes:
> "LaFreniere\, Joseph" <joseph@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

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

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph wrote:
> 
> Marius Bakke <mbakke@fastmail.com> writes:
> > "LaFreniere\, Joseph" <joseph@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@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[LaFreniere, Joseph]

Efraim Flashner <efraim@flashner.co.il> writes:
> On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph 
> wrote:
>>
>> Marius Bakke <mbakke@fastmail.com> writes:
>> > "LaFreniere\, Joseph" <joseph@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

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

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]

On Sat, Feb 08, 2020 at 04:23:08PM -0600, LaFreniere, Joseph wrote:
> 
> Efraim Flashner <efraim@flashner.co.il> writes:
> > On Thu, Feb 06, 2020 at 06:47:23PM -0600, LaFreniere, Joseph wrote:
> > > 
> > > Marius Bakke <mbakke@fastmail.com> writes:
> > > > "LaFreniere\, Joseph" <joseph@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@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[LaFreniere, Joseph]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]


Efraim Flashner <efraim@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-emacs-rg.patch --]
[-- Type: text/x-patch, Size: 2411 bytes --]

From 2035b630c949364e97f08c4ddfd770a3bdfea095 Mon Sep 17 00:00:00 2001
From: Joseph LaFreniere <joseph@lafreniere.xyz>
Date: Sat, 1 Feb 2020 14:23:36 -0600
Subject: [PATCH] gnu: Add emacs-rg.

* gnu/packages/emacs-xyz.scm (emacs-rg): New variable.
---
 gnu/packages/emacs-xyz.scm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 3a026bec9a..281ca76119 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -135,6 +135,7 @@
   #:use-module (gnu packages package-management)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pdf)
+  #:use-module (gnu packages rust-apps)
   #:use-module (gnu packages scheme)
   #:use-module (gnu packages speech)
   #:use-module (gnu packages xiph)
@@ -2779,6 +2780,43 @@ column by drawing a thin line down the length of the editing window.")
      "This Emacs package allows managing multiple grep buffers.")
     (license license:gpl3+)))
 
+(define-public emacs-rg
+  (package
+    (name "emacs-rg")
+    (version "1.8.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/dajva/rg.el.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0k7x5z7mh9flwih35cqy8chs54rack3nswdcpw5wcpgv6xim227y"))))
+    (build-system emacs-build-system)
+    (arguments
+     '(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'hardcode-rg-path
+           ;; Hardcode the path to ripgrep.
+           (lambda _
+             (let ((file "rg.el"))
+               (chmod file #o644)
+               (emacs-substitute-sexps file
+                 ("(defcustom rg-executable" (which "rg")))))))))
+    (propagated-inputs
+     `(("emacs-s" ,emacs-s)
+       ("emacs-wgrep" ,emacs-wgrep)))
+    (inputs
+     `(("ripgrep" ,ripgrep)))
+    (home-page "https://rgel.readthedocs.io/en/latest/")
+    (synopsis "Search tool based on @code{ripgrep}")
+    (description
+     "@code{rg} is an Emacs search package based on the @code{ripgrep} command
+line tool.  It allows one to interactively search based on the editing context
+then refine or modify the search results.")
+    (license license:gpl3+)))
+
 (define-public emacs-inf-ruby
   (package
     (name "emacs-inf-ruby")
-- 
2.25.1

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

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

Looks good to me.
Patch pushed. Thanks!

-- 
Efraim Flashner   <efraim@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]