guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#29099] [PATCH] gnu: Add kodi-cli.


From: Oleg Pykhalov
Subject: [bug#29099] [PATCH] gnu: Add kodi-cli.
Date: Thu, 02 Nov 2017 12:14:54 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)

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.

>> +      (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")))
>> +           (install-file "kodi-cli" (string-append %output "/bin")))))
>
> (install-file ...) has an unspecified return value, so please return #t here.

Done.

>> +      (home-page "https://github.com/nawar/kodi-cli";)
>> +      (synopsis "Bash script to send commands to Kodi using JSON RPC")
>
> Maybe just "Control Kodi from the command line".

Done.

>> +      (description "@code{kodi-cli} provides the Bash script to send 
>> commands to
>> +Kodi using JSON RPC.
>
> Similarly, this sentence can be reduced to something like
> "@code{kodi-cli} is a tool for sending commands to a Kodi server using
> JSON-RPC.".  Bash is an boring implementation detail IMO. :-)

Done.

>> +
>> +Feautures:
>       ^ Stray 'u' character.

Done.

>> +
>> address@hidden
>> address@hidden Play, pause, stop the current played video.
>
> "currently playing"

Done.

>> address@hidden Skip forward or backward in the current played video.
>
> Same here.  Maybe it should be "currently playing item", since I assume
> this works for music as well.

Done.

>> address@hidden Play or queue to the current list YouTube video.
>
> "List of YouTube videos"?

Done.

>> 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”?


New patch is attached.

Attachment: 0001-gnu-Add-kodi-cli.patch
Description: Text Data

>> address@hidden Toggle fullscreen.
>> address@hidden Update or clean Kodi libraries.
>> address@hidden itemize\n")
>> +      (license license:gpl2+))))
>
> LGTM with some cosmetic improvements.  And welcome!
>
> By the way, if you use Kodi from Guix, feel free to update the 18
> snapshot if you can.  My media PC died a while back, and I didn't get a
> replacement yet, so I have not been able to maintain the Kodi package.

OK.  I do, but not much.  Primarily use it on a “media tablet”.  :-)

Oleg.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]