guix-patches
[Top][All Lists]
Advanced

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

[bug#41163] [PATCH] gnu: grub: Allow a PNG image and replace (aspect-rat


From: Mathieu Othacehe
Subject: [bug#41163] [PATCH] gnu: grub: Allow a PNG image and replace (aspect-ratio) with (resolution).
Date: Sun, 10 May 2020 09:07:54 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Hello Stefan,

For some reason I cannot apply this patch. Could you send an updated
revision based on master?

A few comments below:

> * gnu/bootloaders/grub.scm (<grub-image>)[resolution]: Replacement of the
> 'aspect-ratio' field.

You can write something like:

* gnu/bootloaders/grub.scm (<grub-image>)[aspect-ratio]: Remove this
field and replace it by ...
[resolution]: ... this field.

> +  (resolution grub-image-resolution
> +              (default '(1024 . 768)))
> +  (file       grub-image-file
> +              (default (file-append %artwork-repository
> +                                    "/grub/GuixSD-fully-black-4-3.svg"))))

I'm not sure about defaulting to this file. This record is meant to
describe a generic image. Could you keep this empty?

>  (define* (svg->png svg #:key width height)
> -  "Build a PNG of HEIGHT x WIDTH from SVG."
> +  "Build a PNG of HEIGHT x WIDTH from SVG if its file suffix is \".svg\".

I'm not sure having "svg->png" handle other file types than ".svg" is
very clear. 

> +                     #~(if (string-suffix? ".svg" #+svg)
> +                           (begin
> +                             (use-modules (gnu build svg))
> +                             (svg->png #+svg #$output
> +                                       #:width #$width
> +                                       #:height #$height))
> +                           (copy-file #+svg #$output))))))

You could move this check to "grub-background-image" procedure. So that
"svg->png" only deals with ".svg" files.

> +         (let ((resolution (grub-image-resolution image)))
> +           (svg->png (grub-image-file image)
> +                     #:width (car resolution)
> +                     #:height (cdr resolution))))))

"car" and "cdr" should be avoided. You can write something like that
instead:

--8<---------------cut here---------------start------------->8---
(match resolution
       ((width . height)
        (svg->png (grub-image-file image)
                  #:width width
                  #:height height)))
--8<---------------cut here---------------end--------------->8---

Thanks for this patch,

Mathieu





reply via email to

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