* Device instability, gettext and default
@ 2012-03-03 13:37 Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 14:27 ` Jordi Mallach
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-03 13:37 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1: Type: text/plain, Size: 475 bytes --]
Hello, all. Currently we use and recommend using the title for default
variable. It has however following problems:
1) When device names change the title changes (because of the "(on
$device)" part)
2) If user changes locale the part ", with" gets translated and again
the title changes
Attached patch changes it to the use of IDs specified by --id but keeps
title possiblity for backward compatibility. Any comments?
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: menuid.diff --]
[-- Type: text/x-diff, Size: 15916 bytes --]
=== modified file 'grub-core/commands/legacycfg.c'
--- grub-core/commands/legacycfg.c 2012-02-26 16:28:05 +0000
+++ grub-core/commands/legacycfg.c 2012-03-03 12:11:30 +0000
@@ -123,7 +123,7 @@
return grub_errno;
}
args[0] = oldname;
- grub_normal_add_menu_entry (1, args, NULL, NULL, NULL, NULL,
+ grub_normal_add_menu_entry (1, args, NULL, NULL, NULL, NULL, NULL,
entrysrc, 0);
grub_free (args);
entrysrc[0] = 0;
@@ -174,7 +174,8 @@
return grub_errno;
}
args[0] = entryname;
- grub_normal_add_menu_entry (1, args, NULL, NULL, NULL, NULL, entrysrc, 0);
+ grub_normal_add_menu_entry (1, args, NULL, NULL, NULL,
+ NULL, NULL, entrysrc, 0);
grub_free (args);
}
=== modified file 'grub-core/commands/menuentry.c'
--- grub-core/commands/menuentry.c 2012-02-26 16:28:05 +0000
+++ grub-core/commands/menuentry.c 2012-03-03 12:11:30 +0000
@@ -36,6 +36,8 @@
N_("Keyboard key to quickly boot this entry."), N_("KEYBOARD_KEY"), ARG_TYPE_STRING},
{"source", 4, 0,
N_("Use STRING as menu entry body."), N_("STRING"), ARG_TYPE_STRING},
+ {"id", 1, GRUB_ARG_OPTION_REPEATABLE,
+ N_("Menu entry identifier."), N_("STRING"), ARG_TYPE_STRING},
{0, 0, 0, 0, 0, 0}
};
@@ -67,7 +69,8 @@
variable data slot `menu'). As the configuration file is read, the script
parser calls this when a menu entry is to be created. */
grub_err_t
-grub_normal_add_menu_entry (int argc, const char **args, char **classes,
+grub_normal_add_menu_entry (int argc, const char **args,
+ char **classes, const char *id,
const char *users, const char *hotkey,
const char *prefix, const char *sourcecode,
int submenu)
@@ -77,6 +80,7 @@
char *menu_users = NULL;
char *menu_title = NULL;
char *menu_sourcecode = NULL;
+ char *menu_id = NULL;
struct grub_menu_entry_class *menu_classes = NULL;
grub_menu_t menu;
@@ -139,6 +143,10 @@
if (! menu_title)
goto fail;
+ menu_id = grub_strdup (id ? : menu_title);
+ if (! menu_id)
+ goto fail;
+
/* Save argc, args to pass as parameters to block arg later. */
menu_args = grub_malloc (sizeof (char*) * (argc + 1));
if (! menu_args)
@@ -164,6 +172,7 @@
goto fail;
(*last)->title = menu_title;
+ (*last)->id = menu_id;
(*last)->hotkey = menu_hotkey;
(*last)->classes = menu_classes;
if (menu_users)
@@ -196,6 +205,7 @@
grub_free (menu_users);
grub_free (menu_title);
+ grub_free (menu_id);
return grub_errno;
}
@@ -257,7 +267,9 @@
if (! ctxt->script)
return grub_normal_add_menu_entry (argc, (const char **) args,
(ctxt->state[0].set ? ctxt->state[0].args
- : NULL), ctxt->state[1].arg,
+ : NULL),
+ ctxt->state[4].arg,
+ ctxt->state[1].arg,
ctxt->state[2].arg, 0,
ctxt->state[3].arg,
ctxt->extcmd->cmd->name[0] == 's');
@@ -274,7 +286,8 @@
return grub_errno;
r = grub_normal_add_menu_entry (argc - 1, (const char **) args,
- ctxt->state[0].args, ctxt->state[1].arg,
+ ctxt->state[0].args, ctxt->state[4].arg,
+ ctxt->state[1].arg,
ctxt->state[2].arg, prefix, src + 1,
ctxt->extcmd->cmd->name[0] == 's');
@@ -291,10 +304,12 @@
{
cmd = grub_register_extcmd ("menuentry", grub_cmd_menuentry,
GRUB_COMMAND_FLAG_BLOCKS
+ | GRUB_COMMAND_ACCEPT_DASH
| GRUB_COMMAND_FLAG_EXTRACTOR,
N_("BLOCK"), N_("Define a menu entry."), options);
cmd_sub = grub_register_extcmd ("submenu", grub_cmd_menuentry,
GRUB_COMMAND_FLAG_BLOCKS
+ | GRUB_COMMAND_ACCEPT_DASH
| GRUB_COMMAND_FLAG_EXTRACTOR,
N_("BLOCK"), N_("Define a submenu."),
options);
=== modified file 'grub-core/normal/main.c'
--- grub-core/normal/main.c 2012-02-26 17:09:07 +0000
+++ grub-core/normal/main.c 2012-03-03 12:11:30 +0000
@@ -475,7 +475,8 @@
static void (*grub_xputs_saved) (const char *str);
static const char *features[] = {
"feature_chainloader_bpb", "feature_ntldr", "feature_platform_search_hint",
- "feature_default_font_path", "feature_all_video_module"
+ "feature_default_font_path", "feature_all_video_module",
+ "feature_menuentry_id", "feature_menuentry_options"
};
GRUB_MOD_INIT(normal)
=== modified file 'grub-core/normal/menu.c'
--- grub-core/normal/menu.c 2012-03-03 12:05:08 +0000
+++ grub-core/normal/menu.c 2012-03-03 12:11:30 +0000
@@ -188,7 +188,7 @@
grub_env_set ("timeout", "0");
}
- for (ptr = entry->title; *ptr; ptr++)
+ for (ptr = entry->id; *ptr; ptr++)
sz += (*ptr == '>') ? 2 : 1;
if (chosen)
{
@@ -217,7 +217,7 @@
optr = grub_stpcpy (optr, chosen);
*optr++ = '>';
}
- for (ptr = entry->title; *ptr; ptr++)
+ for (ptr = entry->id; *ptr; ptr++)
{
if (*ptr == '>')
*optr++ = '>';
@@ -411,10 +411,10 @@
}
static int
-menuentry_eq (const char *title, const char *spec)
+menuentry_eq (const char *id, const char *spec)
{
const char *ptr1, *ptr2;
- ptr1 = title;
+ ptr1 = id;
ptr2 = spec;
while (1)
{
@@ -459,7 +459,8 @@
for (i = 0; e; i++)
{
- if (menuentry_eq (e->title, val))
+ if (menuentry_eq (e->title, val)
+ || menuentry_eq (e->id, val))
{
entry = i;
break;
=== modified file 'include/grub/menu.h'
--- include/grub/menu.h 2011-01-10 22:27:58 +0000
+++ include/grub/menu.h 2012-03-03 12:11:30 +0000
@@ -32,6 +32,9 @@
/* The title name. */
const char *title;
+ /* The identifier. */
+ const char *id;
+
/* If set means not everybody is allowed to boot this entry. */
int restricted;
=== modified file 'include/grub/normal.h'
--- include/grub/normal.h 2012-02-09 13:38:34 +0000
+++ include/grub/normal.h 2012-03-03 12:11:30 +0000
@@ -120,6 +120,7 @@
grub_err_t
grub_normal_add_menu_entry (int argc, const char **args, char **classes,
+ const char *id,
const char *users, const char *hotkey,
const char *prefix, const char *sourcecode,
int submenu);
=== modified file 'util/grub-mkconfig_lib.in'
--- util/grub-mkconfig_lib.in 2012-02-27 10:04:50 +0000
+++ util/grub-mkconfig_lib.in 2012-03-03 12:11:30 +0000
@@ -162,6 +162,16 @@
fi
}
+grub_get_device_id ()
+{
+ device="$1"
+ if fs_uuid="`"${grub_probe}" --device "${device}" --target=fs_uuid 2> /dev/null`" ; then
+ echo "$fs_uuid";
+ else
+ echo "$device"
+ fi
+}
+
grub_file_is_not_garbage ()
{
if test -f "$1" ; then
=== modified file 'util/grub.d/00_header.in'
--- util/grub.d/00_header.in 2012-02-29 23:40:02 +0000
+++ util/grub.d/00_header.in 2012-03-03 12:11:30 +0000
@@ -63,6 +63,13 @@
EOF
fi
cat <<EOF
+
+if [ x"\${feature_menuentry_id}" = xy ]; then
+ menuentry_id_option="--id"
+else
+ menuentry_id_option=""
+fi
+
if [ "\${prev_saved_entry}" ]; then
set saved_entry="\${prev_saved_entry}"
save_env saved_entry
=== modified file 'util/grub.d/10_hurd.in'
--- util/grub.d/10_hurd.in 2012-02-29 23:40:02 +0000
+++ util/grub.d/10_hurd.in 2012-03-03 12:11:30 +0000
@@ -85,9 +85,9 @@
KERNEL="using ${kernel_base}"
cat << EOF
-menuentry "${OS} ${KERNEL}" ${CLASS} {
+menuentry "${OS} ${KERNEL}" ${CLASS} \$menuentry_id_option 'gnuhurd-$kernel-false-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
EOF
- prepare_grub_to_access_device ${GRUB_DEVICE_BOOT} | sed -e "s/^/\t/"
+ prepare_grub_to_access_device "${GRUB_DEVICE_BOOT}" | sed -e "s/^/\t/"
message="$(gettext_printf "Loading GNU Mach ...")"
cat << EOF
echo '$message'
=== modified file 'util/grub.d/10_illumos.in'
--- util/grub.d/10_illumos.in 2012-01-24 12:17:36 +0000
+++ util/grub.d/10_illumos.in 2012-03-03 12:11:30 +0000
@@ -34,9 +34,9 @@
;;
esac
-echo "menuentry '${OS}' ${CLASS} {"
+echo "menuentry '${OS}' ${CLASS} \$menuentry_id_option 'illumos-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {"
save_default_entry | sed -e "s/^/\t/"
-prepare_grub_to_access_device ${GRUB_DEVICE_BOOT} | sed -e "s/^/\t/"
+prepare_grub_to_access_device "${GRUB_DEVICE_BOOT}" | sed -e "s/^/\t/"
message="$(gettext_printf "Loading kernel of Illumos ...")"
cat << EOF
insmod gzio
=== modified file 'util/grub.d/10_kfreebsd.in'
--- util/grub.d/10_kfreebsd.in 2012-02-03 10:42:22 +0000
+++ util/grub.d/10_kfreebsd.in 2012-03-03 12:11:30 +0000
@@ -74,7 +74,10 @@
else
title="$(gettext_quoted "%s, with kFreeBSD %s")"
fi
- printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")"
+ fi
+ printf "menuentry '${title}' ${CLASS} \$menuentry_id_option 'kfreebsd-$version-$recovery-$boot_device_id' {\n" "${os}" "${version}"
if ! ${recovery} ; then
save_default_entry | sed -e "s/^/\t/"
fi
@@ -132,6 +135,7 @@
if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
done`
prepare_boot_cache=
+boot_device_id=
while [ "x$list" != "x" ] ; do
kfreebsd=`version_find_latest $list`
=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in 2012-02-27 18:07:09 +0000
+++ util/grub.d/10_linux.in 2012-03-03 12:11:30 +0000
@@ -77,7 +77,10 @@
else
title="$(gettext_quoted "%s, with Linux %s")"
fi
- printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")"
+ fi
+ printf "menuentry '${title}' ${CLASS} \$menuentry_id_option 'gnulinux-$version-$recovery-$boot_device_id' {\n" "${os}" "${version}"
if ! ${recovery} ; then
save_default_entry | sed -e "s/^/\t/"
fi
@@ -151,6 +154,7 @@
prepare_boot_cache=
prepare_root_cache=
+boot_device_id=
while [ "x$list" != "x" ] ; do
linux=`version_find_latest $list`
=== modified file 'util/grub.d/10_netbsd.in'
--- util/grub.d/10_netbsd.in 2012-02-03 10:42:22 +0000
+++ util/grub.d/10_netbsd.in 2012-03-03 12:11:30 +0000
@@ -99,7 +99,10 @@
title="$(gettext_quoted "%s, with kernel %s (via %s)")"
fi
- printf "menuentry \"${title}\" {\n" \
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")"
+ fi
+ printf "menuentry \"${title}\" \$menuentry_id_option 'netbsd-$kernel-$recovery-$boot_device_id' {\n" \
"${OS}" "$(echo ${kernel} | sed -e 's,^.*/,,')" "${loader}"
printf "%s\n" "${prepare_boot_cache}"
case "${loader}" in
@@ -119,6 +122,7 @@
}
prepare_boot_cache="$(prepare_grub_to_access_device ${GRUB_DEVICE} | sed -e 's,^, ,')"
+boot_device_id=
# We look for NetBSD kernels in / but not in subdirectories. We simply
# pick all statically linked ELF executable files (or links) in / with a
=== modified file 'util/grub.d/10_windows.in'
--- util/grub.d/10_windows.in 2012-02-29 23:40:02 +0000
+++ util/grub.d/10_windows.in 2012-03-03 12:11:30 +0000
@@ -63,14 +63,16 @@
test -n "$dir" || continue
needmap=
+ osid=
# Check for Vista bootmgr.
if [ -f "$dir"/bootmgr -a -f "$dir"/boot/bcd ] ; then
OS="$(gettext_quoted "Windows Vista/7 (loader)")"
-
+ osid=bootmgr
# Check for NTLDR.
elif [ -f "$dir"/ntldr -a -f "$dir"/ntdetect.com -a -f "$dir"/boot.ini ] ; then
OS=`get_os_name_from_boot_ini "$dir"/boot.ini` || OS="$(gettext_quoted "Windows NT/2000/XP (loader)")"
+ osid=ntldr
needmap=t
else
@@ -82,7 +84,7 @@
gettext_printf "Found %s on %s (%s)\n" "$OS" "$drv" "$dev" >&2
cat << EOF
-menuentry "$OS" {
+menuentry "$OS" \$menuentry_id_option '$osid-$(grub_get_device_id "${dev}")' {
EOF
save_default_entry | sed -e 's,^,\t,'
=== modified file 'util/grub.d/20_linux_xen.in'
--- util/grub.d/20_linux_xen.in 2012-02-27 18:07:09 +0000
+++ util/grub.d/20_linux_xen.in 2012-03-03 12:11:30 +0000
@@ -87,7 +87,10 @@
else
title="$(gettext_quoted "%s, with Xen %s and Linux %s")"
fi
- printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${xen_version}" "${version}"
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")"
+ fi
+ printf "menuentry '${title}' ${CLASS} \$menuentry_id_option 'xen-gnulinux-$version-$recovery-$boot_device_id' {\n" "${os}" "${xen_version}" "${version}"
if ! ${recovery} ; then
save_default_entry | sed -e "s/^/\t/"
fi
@@ -139,6 +142,7 @@
if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
done`
prepare_boot_cache=
+boot_device_id=
while [ "x${xen_list}" != "x" ] ; do
list="${linux_list}"
@@ -147,7 +151,10 @@
xen_dirname=`dirname ${current_xen}`
rel_xen_dirname=`make_system_path_relative_to_its_root $xen_dirname`
xen_version=`echo $xen_basename | sed -e "s,.gz$,,g;s,^xen-,,g"`
- echo "submenu \"Xen ${xen_version}\" {"
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")"
+ fi
+ echo "submenu \"Xen ${xen_version}\" \$menuentry_id_option 'xen-hypervisor-$xen_version-$boot_device_id' {"
while [ "x$list" != "x" ] ; do
linux=`version_find_latest $list`
gettext_printf "Found linux image: %s\n" "$linux" >&2
=== modified file 'util/grub.d/30_os-prober.in'
--- util/grub.d/30_os-prober.in 2012-03-02 14:09:10 +0000
+++ util/grub.d/30_os-prober.in 2012-03-03 12:11:30 +0000
@@ -52,7 +52,7 @@
# TRANSLATORS: it refers on the OS residing on device %s
onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
cat << EOF
-menuentry "${LONGNAME} $bitstr $onstr" --class osx --class darwin --class os {
+menuentry "${LONGNAME} $bitstr $onstr" --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${DEVICE}")' {
EOF
save_default_entry | sed -e "s/^/\t/"
prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
@@ -104,6 +104,8 @@
EOF
}
+used_osprober_linux_ids=
+
for OS in ${OSPROBED} ; do
DEVICE="`echo ${OS} | cut -d ':' -f 1`"
LONGNAME="`echo ${OS} | cut -d ':' -f 2 | tr '^' ' '`"
@@ -121,7 +123,7 @@
onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
cat << EOF
-menuentry "${LONGNAME} $onstr" --class windows --class os {
+menuentry "${LONGNAME} $onstr" --class windows --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
EOF
save_default_entry | sed -e "s/^/\t/"
prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
@@ -144,6 +146,7 @@
linux)
LINUXPROBED="`linux-boot-prober ${DEVICE} 2> /dev/null | tr ' ' '^' | paste -s -d ' '`"
prepare_boot_cache=
+ boot_device_id=
for LINUX in ${LINUXPROBED} ; do
LROOT="`echo ${LINUX} | cut -d ':' -f 1`"
@@ -163,8 +166,17 @@
fi
onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
+ recovery_params="$(echo "${LPARAMS}" | grep single)"
+ counter=1
+ while echo "$used_osprober_linux_ids" | grep 'osprober-gnulinux-$LKERNEL-${recovery_params}-$counter-$boot_device_id' > /dev/null; do
+ counter=$((counter+1));
+ done
+ if [ -z "$boot_device_id" ]; then
+ boot_device_id="$(grub_get_device_id "${DEVICE}")"
+ fi
+ used_osprober_linux_ids="$used_osprober_linux_ids 'osprober-gnulinux-$LKERNEL-${recovery_params}-$counter-$boot_device_id'"
cat << EOF
-menuentry "${LLABEL} $onstr" --class gnu-linux --class gnu --class os {
+menuentry "${LLABEL} $onstr" --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-$LKERNEL-${recovery_params}-$boot_device_id' {
EOF
save_default_entry | sed -e "s/^/\t/"
if [ -z "${prepare_boot_cache}" ]; then
@@ -192,7 +204,7 @@
hurd)
onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
cat << EOF
-menuentry "${LONGNAME} $onstr" --class hurd --class gnu --class os {
+menuentry "${LONGNAME} $onstr" --class hurd --class gnu --class os \$menuentry_id_option 'osprober-gnuhurd-/boot/gnumach.gz-false-$(grub_get_device_id "${DEVICE}")' {
EOF
save_default_entry | sed -e "s/^/\t/"
prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-03-03 14:27 ` Jordi Mallach
2012-03-03 14:51 ` Andreas Vogel
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Jordi Mallach @ 2012-03-03 14:27 UTC (permalink / raw)
To: The development of GNU GRUB
On Sat, Mar 03, 2012 at 02:37:12PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Attached patch changes it to the use of IDs specified by --id but
> keeps title possiblity for backward compatibility. Any comments?
Massive +1 here!
This was something that bugged me quite a lot. Thanks for squeezing it
into 2.0!
--
Jordi Mallach Pérez -- Debian developer http://www.debian.org/
jordi@sindominio.net jordi@debian.org http://www.sindominio.net/
GnuPG public key information available at http://oskuro.net/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 14:27 ` Jordi Mallach
@ 2012-03-03 14:51 ` Andreas Vogel
2012-03-03 15:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 19:48 ` Andreas Vogel
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-03 14:51 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]
Vladimir,
this enhancement is part of my patches which i sent some days ago.....
I introduced an option "--label STRING" which will be used for display
if it's set and which can also have environment variables (another part
of my patches).
It's just a matter of taste though if to use a new option --id for an
invariant menu id or to use the actual menuentry title as the invariant
id and to have a new option --label for the display string. Both ways
provide backward compatibility.
BTW, I just wonder a little that you refused my enhancements due to code
freeze but are now enhancing anyway ;-)
Andreas
Am 03.03.2012 14:37, schrieb Vladimir '?-coder/phcoder' Serbinenko:
> Hello, all. Currently we use and recommend using the title for default
> variable. It has however following problems:
> 1) When device names change the title changes (because of the "(on
> $device)" part)
> 2) If user changes locale the part ", with" gets translated and again
> the title changes
> Attached patch changes it to the use of IDs specified by --id but
> keeps title possiblity for backward compatibility. Any comments?
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
--
Andreas Vogel
Dipl.-Inform.
Hellerweg 60
73728 Esslingen
Germany
____________________________________________
E-Mail: Andreas.Vogel@anvo-it.de
Office: +49 (711) 937 1742
FAX: +49 (711) 937 1745
Mobile: +49 (172) 730 7440
[-- Attachment #2: Type: text/html, Size: 2376 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 14:51 ` Andreas Vogel
@ 2012-03-03 15:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-03 15:06 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Andreas Vogel
On 03.03.2012 15:51, Andreas Vogel wrote:
> Vladimir,
>
> this enhancement is part of my patches which i sent some days ago.....
>
> I introduced an option "--label STRING" which will be used for display
> if it's set and which can also have environment variables (another
> part of my patches).
>
> It's just a matter of taste though if to use a new option --id for an
> invariant menu id or to use the actual menuentry title as the
> invariant id and to have a new option --label for the display string.
> Both ways provide backward compatibility.
>
> BTW, I just wonder a little that you refused my enhancements due to
> code freeze but are now enhancing anyway ;-)
>
The reason is simple: you submitted a huge patch for several things,
most of them new features. If you had submitted small patches each
making one consistent logical change (but it may depend on other
changes) then I would have reviewed the ones fixing bugs (like the one
you describe). That's the general truth that 20 small patches go in much
faster than one patch big patch doing 20 things put together.
> Andreas
>
>
> Am 03.03.2012 14:37, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>> Hello, all. Currently we use and recommend using the title for
>> default variable. It has however following problems:
>> 1) When device names change the title changes (because of the "(on
>> $device)" part)
>> 2) If user changes locale the part ", with" gets translated and again
>> the title changes
>> Attached patch changes it to the use of IDs specified by --id but
>> keeps title possiblity for backward compatibility. Any comments?
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
> --
> Andreas Vogel
> Dipl.-Inform.
> Hellerweg 60
> 73728 Esslingen
> Germany
> ____________________________________________
> E-Mail:Andreas.Vogel@anvo-it.de
> Office: +49 (711) 937 1742
> FAX: +49 (711) 937 1745
> Mobile: +49 (172) 730 7440
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 14:27 ` Jordi Mallach
2012-03-03 14:51 ` Andreas Vogel
@ 2012-03-03 19:48 ` Andreas Vogel
2012-03-03 20:04 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 20:08 ` Andreas Vogel
2012-03-04 23:51 ` Andreas Vogel
4 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-03 19:48 UTC (permalink / raw)
To: The development of GNU GRUB
I was thinking about your proposal again and I think your idea
introducing a --id option is better than my idea having a --label option.
> === modified file 'include/grub/menu.h'
> --- include/grub/menu.h 2011-01-10 22:27:58 +0000
> +++ include/grub/menu.h 2012-03-03 12:11:30 +0000
> @@ -32,6 +32,9 @@
> /* The title name. */
> const char *title;
>
> + /* The identifier. */
> + const char *id;
> +
> /* If set means not everybody is allowed to boot this entry. */
> int restricted;
>
Why are all the string variables declared as "const char *"? In my
opinion the const qualifier is not useful/needed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 19:48 ` Andreas Vogel
@ 2012-03-03 20:04 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-03 20:04 UTC (permalink / raw)
To: grub-devel
On 03.03.2012 20:48, Andreas Vogel wrote:
> I was thinking about your proposal again and I think your idea
> introducing a --id option is better than my idea having a --label option.
>
>> === modified file 'include/grub/menu.h'
>> --- include/grub/menu.h 2011-01-10 22:27:58 +0000
>> +++ include/grub/menu.h 2012-03-03 12:11:30 +0000
>> @@ -32,6 +32,9 @@
>> /* The title name. */
>> const char *title;
>>
>> + /* The identifier. */
>> + const char *id;
>> +
>> /* If set means not everybody is allowed to boot this entry. */
>> int restricted;
>>
> Why are all the string variables declared as "const char *"? In my
> opinion the const qualifier is not useful/needed.
This string is used for quite long time in different functions and the
small corruption can lead to difficult to trace bugs. With const
compiler takes care of checking them.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
` (2 preceding siblings ...)
2012-03-03 19:48 ` Andreas Vogel
@ 2012-03-03 20:08 ` Andreas Vogel
2012-03-03 20:10 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-04 23:51 ` Andreas Vogel
4 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-03 20:08 UTC (permalink / raw)
To: The development of GNU GRUB
> + menu_id = grub_strdup (id ? : menu_title);
I don't know about the policy for grub code regarding GNU compiler
extensions... at least you don't need in this place.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 20:08 ` Andreas Vogel
@ 2012-03-03 20:10 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-03 20:10 UTC (permalink / raw)
To: grub-devel
On 03.03.2012 21:08, Andreas Vogel wrote:
>> + menu_id = grub_strdup (id ? : menu_title);
> I don't know about the policy for grub code regarding GNU compiler
> extensions... at least you don't need in this place.
We support only GCC. Any of its extensions available since 4.2 and not
too obscure can be used freely.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
` (3 preceding siblings ...)
2012-03-03 20:08 ` Andreas Vogel
@ 2012-03-04 23:51 ` Andreas Vogel
2012-03-05 0:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
4 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-04 23:51 UTC (permalink / raw)
To: The development of GNU GRUB
Cc: Vladimir 'φ-coder/phcoder' Serbinenko
Am 03.03.2012 14:37, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
> === modified file 'grub-core/commands/menuentry.c'
> --- grub-core/commands/menuentry.c 2012-02-26 16:28:05 +0000
> +++ grub-core/commands/menuentry.c 2012-03-03 12:11:30 +0000
> @@ -36,6 +36,8 @@
> N_("Keyboard key to quickly boot this entry."), N_("KEYBOARD_KEY"), ARG_TYPE_STRING},
> {"source", 4, 0,
> N_("Use STRING as menu entry body."), N_("STRING"), ARG_TYPE_STRING},
> + {"id", 1, GRUB_ARG_OPTION_REPEATABLE,
> + N_("Menu entry identifier."), N_("STRING"), ARG_TYPE_STRING},
> {0, 0, 0, 0, 0, 0}
> };
1) After bzr-pulling your latest changes and reviewing that source
again, i noticed that you are using GRUB_ARG_OPTION_REPEATABLE for the
new id option but you are not handling multiple ids (as far as i can see).
2) Why do you refuse to allow short options for all of the menuentry
options? Any special reason?
3) Wouldn't it be a good chance to use my patch which uses an anonymous
enum for indexing the options array?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-04 23:51 ` Andreas Vogel
@ 2012-03-05 0:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-05 0:20 ` Andreas Vogel
0 siblings, 1 reply; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-05 0:03 UTC (permalink / raw)
To: Andreas Vogel; +Cc: The development of GNU GRUB
On 05.03.2012 00:51, Andreas Vogel wrote:
> Am 03.03.2012 14:37, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>> === modified file 'grub-core/commands/menuentry.c'
>> --- grub-core/commands/menuentry.c 2012-02-26 16:28:05 +0000
>> +++ grub-core/commands/menuentry.c 2012-03-03 12:11:30 +0000
>> @@ -36,6 +36,8 @@
>> N_("Keyboard key to quickly boot this entry."), N_("KEYBOARD_KEY"), ARG_TYPE_STRING},
>> {"source", 4, 0,
>> N_("Use STRING as menu entry body."), N_("STRING"), ARG_TYPE_STRING},
>> + {"id", 1, GRUB_ARG_OPTION_REPEATABLE,
>> + N_("Menu entry identifier."), N_("STRING"), ARG_TYPE_STRING},
>> {0, 0, 0, 0, 0, 0}
>> };
> 1) After bzr-pulling your latest changes and reviewing that source
> again, i noticed that you are using GRUB_ARG_OPTION_REPEATABLE for the
> new id option but you are not handling multiple ids (as far as i can see).
It wasn't intentional. Fixed now.
> 2) Why do you refuse to allow short options for all of the menuentry
> options? Any special reason?
Because it shares the space with options to menuentry.
> 3) Wouldn't it be a good chance to use my patch which uses an anonymous
> enum for indexing the options array?
No. Thinking like this is a slippery slope. Such patches may also
contain bugs (if you confuse 2 numbers).
Your patch could be committed into experimental but not trunk.
Life doesn't end at 2.00.
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-05 0:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-03-05 0:20 ` Andreas Vogel
2012-03-05 0:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-05 0:20 UTC (permalink / raw)
To: Vladimir 'φ-coder/phcoder' Serbinenko
Cc: The development of GNU GRUB
Am 05.03.2012 01:03, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>> options? Any special reason?
> 2) Why do you refuse to allow short options for all of the menuentry
> Because it shares the space with options to menuentry.
Don't understand what you mean. What is shared?
>> 3) Wouldn't it be a good chance to use my patch which uses an anonymous
>> enum for indexing the options array?
> No. Thinking like this is a slippery slope. Such patches may also
> contain bugs (if you confuse 2 numbers).
> Your patch could be committed into experimental but not trunk.
> Life doesn't end at 2.00.
Nice joke.... that code right now is a slippery slope... with all those
obfuscated numbers. Just my 2 cents...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-05 0:20 ` Andreas Vogel
@ 2012-03-05 0:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-05 11:51 ` Andreas Vogel
0 siblings, 1 reply; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-05 0:34 UTC (permalink / raw)
To: Andreas Vogel; +Cc: The development of GNU GRUB
On 05.03.2012 01:20, Andreas Vogel wrote:
> Am 05.03.2012 01:03, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>>> options? Any special reason?
>> 2) Why do you refuse to allow short options for all of the menuentry
>> Because it shares the space with options to menuentry.
> Don't understand what you mean. What is shared?
menuentry "title" hello {
echo $1; sleep 10
}
>>> 3) Wouldn't it be a good chance to use my patch which uses an anonymous
>>> enum for indexing the options array?
>> No. Thinking like this is a slippery slope. Such patches may also
>> contain bugs (if you confuse 2 numbers).
>> Your patch could be committed into experimental but not trunk.
>> Life doesn't end at 2.00.
> Nice joke.... that code right now is a slippery slope... with all those
> obfuscated numbers. Just my 2 cents...
>
It may be unreadable and more difficult to maintain but neither matters
much for the release. Readability cleanup can be done after release
without problems.
If we start clean this and other files now, we'll spend too much time
and be generally counterproductive.
I understand that it may feel as if I'm having anything against you but
I don't, just releasing 2.00 is now a top priority and because of this
fixing bugs is important. Everything that is not to fix bugs or was
already announced at freeze as still accepted (all of which is now in)
is secondary now.
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-05 0:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-03-05 11:51 ` Andreas Vogel
2012-03-05 12:37 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Vogel @ 2012-03-05 11:51 UTC (permalink / raw)
To: Vladimir 'φ-coder/phcoder' Serbinenko
Cc: The development of GNU GRUB
Am 05.03.2012 01:34, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
> On 05.03.2012 01:20, Andreas Vogel wrote:
>> Am 05.03.2012 01:03, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>>>> options? Any special reason?
>>> 2) Why do you refuse to allow short options for all of the menuentry
>>> Because it shares the space with options to menuentry.
>> Don't understand what you mean. What is shared?
> menuentry "title" hello {
> echo $1; sleep 10
> }
I'm really sorry, but i still don't understand.... and I'd really like
to understand it.
Why is it OK for the menuentry command to accept long options but not OK
to accept short options? What is the difference here handling long
options opposed to handling short options?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Device instability, gettext and default
2012-03-05 11:51 ` Andreas Vogel
@ 2012-03-05 12:37 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-03-05 12:37 UTC (permalink / raw)
Cc: The development of GNU GRUB
On 05.03.2012 12:51, Andreas Vogel wrote:
> Am 05.03.2012 01:34, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>> On 05.03.2012 01:20, Andreas Vogel wrote:
>>> Am 05.03.2012 01:03, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
>>>>> options? Any special reason?
>>>> 2) Why do you refuse to allow short options for all of the menuentry
>>>> Because it shares the space with options to menuentry.
>>> Don't understand what you mean. What is shared?
>> menuentry "title" hello {
>> echo $1; sleep 10
>> }
> I'm really sorry, but i still don't understand.... and I'd really like
> to understand it.
>
> Why is it OK for the menuentry command to accept long options but not OK
> to accept short options? What is the difference here handling long
> options opposed to handling short options?
The menuentry itself might want to define options on its own. Space of
long options is large enough to easily avoid conflicts but the space of
short ones isn't.
>
>
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-03-05 12:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-03 13:37 Device instability, gettext and default Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 14:27 ` Jordi Mallach
2012-03-03 14:51 ` Andreas Vogel
2012-03-03 15:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 19:48 ` Andreas Vogel
2012-03-03 20:04 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-03 20:08 ` Andreas Vogel
2012-03-03 20:10 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-04 23:51 ` Andreas Vogel
2012-03-05 0:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-05 0:20 ` Andreas Vogel
2012-03-05 0:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-03-05 11:51 ` Andreas Vogel
2012-03-05 12:37 ` Vladimir 'φ-coder/phcoder' Serbinenko
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.