[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