move functionality of font_path() directly to util/grub-mkconfig.in and prefer unicode.pf2 over ascii.pf2
PATCH 1/1 move functionality of font_path() directly to util/grub-mkconfig.in and prefer unicode.pf2 over ascii.pf2
As discussed with Robert on IRC I moved the font_path function directly to grub-mkconfig where it's only used.
And now the unicode font gets prefered over the ascii one. And LANG=C gets exported if ascii.pf2 gets used, which is the main reason for the move of it.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
2009-08-06 Felix Zielcke <[email protected]>
* util/grub-mkconfig_lib.in (font_path): Move the functionality of it to ...
* util/grub-mkconfig.in: ... here. Prefer unicode.pf2 and unifont.pf2 over ascii.pf2. Export LANG=C in case ascii.pf2 gets used.
grub-mkconfig.in +21 -11
@@ -24,6 +24,8 @@ sbindir=@sbindir@ libdir=@libdir@ sysconfdir=@sysconfdir@ package_version=@PACKAGE_VERSION@ +datadir=@datadir@ +pkgdatadir=${datadir}/`echo @PACKAGE_TARNAME@ | sed "${transform}"` grub_prefix=`echo /boot/grub | sed ${transform}` grub_cfg="" grub_mkconfig_dir=${sysconfdir}/grub.d
@@ -151,22 +153,30 @@ esac # check for terminals that require fonts case ${GRUB_TERMINAL_OUTPUT} in gfxterm) - if path=`font_path` ; then - GRUB_FONT_PATH="${path}" - else - # fallback to the native terminal for this platform - unset GRUB_TERMINAL_OUTPUT + for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do + for basename in unicode unifont ascii; do + path="${dir}/${basename}.pf2" + if is_path_readable_by_grub ${path} > /dev/null ; then + GRUB_FONT_PATH=${path} + else + continue + fi + if [ "${basename}" = "ascii"] ; then + # make sure all our children behave in conformance with ascii.. + export LANG=C + fi + break 2 + done + done + if [ -z "${GRUB_FONT_PATH}" ] ; then + # fallback to the native terminal for this platform + unset GRUB_TERMINAL_OUTPUT + fi fi ;; -esac - -# does our terminal support utf-8 ? -case ${GRUB_TERMINAL_OUTPUT} in - gfxterm) ;; *) # make sure all our children behave in conformance with ascii.. export LANG=C - ;; esac # These are defined in this script, export them here so that user can
grub-mkconfig_lib.in +0 -17
@@ -148,23 +148,6 @@ prepare_grub_to_access_device () fi } -font_path () -{ - for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do - # FIXME: We prefer ascii because loading complete fonts is too slow (and - # we don't yet provide the gettext magic that would make unicode useful). - for basename in ascii unicode unifont ; do - path="${dir}/${basename}.pf2" - if is_path_readable_by_grub ${path} > /dev/null ; then - echo "${path}" - return 0 - fi - done - done - - return 1 -} - grub_file_is_not_garbage () { if test -f "$1" ; then
Quoted 21 lines · Felix Zielcke
grub-mkconfig.in +19
-0+ for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do + for basename in unicode unifont ascii; do + path="${dir}/${basename}.pf2" + if is_path_readable_by_grub ${path} > /dev/null ; then + GRUB_FONT_PATH=${path} + else + continue + fi + if [ "${basename}" = "ascii"] ; then + # make sure all our children behave in conformance with ascii.. + export LANG=C + fi + break 2 + done + done + if [ -z "${GRUB_FONT_PATH}" ] ; then + # fallback to the native terminal for this platform + unset GRUB_TERMINAL_OUTPUT + fi fi ;;
grub-mkconfig.in esac · line 180
Quoted 10 lines
grub-mkconfig.in +0
-6-esac - -# does our terminal support utf-8 ? -case ${GRUB_TERMINAL_OUTPUT} in - gfxterm) ;; *) # make sure all our children behave in conformance with ascii.. export LANG=C - ;; esac
It seems that "export LANG=C" is issued twice?
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Am Freitag, den 07.08.2009, 13:39 +0200 schrieb Robert Millan:
Quoted 26 lines · Felix Zielcke
Quoted 25 lines
grub-mkconfig.in +19
-4+ for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do + for basename in unicode unifont ascii; do + path="${dir}/${basename}.pf2" + if is_path_readable_by_grub ${path} > /dev/null ; then + GRUB_FONT_PATH=${path} + else + continue + fi + if [ "${basename}" = "ascii"] ; then + # make sure all our children behave in conformance with ascii.. + export LANG=C + fi + break 2 + done + done + if [ -z "${GRUB_FONT_PATH}" ] ; then + # fallback to the native terminal for this platform + unset GRUB_TERMINAL_OUTPUT + fi fi ;; -esac - -# does our terminal support utf-8 ? -case ${GRUB_TERMINAL_OUTPUT} in
Quoted 9 lines
grub-mkconfig.in esac · line 180
Quoted 6 lines
grub-mkconfig.in +0
-2- gfxterm) ;; *) # make sure all our children behave in conformance with ascii.. export LANG=C - ;; esacIt seems that "export LANG=C" is issued twice?
Once if gfxterm gets used and once like before for all other output terminals.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
Am Freitag, den 07.08.2009, 14:02 +0200 schrieb Felix Zielcke:
Quoted 36 lines
Am Freitag, den 07.08.2009, 13:39 +0200 schrieb Robert Millan:
Quoted 26 lines · Felix Zielcke
Quoted 25 lines
grub-mkconfig.in +19
-4+ for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do + for basename in unicode unifont ascii; do + path="${dir}/${basename}.pf2" + if is_path_readable_by_grub ${path} > /dev/null ; then + GRUB_FONT_PATH=${path} + else + continue + fi + if [ "${basename}" = "ascii"] ; then + # make sure all our children behave in conformance with ascii.. + export LANG=C + fi + break 2 + done + done + if [ -z "${GRUB_FONT_PATH}" ] ; then + # fallback to the native terminal for this platform + unset GRUB_TERMINAL_OUTPUT + fi fi ;; -esac - -# does our terminal support utf-8 ? -case ${GRUB_TERMINAL_OUTPUT} inQuoted 9 lines
grub-mkconfig.in esac · line 180
Quoted 6 lines
grub-mkconfig.in +0
-2- gfxterm) ;; *) # make sure all our children behave in conformance with ascii.. export LANG=C - ;; esacIt seems that "export LANG=C" is issued twice?
Quoted 2 lines
Once if gfxterm gets used and once like before for all other output terminals.
I commited it now with an ack from Robert on IRC.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
Quoted 1 line · Felix Zielcke
I commited it now with an ack from Robert on IRC.
Sorry, I'm commenting after it has been committed. Anyway, please note that having an approval doesn't absolve you from testing the code on your own. Reviews are not testing. There was a warning introduced by your change, and there was a syntax error after "ascii". Also, the formatting of the moved code should have been changed to use the same indentation as the target file. I have fixed all that.
Could you please explain what I should do to keep using ascii.pf2? I checked the script, but don't see any variable controlling that. "make install" would install both unicode.pf2 and ascii.pf2, so unicode.pf2 would always be preferred.
If changing the default, it's a good style to provide an easy way for users to keep the old setting, and I just don't see it, short or removing /usr/src/unifont.bdf and /usr/local/share/grub/unicode.pf2 so that they are never reinstalled or detected by GRUB.
Signature · 2 lines
Regards, Pavel Roskin
Am Samstag, den 08.08.2009, 01:12 -0400 schrieb Pavel Roskin:
Quoted 10 lines · Felix Zielcke
Quoted 2 lines
I commited it now with an ack from Robert on IRC.
Sorry, I'm commenting after it has been committed. Anyway, please note that having an approval doesn't absolve you from testing the code on your own. Reviews are not testing. There was a warning introduced by your change, and there was a syntax error after "ascii". Also, the formatting of the moved code should have been changed to use the same indentation as the target file. I have fixed all that.
Sorry.
Quoted 4 lines
Could you please explain what I should do to keep using ascii.pf2? I checked the script, but don't see any variable controlling that. "make install" would install both unicode.pf2 and ascii.pf2, so unicode.pf2 would always be preferred.
With the old code ascii.pf2 would be always preferred. There wasn't either a way to specify it.
Quoted 4 lines
If changing the default, it's a good style to provide an easy way for users to keep the old setting, and I just don't see it, short or removing
/usr/src/unifont.bdfand/usr/local/share/grub/unicode.pf2so that they are never reinstalled or detected by GRUB.
Here's now a patch which allows users to specifiy the used font with GRUB_FONT=ascii
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
2009-08-08 Felix Zielcke <[email protected]>
* util/grub-mkconfig.in: Allow the user to specify the used font with GRUB_FONT.
util/grub-mkconfig.in +14 -0
@@ -155,6 +155,19 @@ esac case ${GRUB_TERMINAL_OUTPUT} in gfxterm) for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do + if [ -n "$GRUB_FONT" ] ; then + path="${dir}/${GRUB_FONT}.pf2" + if is_path_readable_by_grub ${path} > /dev/null ; then + GRUB_FONT_PATH=${path} + if [ "${GRUB_FONT}" = "ascii" ] + export LANG=C + fi + break + else + echo "No such font ${GRUB_FONT}.pf2" >&2 + exit 1 + fi + else for basename in unicode unifont ascii; do path="${dir}/${basename}.pf2" if is_path_readable_by_grub ${path} > /dev/null ; then
@@ -168,6 +181,7 @@ case ${GRUB_TERMINAL_OUTPUT} in fi break 2 done + fi done if [ -z "${GRUB_FONT_PATH}" ] ; then # fallback to the native terminal for this platform
Am Samstag, den 08.08.2009, 07:36 +0200 schrieb Felix Zielcke:
Quoted 22 lines
Am Samstag, den 08.08.2009, 01:12 -0400 schrieb Pavel Roskin:
Quoted 10 lines · Felix Zielcke
Quoted 2 lines
I commited it now with an ack from Robert on IRC.
Sorry, I'm commenting after it has been committed. Anyway, please note that having an approval doesn't absolve you from testing the code on your own. Reviews are not testing. There was a warning introduced by your change, and there was a syntax error after "ascii". Also, the formatting of the moved code should have been changed to use the same indentation as the target file. I have fixed all that.
Sorry.
Quoted 4 lines
Could you please explain what I should do to keep using ascii.pf2? I checked the script, but don't see any variable controlling that. "make install" would install both unicode.pf2 and ascii.pf2, so unicode.pf2 would always be preferred.
With the old code ascii.pf2 would be always preferred. There wasn't either a way to specify it.
Quoted 8 lines
Quoted 4 lines
If changing the default, it's a good style to provide an easy way for users to keep the old setting, and I just don't see it, short or removing
/usr/src/unifont.bdfand/usr/local/share/grub/unicode.pf2so that they are never reinstalled or detected by GRUB.Here's now a patch which allows users to specifiy the used font with GRUB_FONT=ascii
Oh I forgot to mention that I left the indent changes with intent out, so it's more readable.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
Quoted 7 lines · Felix Zielcke
Quoted 4 lines
Could you please explain what I should do to keep using ascii.pf2? I checked the script, but don't see any variable controlling that. "make install" would install both unicode.pf2 and ascii.pf2, so unicode.pf2 would always be preferred.
With the old code ascii.pf2 would be always preferred. There wasn't either a way to specify it.
I see. Maybe that's what we should have fixed first.
Quoted 7 lines
Quoted 4 lines
If changing the default, it's a good style to provide an easy way for users to keep the old setting, and I just don't see it, short or removing
/usr/src/unifont.bdfand/usr/local/share/grub/unicode.pf2so that they are never reinstalled or detected by GRUB.Here's now a patch which allows users to specifiy the used font with GRUB_FONT=ascii
First of all, I hope that the patch you will actually commit will use sane formatting. Diffs that ignore spacing changes are OK for review, but not for applying as is.
I think it would be more natural to let the user specify the full path to the file. Setting LANG=C seems unneeded in this case. After all, it's the user's choice, and we cannot examine the font file to check which characters it has.
Signature · 2 lines
Regards, Pavel Roskin
Am Samstag, den 08.08.2009, 01:49 -0400 schrieb Pavel Roskin:
Quoted 17 lines · Felix Zielcke
Quoted 8 lines
Quoted 5 lines
Could you please explain what I should do to keep using ascii.pf2? I checked the script, but don't see any variable controlling that. "make install" would install both unicode.pf2 and ascii.pf2, so unicode.pf2 would always be preferred.
With the old code ascii.pf2 would be always preferred. There wasn't either a way to specify it.
I see. Maybe that's what we should have fixed first.
Quoted 5 lines
Quoted 4 lines
If changing the default, it's a good style to provide an easy way for users to keep the old setting, and I just don't see it, short or removing
/usr/src/unifont.bdfand/usr/local/share/grub/unicode.pf2so that they are never reinstalled or detected by GRUB.
Quoted 6 lines
Quoted 2 lines
Here's now a patch which allows users to specifiy the used font with GRUB_FONT=ascii
First of all, I hope that the patch you will actually commit will use sane formatting. Diffs that ignore spacing changes are OK for review, but not for applying as is.
Yes.
Quoted 4 lines
I think it would be more natural to let the user specify the full path to the file. Setting LANG=C seems unneeded in this case. After all, it's the user's choice, and we cannot examine the font file to check which characters it has.
Ok here's a new one.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer
2009-08-08 Felix Zielcke <[email protected]>
* util/grub-mkconfig.in: Allow the user to specify the used font with GRUB_FONT.
grub-mkconfig.in +9 -0
@@ -154,6 +154,14 @@ esac # check for terminals that require fonts case ${GRUB_TERMINAL_OUTPUT} in gfxterm) + if [ -n "$GRUB_FONT" ] ; then + if is_path_readable_by_grub ${GRUB_FONT} > /dev/null ; then + GRUB_FONT_PATH=${GRUB_FONT} + else + echo "No such font or not readable by grub: ${GRUB_FONT}" >&2 + exit 1 + fi + else for dir in ${pkgdatadir} /boot/grub /usr/share/grub ; do for basename in unicode unifont ascii; do path="${dir}/${basename}.pf2"
@@ -169,6 +177,7 @@ case ${GRUB_TERMINAL_OUTPUT} in break 2 done done + fi if [ -z "${GRUB_FONT_PATH}" ] ; then # fallback to the native terminal for this platform unset GRUB_TERMINAL_OUTPUT
Quoted 6 lines · Felix Zielcke
Quoted 4 lines
I think it would be more natural to let the user specify the full path to the file. Setting LANG=C seems unneeded in this case. After all, it's the user's choice, and we cannot examine the font file to check which characters it has.
Ok here's a new one.
Looks good to me.
Signature · 2 lines
Regards, Pavel Roskin
Am Samstag, den 08.08.2009, 02:35 -0400 schrieb Pavel Roskin:
Quoted 11 lines · Felix Zielcke
Quoted 7 lines
Quoted 5 lines
I think it would be more natural to let the user specify the full path to the file. Setting LANG=C seems unneeded in this case. After all, it's the user's choice, and we cannot examine the font file to check which characters it has.
Ok here's a new one.
Looks good to me.
Ok commited with style changed.
Signature · 2 lines
Felix Zielcke Proud Debian Maintainer