GNU bug report logs - #78597
[PATCH 0/2] Update mpv configuration

Previous Next

Full log

View this message in rfc822 format

From: Tomas Volf <~@wolfsden.cz>
To: Sergey Trofimov <sarg <at> sarg.org.ru>
Cc: 78597 <at> debbugs.gnu.org
Subject: [bug#78597] [bug#74801] [PATCH v4] gnu: home: services: Add home-mpv-service-type.
Date: Tue, 27 May 2025 14:44:32 +0200

Sergey Trofimov <sarg <at> sarg.org.ru> writes:

> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
>> Hi,
>>
>> could you give https://issues.guix.gnu.org/78597 a try?
>
> list-of-files should be joined with ":"
>
> diff --git a/gnu/home/services/mpv.scm b/gnu/home/services/mpv.scm
> index 50d3f06c46..54c6b86680 100644
> --- a/gnu/home/services/mpv.scm
> +++ b/gnu/home/services/mpv.scm
> @@ -227,7 +227,7 @@ (define (mpv/list-of-string? lst)
>  (define (serialize-mpv/list-of-file field-name lst)
>    #~(string-append #$(symbol->string field-name)
>                     "="
> -                   (string-join '#$lst ",")
> +                   (string-join '#$lst ":")
>                     "\n"))
>  (define (mpv/list-of-file? lst)
>    (every mpv/file? lst))
>
>
> it works with this patch in place, here is my conf for reference:
>
> (service home-mpv-service-type
>                   (make-home-mpv-configuration
>                    #:global
>                    (make-mpv-profile-configuration
>                     #:audio-display 'no
>                     #:hwdec '("auto")
>                     #:fullscreen? #t
>                     #:sub-font-size 24
>                     #:x11-name "mpv"
>                     #:ytdl-format "bestvideo[height<=?900]+bestaudio"
>                     #:osd-fonts-dir (file-append (pkg "mpv-uosc") "/share/mpv/fonts")
>                     #:script-opts '(("network" . "yes")) ; thumbfast
>                     #:input-conf
>                     (plain-file "input.conf"
>                                 (string-join
>                                  '("q quit-watch-later"
>                                    "h seek -5" "l seek 5"
>                                    "H seek -60" "L seek 60")
>                                  "\n"))
>                     #:scripts (list
>                                (file-append (pkg "mpv-mpris") "/lib/mpris.so")
>                                (file-append (pkg "mpv-thumbfast") "/share/mpv/scripts/thumbfast.lua")
>                                (file-append (pkg "mpv-uosc") "/share/mpv/scripts/uosc")))))

Thank you for testing, I have sent a v2 that uses the ":".

> One last thing - I'd allow mpv/string to take symbols:
> `#:hwdec '(auto)` or `#:script-opts '((network . yes))` look better

I like the split of using symbols for enums with well defined values,
and strings for arbitrary inputs.

While I agree that #:hwdec '(auto) might be more aesthetically pleasing
compared to #:hwdec '("auto"), for non-list values, the difference is
minimal '/home/user/.mpv.cookies and "/home/user/.mpv.cookies" are close
enough.  And, for list values, it would allow mixing strings and symbols
in a single list which is somewhat meh.

So, ugh, I probably prefer the current, more strict, approach, but you
are of course free to send a patch and see if it will be accepted (I
mean, it probably will ¯\_(ツ)_/¯).

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.