diff mbox series

[bug#49457,v2] gnu: Add emacs-consult-notmuch.

Message ID 87r1gaw6u8.fsf@trop.in
State Accepted
Headers show
Series [bug#49457,v2] gnu: Add emacs-consult-notmuch. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Andrew Tropin July 7, 2021, 2:12 p.m. UTC
v2 uses propagated-inputs instead of inputs, because using
consult-notmuch without consult and notmuch doesn't make much sense.

Comments

Nicolas Goaziou July 7, 2021, 2:18 p.m. UTC | #1
Hello,

Andrew Tropin <andrew@trop.in> writes:

> v2 uses propagated-inputs instead of inputs, because using
> consult-notmuch without consult and notmuch doesn't make much sense.

Thank you. However, notmuch should be an input, there's no reason to
propagate it into user's profile.

Regards,
Andrew Tropin July 7, 2021, 2:33 p.m. UTC | #2
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Andrew Tropin <andrew@trop.in> writes:
>
>> v2 uses propagated-inputs instead of inputs, because using
>> consult-notmuch without consult and notmuch doesn't make much sense.
>
> Thank you. However, notmuch should be an input, there's no reason to
> propagate it into user's profile.

There is a (require 'notmuch) in consult-notmuch code, so we need
notmuch.el on the load-path, sadly, but we don't have a separate
emacs-notmuch package for it and therefore we have to propagate the
whole notmuch package here :(
Nicolas Goaziou July 8, 2021, 7:40 a.m. UTC | #3
Hello,

Andrew Tropin <andrew@trop.in> writes:

> v2 uses propagated-inputs instead of inputs, because using
> consult-notmuch without consult and notmuch doesn't make much sense.

I applied this patch with the following changes:

> +       (uri (git-reference
> +             (url "https://codeberg.org/jao/consult-notmuch.git")

I removed the ".git" suffix.

> +    (license license:gpl3+)
> +    (home-page "https://codeberg.org/jao/consult-notmuch")
> +    (synopsis "Search and preview notmuch emails using consult")

I properly capitalized synopsis (Notmuch, Consult).

> +    (description "\
> +This package provides two commands using consult to query notmuch
> +emails and present results either as single emails @code{consult-notmuch}
> +or full trees @code{consult-notmuch-tree}.
> +
> +The package also defines a narrowing source for @code{consult-buffer}, which
> +can be activated with @code{(add-to-list 'consult-buffer-sources
> +'consult-notmuch-buffer-source)}.")))

I shortened the description as it looked like an introduction part of
the manual instead.

I also reordered licence, home-page, synopsis and description parts.

Thank you!

Regards,
Nicolas Goaziou July 8, 2021, 7:41 a.m. UTC | #4
Hello,

Andrew Tropin <andrew@trop.in> writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> Thank you. However, notmuch should be an input, there's no reason to
>> propagate it into user's profile.
>
> There is a (require 'notmuch) in consult-notmuch code, so we need
> notmuch.el on the load-path, sadly, but we don't have a separate
> emacs-notmuch package for it and therefore we have to propagate the
> whole notmuch package here :(

Duh! You're right, of course.

Applied, as detailed in another message.

Regards,
Simon Tournier July 8, 2021, 9:13 a.m. UTC | #5
Hi,

(Sorry for this naive question.)

On Thu, 08 Jul 2021 at 09:41, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Andrew Tropin <andrew@trop.in> writes:
>> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>
>>> Thank you. However, notmuch should be an input, there's no reason to
>>> propagate it into user's profile.
>>
>> There is a (require 'notmuch) in consult-notmuch code, so we need
>> notmuch.el on the load-path, sadly, but we don't have a separate
>> emacs-notmuch package for it and therefore we have to propagate the
>> whole notmuch package here :(
>
> Duh! You're right, of course.

For my personal understanding because I am not familiar with Emacs
packages and package propagation.  The package ’emacs-magit’ has ’git’
as inputs and AFAIK it does not work without Git installed in the
profile.  Why is it different?

Cheers,
simon
Nicolas Goaziou July 8, 2021, 9:41 a.m. UTC | #6
Hello,

zimoun <zimon.toutoune@gmail.com> writes:

> For my personal understanding because I am not familiar with Emacs
> packages and package propagation.  The package ’emacs-magit’ has ’git’
> as inputs and AFAIK it does not work without Git installed in the
> profile.  Why is it different?

It is different because Notmuch is both an external executable and an
Emacs library. Git, OTOH, does not ship with an Elisp library.

However, I think it is a bug if Emacs Magit does not work with git in
profile. We should modify the source so both magit-git-executable and
magit-gitk-executable point to those from inputs.

I suggest to open a new bug report about it, WDYT?

Regards,
Andrew Tropin July 8, 2021, 10:14 a.m. UTC | #7
zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> (Sorry for this naive question.)
>
> On Thu, 08 Jul 2021 at 09:41, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
>> Andrew Tropin <andrew@trop.in> writes:
>>> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>>
>>>> Thank you. However, notmuch should be an input, there's no reason to
>>>> propagate it into user's profile.
>>>
>>> There is a (require 'notmuch) in consult-notmuch code, so we need
>>> notmuch.el on the load-path, sadly, but we don't have a separate
>>> emacs-notmuch package for it and therefore we have to propagate the
>>> whole notmuch package here :(
>>
>> Duh! You're right, of course.
>
> For my personal understanding because I am not familiar with Emacs
> packages and package propagation.  The package ’emacs-magit’ has ’git’
> as inputs and AFAIK it does not work without Git installed in the
> profile.  Why is it different?

The magit package itself will be loaded correctly, because its emacs
package requirements are satisfied and even some functions will work,
for example magit-dispatch will show a transient interface.  Other
functions that rely on git binary will throw an error and there are two
solutions for that: make a git a propagated input or set
`magit-git-executable` to a path to the git binary inside store, which
will make magit work with git as a regular input.

notmuch in contrast to git provides not only notmuch binary, but also
notmuch.el.  consult-notmuch requires only notmuch.el, which have to be
propagated and doesn't depend on notmuch binary, but because notmuch.el
is a part of notmuch package the binary will be propagated too.  The
possible solution here is to split notmuch into two packages: notmuch and
emacs-notmuch, so we will be able to set `notmuch-command` variable for
notmuch.el and have notmuch as an input, but not installed in the profile.
Andrew Tropin July 8, 2021, 10:22 a.m. UTC | #8
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Andrew Tropin <andrew@trop.in> writes:
>
>> v2 uses propagated-inputs instead of inputs, because using
>> consult-notmuch without consult and notmuch doesn't make much sense.
>
> I applied this patch with the following changes:
>
>> +       (uri (git-reference
>> +             (url "https://codeberg.org/jao/consult-notmuch.git")
>
> I removed the ".git" suffix.
>
>> +    (license license:gpl3+)
>> +    (home-page "https://codeberg.org/jao/consult-notmuch")
>> +    (synopsis "Search and preview notmuch emails using consult")
>
> I properly capitalized synopsis (Notmuch, Consult).
>
>> +    (description "\
>> +This package provides two commands using consult to query notmuch
>> +emails and present results either as single emails @code{consult-notmuch}
>> +or full trees @code{consult-notmuch-tree}.
>> +
>> +The package also defines a narrowing source for @code{consult-buffer}, which
>> +can be activated with @code{(add-to-list 'consult-buffer-sources
>> +'consult-notmuch-buffer-source)}.")))
>
> I shortened the description as it looked like an introduction part of
> the manual instead.

It was complete) but shorter version is also fine.

>
> I also reordered licence, home-page, synopsis and description parts.
>
> Thank you!

Thank you!)
Nicolas Goaziou July 8, 2021, 10:32 a.m. UTC | #9
Andrew Tropin <andrew@trop.in> writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> I shortened the description as it looked like an introduction part of
>> the manual instead.
>
> It was complete) but shorter version is also fine.

Yes, it was complete. 

What I meant is the following: when I look at a description, I try to
see what kind of question it answers. If it answers the question: "Do
I want to install this package?", that's good. But if it answers: "How
can I start using this?", then the description belongs to a manual
instead.

For example,

    which can be activated with @code{(add-to-list
    'consult-buffer-sources 'consult-notmuch-buffer-source)}.

clearly belongs to the second question. So does the name of functions,
in general.
Kyle Meyer July 8, 2021, 11:28 a.m. UTC | #10
Nicolas Goaziou writes:

> However, I think it is a bug if Emacs Magit does not work with git in
> profile. We should modify the source so both magit-git-executable and
> magit-gitk-executable point to those from inputs.
>
> I suggest to open a new bug report about it, WDYT?

Rewriting magit-git-executable to an absolute file name will break Magit
for those that use it over TRAMP.
Nicolas Goaziou July 8, 2021, 11:38 a.m. UTC | #11
Hello,

Kyle Meyer <kyle@kyleam.com> writes:

> Nicolas Goaziou writes:
>
>> However, I think it is a bug if Emacs Magit does not work with git in
>> profile. We should modify the source so both magit-git-executable and
>> magit-gitk-executable point to those from inputs.
>>
>> I suggest to open a new bug report about it, WDYT?
>
> Rewriting magit-git-executable to an absolute file name will break Magit
> for those that use it over TRAMP.

Point taken. However, such users can fix it by modifying the
`magit-git-executable' defcustom again, can't they?

So, the situation would arguably not be worse that what it is currently,
since it simply doesn't work for users relying on a local installation
of git, and those are probably more common.

Also, a third option: since we're not making use of git input at
runtime, why should it be an input at all? If it is needed at build
time, a native-input should suffice.

Regards,
Kyle Meyer July 9, 2021, 12:13 a.m. UTC | #12
Nicolas Goaziou writes:

> Kyle Meyer <kyle@kyleam.com> writes:
>
>> Nicolas Goaziou writes:
>>
>>> However, I think it is a bug if Emacs Magit does not work with git in
>>> profile. We should modify the source so both magit-git-executable and
>>> magit-gitk-executable point to those from inputs.
>>>
>>> I suggest to open a new bug report about it, WDYT?
>>
>> Rewriting magit-git-executable to an absolute file name will break Magit
>> for those that use it over TRAMP.
>
> Point taken. However, such users can fix it by modifying the
> `magit-git-executable' defcustom again, can't they?

Sure.

Given the number of "oops, some change didn't consider tramp
compatibility" bugs that crop up in Magit, my guess is that Magit is
widely used over TRAMP.  So, potentially many users are affected, but I
agree it's an easy fix once they spend time to debug what the issue is.
(That collectively could amount to a lot of wasted time, so in my eyes
it depends on what the practical problem being fixed is.)

> So, the situation would arguably not be worse that what it is currently,
> since it simply doesn't work for users relying on a local installation
> of git, and those are probably more common.

Hmm, I'm not clear on what the current problem is; a user will typically
have "git" on their path one way or the other.  Given Magit's heavily
used, I would have expected to see many Guix bug reports about about it
if it were a problem in practice.

But 1) perhaps I'm missing something or 2) perhaps for philosophical
reasons or to promote best practices rewriting magit-git-executable is
worth the breakage.  My drive-by comment was just to mention the
possible breakage [*].  (I don't have a personal stake in what's done
because I don't install Magit from Guix.)

> Also, a third option: since we're not making use of git input at
> runtime, why should it be an input at all? If it is needed at build
> time, a native-input should suffice.

Yeah, that sounds right (though I haven't tested it).


[*] And it might not break things for much longer.  On Magit's end,
    Jonas recently floated the idea of not using magit-git-executable
    when using TRAMP.

    https://github.com/magit/magit/issues/4433#issuecomment-875964528
diff mbox series

Patch

From 7ba18d68face76d8858ad67a6113f5773fa45cec Mon Sep 17 00:00:00 2001
From: Andrew Tropin <andrew@trop.in>
Date: Wed, 7 Jul 2021 16:55:41 +0300
Subject: [PATCH v2] gnu: Add emacs-consult-notmuch.

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

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 9591a4e6e7..492572322c 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -8084,6 +8084,36 @@  Emacs completion function completing-read, which allows quickly selecting from a
 list of candidates.")
     (license license:gpl3+)))
 
+(define-public emacs-consult-notmuch
+  (package
+    (name "emacs-consult-notmuch")
+    (version "0.3")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://codeberg.org/jao/consult-notmuch.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "19flyh3v1xm2zswzjkvjbijvpbq5r8isafza4fd0yicvqbjyklhx"))))
+    (build-system emacs-build-system)
+    (propagated-inputs
+     `(("emacs-consult" ,emacs-consult)
+       ("notmuch" ,notmuch)))
+    (license license:gpl3+)
+    (home-page "https://codeberg.org/jao/consult-notmuch")
+    (synopsis "Search and preview notmuch emails using consult")
+    (description "\
+This package provides two commands using consult to query notmuch
+emails and present results either as single emails @code{consult-notmuch}
+or full trees @code{consult-notmuch-tree}.
+
+The package also defines a narrowing source for @code{consult-buffer}, which
+can be activated with @code{(add-to-list 'consult-buffer-sources
+'consult-notmuch-buffer-source)}.")))
+
 (define-public emacs-marginalia
   (package
     (name "emacs-marginalia")
-- 
2.32.0