Subject: Re: [bug#34929] [PATCH 04/12] bootloader: Add a 'keyboard-layout' field.

From: Julien Lepiller <[email-masked]>
Date: Sun, 24 Mar 2019 10:31:57 +0100

Le Thu, 21 Mar 2019 00:04:27 +0100,
Ludovic Courtès <[email-masked]> a écrit :

> * gnu/bootloader/grub.scm (keyboard-layout-file): New procedure.
> (grub-configuration-file)[keyboard-layout-file]: New variable.
> [builder]: Use it.
> * gnu/bootloader.scm (<bootloader-configuration>)[keyboard-layout]:
> New field.
> * doc/guix.texi (Bootloader Configuration): Document it.
> 
> Co-authored-by: nee <[email-masked]>
> ---
>  doc/guix.texi           | 28 ++++++++++++++++++++++++++++
>  gnu/bootloader.scm      |  3 +++
>  gnu/bootloader/grub.scm | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 63581bbe41..c687f78eac 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -23134,6 +23134,34 @@ current system.
>  The number of seconds to wait for keyboard input before booting.
> Set to 0 to boot immediately, and to -1 to wait indefinitely.
>  
> +@cindex keyboard layout, for the bootloader
> +@item @code{keyboard-layout} (default: @code{#f})
> +If this is @code{#f}, the bootloader's menu (if any) uses the
> default keyboard +layout, usually US@tie{}English (``qwerty'').
> +
> +Otherwise, this must be a @code{keyboard-layout} object.  For
> instance, the +following example defines a standard German keyboard
> layout: +
> +@example
> +(keyboard-layout "de")
> +@end example
> +
> +@noindent
> +while the example below designates the bépo layout for French:
> +
> +@example
> +(keyboard-layout "fr" "bepo")
> +@end example

I think these examples are a bit confusing, because they could be
interpreted as:

(bootloader (bootloader-configuration
                (target "/boot/efi")
                (bootloader grub-efi-bootloader)
                (keyboard-layout "fr" "bepo")))

as well as:

(bootloader (bootloader-configuration
                (target "/boot/efi")
                (bootloader grub-efi-bootloader)
                (keyboard-layout (keyboard-layout "fr" "bepo"))))

and I think you mean the second one, but the first one is the one that
comes immediately to mind when reading the manual. Maybe you could add
the full example?

> +
> +The layout name and variant must match an existing layout in the
> +@code{xkeyboard-config} package under the
> @file{share/X11/xkb/symbols} +directory.
> +
> +@quotation Note
> +This option is currently ignored by bootloaders other than
> @code{grub} and +@code{grub-efi}.
> +@end quotation
> +
>  @item @code{theme} (default: @var{#f})
>  The bootloader theme object describing the theme to use.  If no theme
>  is provided, some bootloaders might use a default theme, that's true
> diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm
> index e48bcc073c..e7f7331dd8 100644
> --- a/gnu/bootloader.scm
> +++ b/gnu/bootloader.scm
> @@ -47,6 +47,7 @@
>              bootloader-configuration-menu-entries
>              bootloader-configuration-default-entry
>              bootloader-configuration-timeout
> +            bootloader-configuration-keyboard-layout
>              bootloader-configuration-theme
>              bootloader-configuration-terminal-outputs
>              bootloader-configuration-terminal-inputs
> @@ -113,6 +114,8 @@
>                        (default 0))
>    (timeout            bootloader-configuration-timeout ;seconds as
> integer (default 5))
> +  (keyboard-layout
> bootloader-configuration-keyboard-layout ;string | #f

I think you mean "<keyboard-layout> | #f"

> +                      (default #f))

The rest of the patch lgtm :)