* Re: [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle select-by-right-arrow-key
[not found] <mailman.47.1731690025.13661.grub-devel@gnu.org>
@ 2024-11-18 6:54 ` Avnish Chouhan
2024-11-18 7:32 ` jeffbai
0 siblings, 1 reply; 3+ messages in thread
From: Avnish Chouhan @ 2024-11-18 6:54 UTC (permalink / raw)
To: grub-devel; +Cc: jeffbai
> Message: 1
> Date: Fri, 15 Nov 2024 18:04:03 +0800
> From: Mingcong Bai <jeffbai@aosc.io>
> To: grub-devel@gnu.org
> Cc: sakiiily@aosc.io, cyan@cyano.uk, chenx97@aosc.io, jeffbai@aosc.io,
> fsf@live.com
> Subject: [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle
> select-by-right-arrow-key
> Message-ID: <20241115100403.6305-1-jeffbai@aosc.io>
>
> Normally, GRUB allows using a combination of the Return/Enter (`\n' and
> `\r'), Ctrl-F, and the right arrow key to select a menu item. However,
> on
> some keyboards (especially those half-height arrow keys found on laptop
> computers), it is very easy to accidentally select a menu item when the
> user meant to press the up/down arrow key.
>
> Implement an GRUB_RIGHT_TO_SELECT option (boolean) to enable/disable
> right
> arrow key for selecting menu items.
>
> Co-developed-by: Mag Mell <sakiiily@aosc.io>
> Signed-off-by: Mingcong Bai <jeffbai@aosc.io>
> ---
> docs/grub.texi | 4 ++++
> grub-core/normal/menu.c | 20 +++++++++++++++++---
> util/grub-mkconfig.in | 3 ++-
> util/grub.d/00_header.in | 6 ++++++
> 4 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index acf6f4428..7ae4f4886 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1593,6 +1593,10 @@ This option may be set to a list of GRUB module
> names separated by spaces.
> Each module will be loaded as early as possible, at the start of
> @file{grub.cfg}.
>
> +@item GRUB_RIGHT_TO_SELECT
> +This option may be set to specify whether the right arrow key may be
> used
> +to make a selection. This option defaults to @samp{true}.
> +
> @end table
>
> The following options are still accepted for compatibility with
> existing
> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
> index 6a90e091f..78566b264 100644
> --- a/grub-core/normal/menu.c
> +++ b/grub-core/normal/menu.c
> @@ -32,6 +32,7 @@
> #include <grub/script_sh.h>
> #include <grub/gfxterm.h>
> #include <grub/dl.h>
> +#include <grub/env.h>
>
> /* Time to delay after displaying an error message about a
> default/fallback
> entry failing to boot. */
> @@ -579,11 +580,16 @@ run_menu (grub_menu_t menu, int nested, int
> *auto_boot, int *notify_boot)
> int default_entry, current_entry;
> int timeout;
> enum timeout_style timeout_style;
> + bool right_to_select;
>
> *notify_boot = 1;
>
> default_entry = get_entry_number (menu, "default");
>
> + /* Read if the right arrow key is enabled for selection. Default to
> `true'
> + if unset. */
Hi Mingcong,
Please make this multiline comment as per GNU GRUB coding standards.
Something like
/*
* xxxxxxxx
* xxxxxxxx
*/
> + right_to_select = grub_env_get_bool ("right_to_select", true);
> +
> /* If DEFAULT_ENTRY is not within the menu entries, fall back to
> the first entry. */
> if (default_entry < 0 || default_entry >= menu->size)
> @@ -762,9 +768,17 @@ run_menu (grub_menu_t menu, int nested, int
> *auto_boot, int *notify_boot)
> case '\r':
> case GRUB_TERM_KEY_RIGHT:
> case GRUB_TERM_CTRL | 'f':
> - menu_fini ();
> - *auto_boot = 0;
> - return current_entry;
> + /* Right arrow key to select only when boolean value
> + `right_to_select' is set to `true' or not specified
> + (defaults to `true'). */
Same as commented earlier!
> + if ((c != GRUB_TERM_KEY_RIGHT) ||
> + right_to_select)
> + {
> + menu_fini ();
> + *auto_boot = 0;
> + return current_entry;
> + }
> + break;
This also seems little off on coding style.
Thank you!
Regards,
Avnish Chouhan
>
> case GRUB_TERM_ESC:
> if (nested)
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 73a973b25..b15bfc13a 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -254,7 +254,8 @@ export GRUB_DEFAULT \
> GRUB_ENABLE_CRYPTODISK \
> GRUB_BADRAM \
> GRUB_OS_PROBER_SKIP_LIST \
> - GRUB_DISABLE_SUBMENU
> + GRUB_DISABLE_SUBMENU \
> + GRUB_RIGHT_TO_SELECT
>
> if test "x${grub_cfg}" != "x"; then
> rm -f "${grub_cfg}.new"
> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
> index 5e17a9db4..0adf84019 100644
> --- a/util/grub.d/00_header.in
> +++ b/util/grub.d/00_header.in
> @@ -37,6 +37,7 @@ if [ "x${GRUB_DEFAULT}" = "x" ] ; then GRUB_DEFAULT=0
> ; fi
> if [ "x${GRUB_DEFAULT}" = "xsaved" ] ; then
> GRUB_DEFAULT='${saved_entry}' ; fi
> if [ "x${GRUB_TIMEOUT}" = "x" ] ; then GRUB_TIMEOUT=5 ; fi
> if [ "x${GRUB_GFXMODE}" = "x" ] ; then GRUB_GFXMODE=auto ; fi
> +if [ "x${GRUB_RIGHT_TO_SELECT}" = "x" ] ; then
> GRUB_RIGHT_TO_SELECT=true ; fi
>
> if [ "x${GRUB_DEFAULT_BUTTON}" = "x" ] ; then
> GRUB_DEFAULT_BUTTON="$GRUB_DEFAULT" ; fi
> if [ "x${GRUB_DEFAULT_BUTTON}" = "xsaved" ] ; then
> GRUB_DEFAULT_BUTTON='${saved_entry}' ; fi
> @@ -373,3 +374,8 @@ fi
> if [ "x${GRUB_BADRAM}" != "x" ] ; then
> echo "badram ${GRUB_BADRAM}"
> fi
> +
> +# Whether to allow selecting with the right arrow key.
> +cat << EOF
> +set right_to_select=${GRUB_RIGHT_TO_SELECT}
> +EOF
> --
> 2.47.0
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle select-by-right-arrow-key
2024-11-18 6:54 ` [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle select-by-right-arrow-key Avnish Chouhan
@ 2024-11-18 7:32 ` jeffbai
0 siblings, 0 replies; 3+ messages in thread
From: jeffbai @ 2024-11-18 7:32 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Avnish Chouhan
Hi Avnish,
在 2024-11-18 14:54，Avnish Chouhan 写道：
>> Message: 1
>> Date: Fri, 15 Nov 2024 18:04:03 +0800
>> From: Mingcong Bai <jeffbai@aosc.io>
>> To: grub-devel@gnu.org
>> Cc: sakiiily@aosc.io, cyan@cyano.uk, chenx97@aosc.io, jeffbai@aosc.io,
>> fsf@live.com
>> Subject: [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle
>> select-by-right-arrow-key
>> Message-ID: <20241115100403.6305-1-jeffbai@aosc.io>
>>
>> Normally, GRUB allows using a combination of the Return/Enter (`\n'
>> and
>> `\r'), Ctrl-F, and the right arrow key to select a menu item. However,
>> on
>> some keyboards (especially those half-height arrow keys found on
>> laptop
>> computers), it is very easy to accidentally select a menu item when
>> the
>> user meant to press the up/down arrow key.
>>
>> Implement an GRUB_RIGHT_TO_SELECT option (boolean) to enable/disable
>> right
>> arrow key for selecting menu items.
>>
>> Co-developed-by: Mag Mell <sakiiily@aosc.io>
>> Signed-off-by: Mingcong Bai <jeffbai@aosc.io>
>> ---
>> docs/grub.texi | 4 ++++
>> grub-core/normal/menu.c | 20 +++++++++++++++++---
>> util/grub-mkconfig.in | 3 ++-
>> util/grub.d/00_header.in | 6 ++++++
>> 4 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index acf6f4428..7ae4f4886 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1593,6 +1593,10 @@ This option may be set to a list of GRUB module
>> names separated by spaces.
>> Each module will be loaded as early as possible, at the start of
>> @file{grub.cfg}.
>>
>> +@item GRUB_RIGHT_TO_SELECT
>> +This option may be set to specify whether the right arrow key may be
>> used
>> +to make a selection. This option defaults to @samp{true}.
>> +
>> @end table
>>
>> The following options are still accepted for compatibility with
>> existing
>> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
>> index 6a90e091f..78566b264 100644
>> --- a/grub-core/normal/menu.c
>> +++ b/grub-core/normal/menu.c
>> @@ -32,6 +32,7 @@
>> #include <grub/script_sh.h>
>> #include <grub/gfxterm.h>
>> #include <grub/dl.h>
>> +#include <grub/env.h>
>>
>> /* Time to delay after displaying an error message about a
>> default/fallback
>> entry failing to boot. */
>> @@ -579,11 +580,16 @@ run_menu (grub_menu_t menu, int nested, int
>> *auto_boot, int *notify_boot)
>> int default_entry, current_entry;
>> int timeout;
>> enum timeout_style timeout_style;
>> + bool right_to_select;
>>
>> *notify_boot = 1;
>>
>> default_entry = get_entry_number (menu, "default");
>>
>> + /* Read if the right arrow key is enabled for selection. Default to
>> `true'
>> + if unset. */
>
> Hi Mingcong,
>
> Please make this multiline comment as per GNU GRUB coding standards.
> Something like
>
> /*
> * xxxxxxxx
> * xxxxxxxx
> */
>
>
Noted.
>> + right_to_select = grub_env_get_bool ("right_to_select", true);
>> +
>> /* If DEFAULT_ENTRY is not within the menu entries, fall back to
>> the first entry. */
>> if (default_entry < 0 || default_entry >= menu->size)
>> @@ -762,9 +768,17 @@ run_menu (grub_menu_t menu, int nested, int
>> *auto_boot, int *notify_boot)
>> case '\r':
>> case GRUB_TERM_KEY_RIGHT:
>> case GRUB_TERM_CTRL | 'f':
>> - menu_fini ();
>> - *auto_boot = 0;
>> - return current_entry;
>> + /* Right arrow key to select only when boolean value
>> + `right_to_select' is set to `true' or not specified
>> + (defaults to `true'). */
>
> Same as commented earlier!
>
>> + if ((c != GRUB_TERM_KEY_RIGHT) ||
>> + right_to_select)
>> + {
>> + menu_fini ();
>> + *auto_boot = 0;
>> + return current_entry;
>> + }
>> + break;
>
> This also seems little off on coding style.
> Thank you!
Got it. I will send a v2 patch in a bit, thanks for your review!
Best Regards,
Mingcong Bai
>
> Regards,
> Avnish Chouhan
>
>
>>
>> case GRUB_TERM_ESC:
>> if (nested)
>> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
>> index 73a973b25..b15bfc13a 100644
>> --- a/util/grub-mkconfig.in
>> +++ b/util/grub-mkconfig.in
>> @@ -254,7 +254,8 @@ export GRUB_DEFAULT \
>> GRUB_ENABLE_CRYPTODISK \
>> GRUB_BADRAM \
>> GRUB_OS_PROBER_SKIP_LIST \
>> - GRUB_DISABLE_SUBMENU
>> + GRUB_DISABLE_SUBMENU \
>> + GRUB_RIGHT_TO_SELECT
>>
>> if test "x${grub_cfg}" != "x"; then
>> rm -f "${grub_cfg}.new"
>> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
>> index 5e17a9db4..0adf84019 100644
>> --- a/util/grub.d/00_header.in
>> +++ b/util/grub.d/00_header.in
>> @@ -37,6 +37,7 @@ if [ "x${GRUB_DEFAULT}" = "x" ] ; then
>> GRUB_DEFAULT=0 ; fi
>> if [ "x${GRUB_DEFAULT}" = "xsaved" ] ; then
>> GRUB_DEFAULT='${saved_entry}' ; fi
>> if [ "x${GRUB_TIMEOUT}" = "x" ] ; then GRUB_TIMEOUT=5 ; fi
>> if [ "x${GRUB_GFXMODE}" = "x" ] ; then GRUB_GFXMODE=auto ; fi
>> +if [ "x${GRUB_RIGHT_TO_SELECT}" = "x" ] ; then
>> GRUB_RIGHT_TO_SELECT=true ; fi
>>
>> if [ "x${GRUB_DEFAULT_BUTTON}" = "x" ] ; then
>> GRUB_DEFAULT_BUTTON="$GRUB_DEFAULT" ; fi
>> if [ "x${GRUB_DEFAULT_BUTTON}" = "xsaved" ] ; then
>> GRUB_DEFAULT_BUTTON='${saved_entry}' ; fi
>> @@ -373,3 +374,8 @@ fi
>> if [ "x${GRUB_BADRAM}" != "x" ] ; then
>> echo "badram ${GRUB_BADRAM}"
>> fi
>> +
>> +# Whether to allow selecting with the right arrow key.
>> +cat << EOF
>> +set right_to_select=${GRUB_RIGHT_TO_SELECT}
>> +EOF
>> --
>> 2.47.0
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle select-by-right-arrow-key
@ 2024-11-15 10:04 Mingcong Bai
0 siblings, 0 replies; 3+ messages in thread
From: Mingcong Bai @ 2024-11-15 10:04 UTC (permalink / raw)
To: grub-devel; +Cc: sakiiily, cyan, chenx97, jeffbai, fsf
Normally, GRUB allows using a combination of the Return/Enter (`\n' and
`\r'), Ctrl-F, and the right arrow key to select a menu item. However, on
some keyboards (especially those half-height arrow keys found on laptop
computers), it is very easy to accidentally select a menu item when the
user meant to press the up/down arrow key.
Implement an GRUB_RIGHT_TO_SELECT option (boolean) to enable/disable right
arrow key for selecting menu items.
Co-developed-by: Mag Mell <sakiiily@aosc.io>
Signed-off-by: Mingcong Bai <jeffbai@aosc.io>
---
docs/grub.texi | 4 ++++
| 20 +++++++++++++++++---
util/grub-mkconfig.in | 3 ++-
| 6 ++++++
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index acf6f4428..7ae4f4886 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1593,6 +1593,10 @@ This option may be set to a list of GRUB module names separated by spaces.
Each module will be loaded as early as possible, at the start of
@file{grub.cfg}.
+@item GRUB_RIGHT_TO_SELECT
+This option may be set to specify whether the right arrow key may be used
+to make a selection. This option defaults to @samp{true}.
+
@end table
The following options are still accepted for compatibility with existing
--git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
index 6a90e091f..78566b264 100644
--- a/grub-core/normal/menu.c
+++ b/grub-core/normal/menu.c
@@ -32,6 +32,7 @@
#include <grub/script_sh.h>
#include <grub/gfxterm.h>
#include <grub/dl.h>
+#include <grub/env.h>
/* Time to delay after displaying an error message about a default/fallback
entry failing to boot. */
@@ -579,11 +580,16 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot, int *notify_boot)
int default_entry, current_entry;
int timeout;
enum timeout_style timeout_style;
+ bool right_to_select;
*notify_boot = 1;
default_entry = get_entry_number (menu, "default");
+ /* Read if the right arrow key is enabled for selection. Default to `true'
+ if unset. */
+ right_to_select = grub_env_get_bool ("right_to_select", true);
+
/* If DEFAULT_ENTRY is not within the menu entries, fall back to
the first entry. */
if (default_entry < 0 || default_entry >= menu->size)
@@ -762,9 +768,17 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot, int *notify_boot)
case '\r':
case GRUB_TERM_KEY_RIGHT:
case GRUB_TERM_CTRL | 'f':
- menu_fini ();
- *auto_boot = 0;
- return current_entry;
+ /* Right arrow key to select only when boolean value
+ `right_to_select' is set to `true' or not specified
+ (defaults to `true'). */
+ if ((c != GRUB_TERM_KEY_RIGHT) ||
+ right_to_select)
+ {
+ menu_fini ();
+ *auto_boot = 0;
+ return current_entry;
+ }
+ break;
case GRUB_TERM_ESC:
if (nested)
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 73a973b25..b15bfc13a 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -254,7 +254,8 @@ export GRUB_DEFAULT \
GRUB_ENABLE_CRYPTODISK \
GRUB_BADRAM \
GRUB_OS_PROBER_SKIP_LIST \
- GRUB_DISABLE_SUBMENU
+ GRUB_DISABLE_SUBMENU \
+ GRUB_RIGHT_TO_SELECT
if test "x${grub_cfg}" != "x"; then
rm -f "${grub_cfg}.new"
--git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
index 5e17a9db4..0adf84019 100644
--- a/util/grub.d/00_header.in
+++ b/util/grub.d/00_header.in
@@ -37,6 +37,7 @@ if [ "x${GRUB_DEFAULT}" = "x" ] ; then GRUB_DEFAULT=0 ; fi
if [ "x${GRUB_DEFAULT}" = "xsaved" ] ; then GRUB_DEFAULT='${saved_entry}' ; fi
if [ "x${GRUB_TIMEOUT}" = "x" ] ; then GRUB_TIMEOUT=5 ; fi
if [ "x${GRUB_GFXMODE}" = "x" ] ; then GRUB_GFXMODE=auto ; fi
+if [ "x${GRUB_RIGHT_TO_SELECT}" = "x" ] ; then GRUB_RIGHT_TO_SELECT=true ; fi
if [ "x${GRUB_DEFAULT_BUTTON}" = "x" ] ; then GRUB_DEFAULT_BUTTON="$GRUB_DEFAULT" ; fi
if [ "x${GRUB_DEFAULT_BUTTON}" = "xsaved" ] ; then GRUB_DEFAULT_BUTTON='${saved_entry}' ; fi
@@ -373,3 +374,8 @@ fi
if [ "x${GRUB_BADRAM}" != "x" ] ; then
echo "badram ${GRUB_BADRAM}"
fi
+
+# Whether to allow selecting with the right arrow key.
+cat << EOF
+set right_to_select=${GRUB_RIGHT_TO_SELECT}
+EOF
--
2.47.0
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-18 7:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <mailman.47.1731690025.13661.grub-devel@gnu.org>
2024-11-18 6:54 ` [PATCH] menu: add GRUB_RIGHT_TO_SELECT to toggle select-by-right-arrow-key Avnish Chouhan
2024-11-18 7:32 ` jeffbai
2024-11-15 10:04 Mingcong Bai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.