Hello Marius,

Thank you for review!

Marius Bakke <mbakke@fastmail.com> 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.

>> +
>> +@itemize
>> +@item Play, pause, stop the current played video.
>
> "currently playing"

Done.

>> +@item 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.

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

Done.

>> +@item Interactive and noninteractive volume control.
>> +@item Interactive navigation.
>> +@item 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.