| Message ID | 20220205112019.31624-1-db@minikn.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#53797] gnu: Add emacs-app-launcher. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: > + "app-launcher defines the app-launcher-run-app command which uses Emacs > +standard completion feature to select an application installed on your machine > +and launch it.") What does ‘launching’ mean here? Is ‘launching an application’ functionally the same as running it (with 'fork'+'exec' or 'system' or whatever)? Does it have extra bells and whistles? It's a super vague word, e.g. when Xiden has been announced on guix- devel, one of the words that kept being repeated was 'launcher', but it wasn't defined anywhere in its practically empty manual. (Nowadays there's actually a manual and it does define launchers, but it didn't use to.) Greetings, Maxime.
Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: > + (let ((commit "80a9ed37892ee6e21fe44487ed11f66a15e3f440") > + (revision "1") > + (version "0.1")) It's the first version in Guix, so revision should be "0." Also, version "0.1" seems to be incorrect here, since there aren't any releases at all. So I'd start at "0.0". I could be wrong, but I don't think that the auto-updater (guix refresh -u) understands let-bound versions (let-bound commits and revisions are ok though), and you're only using the 'version' variable in a single place, so I'd move it to the 'version' field of the package record. Greetings, Maxime.
Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]:
> + "Use Emacs an app launcher")
Seems ingrammatical, "Use Emacs as an app launcher" would be better
though still rather vague.
Greetings,
Maxime.
Maxime Devos schreef op za 05-02-2022 om 13:12 [+0100]: > It's the first version in Guix, so revision should be "0." > Also, version "0.1" seems to be incorrect here, since there > aren't any releases at all. So I'd start at "0.0". Nevermind, I now see 'app-launcher.el' says Version: 0.1.
Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]:
> +(define-public emacs-app-launcher
app-launcher.el has (require 'xdg) (require 'cl-seq),
are these built-in to Emacs or do they need to be added to propagated-
inputs?
Greetings,
Maxime.
Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]:
> +(define-public emacs-app-launcher
app-launcher.el says it includes code from the Counsel package by Oleh
Krehel and the counsel source files have "Copyright (C) YEARS Foo"
lines, but app-launcher.el doesn't have those.
IIRC, the GPL has a clause that says you need to preserve copyright
lines, so this looks like a violation of the license to me. Can they
be re-added upstream?
Greetings,
Maxime.
On 2022-02-05 13:05, Maxime Devos wrote: > Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: >> + "app-launcher defines the app-launcher-run-app command which uses Emacs >> +standard completion feature to select an application installed on your machine >> +and launch it.") > > What does ‘launching’ mean here? Is ‘launching an application’ > functionally the same as running it (with 'fork'+'exec' or 'system' or > whatever)? Does it have extra bells and whistles? > > It's a super vague word, e.g. when Xiden has been announced on guix- > devel, one of the words that kept being repeated was 'launcher', but it > wasn't defined anywhere in its practically empty manual. (Nowadays > there's actually a manual and it does define launchers, but it didn't > use to.) > > Greetings, > Maxime. Hello. I don't know if that's what you wanted to hear but technically, the script is a big wrapper around `call-process-shell-command'. `app-launcher--action-function' can be set to whatever function to execute on the selected candidate, by default this is set to `app-launcher--action-function-default' which ultimately runs `call-process-shell-command'. See: https://github.com/SebastienWae/app-launcher/blob/main/app-launcher.el#L167 Is this enough information?
On 2022-02-05 13:13, Maxime Devos wrote: > Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: >> + "Use Emacs an app launcher") > > Seems ingrammatical, "Use Emacs as an app launcher" would be better > though still rather vague. > > Greetings, > Maxime. > This is indeed a mistake. But please, if you have a better synopsis/description in mind - share it. I'm always bad at these things. Especially since the package itself doesn't provide a whole lot of documentation since it's so simple.
On 2022-02-05 13:16, Maxime Devos wrote: > Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: >> +(define-public emacs-app-launcher > > app-launcher.el has (require 'xdg) (require 'cl-seq), > are these built-in to Emacs or do they need to be added to propagated- > inputs? > > Greetings, > Maxime. > I think they are built into emacs. I had no trouble with the package definition.
On 2022-02-05 13:20, Maxime Devos wrote: > Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: >> +(define-public emacs-app-launcher > > app-launcher.el says it includes code from the Counsel package by Oleh > Krehel and the counsel source files have "Copyright (C) YEARS Foo" > lines, but app-launcher.el doesn't have those. > > IIRC, the GPL has a clause that says you need to preserve copyright > lines, so this looks like a violation of the license to me. Can they > be re-added upstream? > > Greetings, > Maxime. I don't know if that's a violation sorry. Regarding adding it upstream. I don't think so. At least judging by the activity of the maintainer. See here: https://github.com/SebastienWae/app-launcher/issues There are open issues starting a year ago and the author hasn't replied to any of them. But I could try my luck and contact them to add the copyright notices.
Demis Balbach schreef op zo 06-02-2022 om 11:58 [+0100]: > > Demis Balbach schreef op za 05-02-2022 om 12:20 [+0100]: > > > + "Use Emacs an app launcher") > > > > Seems ingrammatical, "Use Emacs as an app launcher" would be better > > though still rather vague. > > > > Greetings, > > Maxime. > > > > This is indeed a mistake. But please, if you have a better > synopsis/description in mind - share it. I'm always bad at these > things. Especially since the package itself doesn't provide a whole > lot > of documentation since it's so simple. I'm also bad at descriptions and synopses. How about (synopsis "Start applications from Emacs") (description "This package allows starting applications from within Emacs with the @lisp{app-launcher-run-app} command. It supports autocompletion. It cannot be used to start arbitrary software; it assumes that the application has a .desktop file. This is the case for most, but certainly not all, graphical software.") ? ‘Application’ in the synopsis is not really specific, but the description explains what counts as ‘application’ to emacs-app- launcher. Greetings, Maxime
Demis Balbach schreef op zo 06-02-2022 om 11:59 [+0100]: > I think they are built into emacs. I had no trouble with the package > definition. OK.
Demis Balbach schreef op zo 06-02-2022 om 12:01 [+0100]: > I don't know if that's a violation sorry. Regarding adding it > upstream. I don't think so. At least judging by the activity of the > maintainer. See here: > https://github.com/SebastienWae/app-launcher/issues > > There are open issues starting a year ago and the author hasn't > replied > to any of them. But I could try my luck and contact them to add the > copyright notices. IANAL but it's such a minor and technical violation (the app- launcher.el even acknowledges Oleh Krehel, it just didn't say Copyright YEAR Oleh Krehel (or Copyright YEAR FSF, since it looks like Counsel has some copyright assignement to the FSF going on)) that I would consider making a token effort to correct it upstream sufficient. If not, it could be patched (with an origin snippet or a patch) in Guix. Greetings, Maxime.
On 2022-02-05, Demis Balbach wrote:
> +(define-public emacs-app-launcher
This was added in commit:
90174c77cf59fd9681eb7442a92015cd61c8495d gnu: Add emacs-app-launcher.
Marking as done.
live well,
vagrant
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm index 444b761116..4ce7525dee 100644 --- a/gnu/packages/emacs-xyz.scm +++ b/gnu/packages/emacs-xyz.scm @@ -2615,6 +2615,33 @@ (define-public emacs-async as a library for other Emacs packages.") (license license:gpl3+))) +(define-public emacs-app-launcher + (let ((commit "80a9ed37892ee6e21fe44487ed11f66a15e3f440") + (revision "1") + (version "0.1")) + (package + (name "emacs-app-launcher") + (version (git-version version revision commit)) + (source + (origin + (method git-fetch) + (uri + (git-reference + (url "https://github.com/SebastienWae/app-launcher") + (commit commit))) + (file-name (git-file-name name version)) + (sha256 + (base32 "1ywhfx8604ifmvcy2397bmvq2wj03jyqnm0g7lmqqi5p97rjbdgc")))) + (build-system emacs-build-system) + (home-page "https://github.com/SebastienWae/app-launcher") + (synopsis + "Use Emacs an app launcher") + (description + "app-launcher defines the app-launcher-run-app command which uses Emacs +standard completion feature to select an application installed on your machine +and launch it.") + (license license:gpl3+)))) + (define-public emacs-auctex (package (name "emacs-auctex")