[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#29099] [PATCH] gnu: Add kodi-cli.
From: |
Marius Bakke |
Subject: |
[bug#29099] [PATCH] gnu: Add kodi-cli. |
Date: |
Thu, 02 Nov 2017 21:00:14 +0100 |
User-agent: |
Notmuch/0.25.1 (https://notmuchmail.org) Emacs/25.3.1 (x86_64-pc-linux-gnu) |
Oleg Pykhalov <address@hidden> writes:
> Hello Marius,
>
> Thank you for review!
>
> Marius Bakke <address@hidden> writes:
>
>>> +(define-public kodi-cli
>>> + (let ((commit "104dc23b2a993c8e6db8c46f4f8bec24b146549b") ; Add support
>>> for
>>> + (revision "1")) ;
>>> `$HOME/.kodirc'.
>>> + (package
>>> + (name "kodi-cli")
>>> + (version (string-append "1.1-" revision "." (string-take commit 7)))
>>> + (source (origin
>>> + (method git-fetch)
>>> + (uri (git-reference (url
>>> "https://github.com/nawar/kodi-cli")
>>> + (commit commit)))
>>> + (sha256
>>> + (base32
>>> + "1xjhasc5gngfxpr1dlzy6q24w0wpdfjx12p43fanjppxw4i49n5p"))
>>> + (file-name (string-append name "-" version "-checkout"))))
>>> + (build-system trivial-build-system)
>>> + (inputs `(("bash" ,bash)))
>>> + (propagated-inputs `(("curl" ,curl)))
>>
>> There is only one reference to `curl` in the script, can you try to
>> substitute it with the absolute path and make it a regular input? It's
>> good to avoid propagation when we can.
>
> Done. I also added input mps-youtube and did the same.
Excellent.
>>> address@hidden Interactive and noninteractive volume control.
>>> address@hidden Interactive navigation.
>>> address@hidden Send text.
>>
>> What does this mean? Arbitrary commands, or text notifications?
>
> From my experience this allows you do the following:
>
> 1. Open Videos.
> 2. Add videos…
> 3. Select <none>.
> 4. Enter path by sending a command via 'kodi-cli -t PATH'.
>
> Maybe call this item of the feature list as
> “Send text to the Kodi keyboard”?
"Send text to the Kodi keyboard" is much better, thanks!
> New patch is attached.
[...]
> + (arguments
> + `(#:modules ((guix build utils))
> + #:builder
> + (begin
> + (use-modules (guix build utils))
> + (copy-recursively (assoc-ref %build-inputs "source") ".")
> + (substitute* "kodi-cli"
> + (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
> + "/bin/bash"))
> + (("output=\\$\\((curl)" all curl)
> + (string-append "output=$("
> + (assoc-ref %build-inputs "curl")
> + "/bin/" curl))
> + (("play_youtube `(mpsyt)" all mpsyt)
> + (string-append "play_youtube `"
> + (assoc-ref %build-inputs "mps-youtube")
> + "/bin/" mpsyt)))
FYI, you could also use (which "curl") etc from (guix build utils) here,
but I usually prefer this form.
[...]
> address@hidden
> address@hidden Play, pause, stop the current playing item.
> address@hidden Skip forward or backward in the current playing item.
These should be "currently playing".
> address@hidden Play or queue to the current list YouTube videos.
"... current list of YouTube videos."
(note the "of")
LGTM otherwise. Don't forget to add a copyright line for yourself.
Thanks!
signature.asc
Description: PGP signature