Add helper for .desktop file creation?

Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

Hi,

Quite a bunch of packages need to provide their own .desktop since
upstream does not provide them.  It's very evident in games.scm in
particular.

It seems to be a good opportunity to factor this out.  What about the
following?

--8<---------------cut here---------------start------------->8---
(define* (make-desktop-file destination #:key
                            (encoding "UTF-8")
                            name
                            (generic-name name)
                            comment
                            exec
                            icon
                            (startup-notify "true")
                            (terminal "false")
                            (type "Application")
                            (categories "Application"))
  "Create a desktop file at DESTINATION for executable EXEC with name NAME.
Other arguments are optional."
  (let ((maybe-print (lambda (key value)
                       (if value
                           (string-append key "=" value "\n")
                           ""))))
    (mkdir-p (dirname destination))
    (with-output-to-file destination
      (lambda _
        (display
         (string-append
          "[Desktop Entry]" "\n"
          "Encoding=" encoding "\n"
          "Name=" name "\n"
          "GenericName=" generic-name "\n"
          (maybe-print "Comment" comment)
          "Exec=" exec "\n"
          (maybe-print "Icon" icon)
          "StartupNotify=" startup-notify "\n"
          "Terminal=" terminal "\n"
          "Type=" type "\n"
          "Categories=" categories "\n"))))))
--8<---------------cut here---------------end--------------->8---

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Hello,

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Quite a bunch of packages need to provide their own .desktop since
> upstream does not provide them.  It's very evident in games.scm in
> particular.

Interestingly, I had the same itch recently.

> It seems to be a good opportunity to factor this out.  What about the
> following?
>
> --8<---------------cut here---------------start------------->8---
> (define* (make-desktop-file destination #:key
>                             (encoding "UTF-8")
>                             name
>                             (generic-name name)
>                             comment
>                             exec
>                             icon
>                             (startup-notify "true")
>                             (terminal "false")
>                             (type "Application")
>                             (categories "Application"))
>   "Create a desktop file at DESTINATION for executable EXEC with name NAME.
> Other arguments are optional."
>   (let ((maybe-print (lambda (key value)
>                        (if value
>                            (string-append key "=" value "\n")
>                            ""))))
>     (mkdir-p (dirname destination))
>     (with-output-to-file destination
>       (lambda _
>         (display
>          (string-append
>           "[Desktop Entry]" "\n"
>           "Encoding=" encoding "\n"
>           "Name=" name "\n"
>           "GenericName=" generic-name "\n"
>           (maybe-print "Comment" comment)
>           "Exec=" exec "\n"
>           (maybe-print "Icon" icon)
>           "StartupNotify=" startup-notify "\n"
>           "Terminal=" terminal "\n"
>           "Type=" type "\n"
>           "Categories=" categories "\n"))))))
> --8<---------------cut here---------------end--------------->8---

It looks interesting. I have a couple of suggestions, if you don't mind:
- some items are missing, e.g., "Keywords". 

  As you may know, you can have a look at
  <https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-
  spec-latest.html> for a full list.

- some items you use a list of strings instead of a string (e.g.,
  "Keywords", "Categories"),

- it would be nice to handle localized values for keys. For example
  `drascula' package uses "Comment[fr]". It could possibly be
  implemented with an alist as the value.

I know Nix provides such a function, but I haven't looked at its
implementation yet.

Regards,

-- 
Nicolas Goaziou

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>>         (display
>>          (string-append
>>           "[Desktop Entry]" "\n"
>>           "Encoding=" encoding "\n"

Also, "Encoding" is deprecated. It may be worth using `maybe-print' for
this one.

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

Thanks for the feedback!

You are absolutely right, my initial implementation is not ready for
merge, but it's already useful as a proof of concept.

Would you happen to know where this is implemented in Nix?
Otherwise I'll look at the doc you've linked.

Or... I'm thinking we could do even better: use #:allow-other-keys then
generate the entry from the key symbol (e.g. #:categories would generate
"Categories").  This way the function would not have to hard-code the keys.

Regarding your point on the string vs. list values, I suggest we accept
both for all arguments and we string-join the lists.

Thoughts?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> You are absolutely right, my initial implementation is not ready for
> merge, but it's already useful as a proof of concept.

Certainly. Also, it is a step in the right direction.

> Would you happen to know where this is implemented in Nix?

<https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/make-desktopitem/default.nix>

It is very straightforward, actually. There is not much to borrow.

> Or... I'm thinking we could do even better: use #:allow-other-keys then
> generate the entry from the key symbol (e.g. #:categories would generate
> "Categories").  This way the function would not have to hard-code the
> keys.

Making typos would be easier, then. I'd rather have a more limited, yet
more robust, function. I.e., I think it is better to stick to standard
keys. Otherwise, it adds little over current solution.

> Regarding your point on the string vs. list values, I suggest we accept
> both for all arguments and we string-join the lists.

If we go the "limited" route, we should also pay attention to expected
value types, i.e., only accept list of strings for entries with
"string(s)" values. Semicolons would need to be escaped when a list of
strings is allowed, too. 

In a nutshell, my suggestion would be:

- string, boolean, numeric : string, possibly with a check for boolean
  (throw an error if not "true" or "false"),

- string(s) : string, or list of strings, semicolons are escaped
  automatically,

- localestring : alist with "lang" as keys and strings as values, with
  a default key (possibly #f), or a plain string if there is only the
  default key.

This is more work than on the #:allow-other-keys options, but I think it
would make for a better tool.

WDYT?

Re: Add helper for .desktop file creation?

[Amin Bandali]

Hi Pierre, Nicolas,

Pierre Neidhardt <mail@ambrevar.xyz> writes:

[...]

> Would you happen to know where this is implemented in Nix?
> Otherwise I'll look at the doc you've linked.
>

It seems to be implemented here [1].  For examples of use, look for
“makeDesktopItem”.

[1]: https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/make-desktopitem/default.nix

Best,
amin

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> - string, boolean, numeric : string, possibly with a check for boolean
>   (throw an error if not "true" or "false"),

or simply:

- boolean : #t, #f
- numeric : number
- string : string

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> Or... I'm thinking we could do even better: use #:allow-other-keys then
>> generate the entry from the key symbol (e.g. #:categories would generate
>> "Categories").  This way the function would not have to hard-code the
>> keys.
>
> Making typos would be easier, then. I'd rather have a more limited, yet
> more robust, function. I.e., I think it is better to stick to standard
> keys. Otherwise, it adds little over current solution.

Agreed, makes sense.  It's a little more work in one central spot, while
it eases code and maintainability everywhere else.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

Hi,

I agree that it would be nice to have such a helper function.  But we should
also report a bug upstream for the affected packages.  Not having a desktop
file means that a normal user (TM) can't start the program.
I'd say that's a pretty bad bug.

As a stopgap, we can work around it, but it makes much more sense to fix it at
the source long term.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Just thought of maybe the best of both approaches:  use a #:rest
argument to iterate over all keys.  This way we don't allow unspecified
keys (no risk for typos) and the code of the function remains lean.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]

I came up with the following function, it seems to work well (need to
test a little more though).

Before I could submit a patch, I was wondering where I should place it:
it seems that placing it in guix/utils.scm triggers a whole world
rebuild.

Is there a way around it or should I send this patch to core-updates?

--8<---------------cut here---------------start------------->8---
(define* (make-desktop-entry-file destination #:key
                                  (type "Application") ; One of "Application", "Link" or "Directory".
                                  (version "1.1")
                                  name
                                  (generic-name name)
                                  (no-display #f)
                                  comment
                                  icon
                                  (hidden #f)
                                  only-show-in
                                  not-show-in
                                  (d-bus-activatable #f)
                                  try-exec
                                  exec
                                  path
                                  (terminal #f)
                                  actions
                                  mime-type
                                  (categories "Application")
                                  implements
                                  keywords
                                  (startup-notify #t)
                                  startup-w-m-class
                                  #:rest all-args)
  "Create a desktop entry file at DESTINATION.
You must specify NAME.

Values can be booleans, numbers, strings or list of strings.

Additionally, locales can be specified with an alist where the key is the
locale.  The #f key specifies the default.  Example:

  #:name '((#f \"I love Guix\") (\"fr\" \"J'aime Guix\"))

produces

  Name=I love Guix
  Name[fr]=J'aime Guix

For a complete description of the format, see the specifications at
https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html."
  (define (escape-semicolon s)
    (string-join (string-split s #\;) "\\;"))
  (define* (parse key value #:optional locale)
    (set! value (match value
                  (#t "true")
                  (#f "false")
                  ((?  number? n) n)
                  ((?  string? s) (escape-semicolon s))
                  ((?  list? value)
                   (catch 'wrong-type-arg
                     (lambda () (string-join (map escape-semicolon value) ";"))
                     (lambda args (error "List arguments can only contain strings: ~a" args))))
                  (_ (error "Value must be a boolean, number, string or list of strings"))))
    (format #t "~a=~a~%"
            (if locale
                (format #f "~a[~a]" key locale)
                key)
            value))

  (define key-error-message "This procedure only takes key arguments beside DESTINATION")

  (unless name
    (error "Missing NAME key argument"))
  (unless (member #:type all-args)
    (set! all-args (append (list #:type type) all-args)))
  (mkdir-p (dirname destination))

  (with-output-to-file destination
    (lambda ()
      (format #t "[Desktop Entry]~%")
      (let loop ((args all-args))
        (match args
          (() #t)
          ((_) (error key-error-message))
          ((key value . ...)
           (unless (keyword? key)
             (error key-error-message))
           (set! key
                 (string-join (map string-titlecase
                                   (string-split (symbol->string
                                                  (keyword->symbol key))
                                                 #\-))
                              ""))
           (match value
             (((_ . _) . _)
              (for-each (lambda (locale-subvalue)
                          (parse key
                                 (if (and (list? (cdr locale-subvalue))
                                          (= 1 (length (cdr locale-subvalue))))
                                     ;; Support both proper and improper lists for convenience.
                                     (cadr locale-subvalue)
                                     (cdr locale-subvalue))
                                 (car locale-subvalue)))
                        value))
             (_
              (parse key value)))
           (loop (cddr args))))))))
--8<---------------cut here---------------end--------------->8---

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Hello,

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> I came up with the following function, it seems to work well (need to
> test a little more though).
>
> Before I could submit a patch, I was wondering where I should place it:
> it seems that placing it in guix/utils.scm triggers a whole world
> rebuild.
>
> Is there a way around it or should I send this patch to core-updates?

Not what you're asking for, but I had a few comments about the
implementation:

> --8<---------------cut here---------------start------------->8---
> (define* (make-desktop-entry-file destination #:key
>                                   (type "Application") ; One of "Application", "Link" or "Directory".
>                                   (version "1.1")
>                                   name
>                                   (generic-name name)
>                                   (no-display #f)

What about only providing default values only for mandatory keys, i.e.,
only "type" (name is also mandatory, but it has no default value).
I think the function currently adds entries that are not necessary.

>   (define* (parse key value #:optional locale)
>     (set! value (match value
>                   (#t "true")
>                   (#f "false")
>                   ((?  number? n) n)
>                   ((?  string? s) (escape-semicolon s))
>                   ((?  list? value)
>                    (catch 'wrong-type-arg
>                      (lambda () (string-join (map escape-semicolon value) ";"))
>                      (lambda args (error "List arguments can only contain strings: ~a" args))))
>                   (_ (error "Value must be a boolean, number, string or list of strings"))))
>     (format #t "~a=~a~%"
>             (if locale
>                 (format #f "~a[~a]" key locale)
>                 key)
>             value))

I wonder if it wouldn't be better to stick to the specification. For
example :comment expects a string, or an alist: shouldn't the function
return an error if its value is something else?

It requires us to hard-code the allow value types in the function, but
Not every packager knows these specifications, and it could help them
a bit. Also, we can limit keys to allowed ones, for increased typo
checking.

>            (set! key
>                  (string-join (map string-titlecase
>                                    (string-split (symbol->string
>                                                   (keyword->symbol key))
>                                                  #\-))
>                               ""))

The docstring may explain that, e.g., compound :mime-type key becomes
MimeType.

In any case, it looks nice and useful.

Regards,

-- 
Nicolas Goaziou

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Not what you're asking for, but I had a few comments about the
> implementation:

Thanks for the feedback!

>> --8<---------------cut here---------------start------------->8---
>> (define* (make-desktop-entry-file destination #:key
>>                                   (type "Application") ; One of "Application", "Link" or "Directory".
>>                                   (version "1.1")
>>                                   name
>>                                   (generic-name name)
>>                                   (no-display #f)
>
> What about only providing default values only for mandatory keys, i.e.,
> only "type" (name is also mandatory, but it has no default value).
> I think the function currently adds entries that are not necessary.

Nope, the current function only forces "Type" (the only mandatory
field), everything else is omitted unless the key is explicitly set by
the user.  This is because I looped over ALL-ARGS (the #:rest argument)
and I force-added #:type to it.

>>   (define* (parse key value #:optional locale)
>>     (set! value (match value
>>                   (#t "true")
>>                   (#f "false")
>>                   ((?  number? n) n)
>>                   ((?  string? s) (escape-semicolon s))
>>                   ((?  list? value)
>>                    (catch 'wrong-type-arg
>>                      (lambda () (string-join (map escape-semicolon value) ";"))
>>                      (lambda args (error "List arguments can only contain strings: ~a" args))))
>>                   (_ (error "Value must be a boolean, number, string or list of strings"))))
>>     (format #t "~a=~a~%"
>>             (if locale
>>                 (format #f "~a[~a]" key locale)
>>                 key)
>>             value))
>
> I wonder if it wouldn't be better to stick to the specification. For
> example :comment expects a string, or an alist: shouldn't the function
> return an error if its value is something else?

True, but that's more work ;)

I did not implement the full specs simply because I decided to draw an
arbitrary line between short code, convenience and completeness.  This
is debatable of course and maybe a simple type-checking would not cost much.

Supporting the full specs would require a significant amount of work however.

>>            (set! key
>>                  (string-join (map string-titlecase
>>                                    (string-split (symbol->string
>>                                                   (keyword->symbol key))
>>                                                  #\-))
>>                               ""))
>
> The docstring may explain that, e.g., compound :mime-type key becomes
> MimeType.

Hmm, OK but why?  The procedure produces the expected behaviour with
#:mime-type, is there anything else to clarify?

> In any case, it looks nice and useful.

Thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Nope, the current function only forces "Type" (the only mandatory
> field), everything else is omitted unless the key is explicitly set by
> the user.  This is because I looped over ALL-ARGS (the #:rest argument)
> and I force-added #:type to it.

Oh, true. I stand corrected.

> I did not implement the full specs simply because I decided to draw an
> arbitrary line between short code, convenience and completeness.  This
> is debatable of course and maybe a simple type-checking would not cost
> much.

I was thinking about a match against key before (match value ...), which
would then match value against a chosen predicate, and return an error
if it doesn't match.

- If key is either :no-display, :hidden, :d-bus-activatable, :terminal,
  or :startup-notify, predicate should be `boolean?'.

- If it is :type, :version, :try-exec, :exec, :path, :startup-wm-class,
  or :url, it should be `string?`.

- If it
  is :only-show-in, :not-show-in, :actions, :mime-type, :categories, :implements,
  it should be a string or a list of strings only.

- If it is :name, :generic-name, :comment, :icon, :keywords, it should
  be a a string or an alist.

- If key is anything else, it should be an error, because it is a typo.

I realize that we don't need numbers actually.

> Supporting the full specs would require a significant amount of work
> however.

Possibly, but we do not need more than this simple value type checking.

>> The docstring may explain that, e.g., compound :mime-type key becomes
>> MimeType.
>
> Hmm, OK but why?  The procedure produces the expected behaviour with
> #:mime-type, is there anything else to clarify?

As a packager, I need to know what key is going to produce
StartupWMClass (note that :startup-wm-class produces, StartupWmClass, if
that matters), or DBusActivatable. Unless I'm missing something, it is
not obvious from the docstring.

WDYT?

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> I was thinking about a match against key before (match value ...), which
> would then match value against a chosen predicate, and return an error
> if it doesn't match.
> ...

Could work, I'll see what I can do.  But first I'd like to know how to
actually add this to Guix! :)
Rebuild the world on core-updates or is there another way?

>>> The docstring may explain that, e.g., compound :mime-type key becomes
>>> MimeType.
>>
>> Hmm, OK but why?  The procedure produces the expected behaviour with
>> #:mime-type, is there anything else to clarify?
>
> As a packager, I need to know what key is going to produce
> StartupWMClass (note that :startup-wm-class produces, StartupWmClass, if
> that matters), or DBusActivatable. Unless I'm missing something, it is
> not obvious from the docstring.

I could be misunderstanding you.  The keys are self-documented like all
key arguments.  From any REPL / text editor, you would typically "show
the procedure arguments" to see what's available.  In Emacs/Geiser,
you can use Eldoc or completion to access the full list of key parameters.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Nicolas Goaziou]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Could work, I'll see what I can do.

Great! Thank you.

> But first I'd like to know how to actually add this to Guix! :)
> Rebuild the world on core-updates or is there another way?

I'll let experts answer this :)

> I could be misunderstanding you.  

No, it's a misconception of mine. Sorry for the noise.

Re: Add helper for .desktop file creation?

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Before I could submit a patch, I was wondering where I should place it:
> it seems that placing it in guix/utils.scm triggers a whole world
> rebuild.

I assume you mean (guix build utils), as changing (guix utils) should
not rebuild anything.

> Is there a way around it or should I send this patch to core-updates?

You might be able to alter (guix build-system gnu) without triggering a
world rebuild.  But I don't know how satisfactory e.g. a #:desktop-entry
would be.

There are other ways too (say a package property and profile hook),
depending on how urgently we need this feature.

In any case core-updates is long overdue, so the waiting time should not
be too long.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Pierre Neidhardt <mail@ambrevar.xyz> writes:
>
>> Before I could submit a patch, I was wondering where I should place it:
>> it seems that placing it in guix/utils.scm triggers a whole world
>> rebuild.
>
> I assume you mean (guix build utils), as changing (guix utils) should
> not rebuild anything.

I tried and unless I'm mistaken it did trigger a world rebuild.

>> Is there a way around it or should I send this patch to core-updates?
>
> You might be able to alter (guix build-system gnu) without triggering a
> world rebuild.  But I don't know how satisfactory e.g. a #:desktop-entry
> would be.
>
> There are other ways too (say a package property and profile hook),
> depending on how urgently we need this feature.

I find a separate procedure more meaningful since desktop entries or
independent of any build system (they are particularly useful for the
trivial build system) and I'm not sure how they would fit into a package
property of profile hook.

> In any case core-updates is long overdue, so the waiting time should not
> be too long.

No problem, desktop entries can wait until the next core update.  I'll
send it there then.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Add helper for .desktop file creation?

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

I've sent a patch to #36000.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Pierre Neidhardt <mail@ambrevar.xyz> writes:
>
>> Could work, I'll see what I can do.
>
> Great! Thank you.

Sorry, don't have the time this week.  Feel free to update the patch if
you do.  Worse case, this can always be improved later.

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]