Daniel Reurich <dan...@centurion.net.nz> writes: > I'd like some other eyes to look over my code and point out any > improvements/flaw. > > I haven't tested the code for setting the GRUB_THEME variable in > /etc/default/grub (in debian/postinst) and I think it's rather crude to > do it that way.
Some comments on that: -------- $DEFAULT_GRUB="/etc/default/grub" -------- The $ seems to be a syntax error. -------- update-boot(){ if [ -n $1 ]; then case $1 in commented) echo -e "\n$DEF_GRUB_COMMENT\n# $DEF_GRUB_SETTING" >> $DEFAULT_GRUB return ;; *) echo -e "\n$DEF_GRUB_COMMENT\n$DEF_GRUB_SETTING" >> $DEFAULT_GRUB ;; esac fi if which update-grub2 > /dev/null ; then sync update-grub2 || true fi if [ -x /usr/sbin/update-initramfs ]; then update-initramfs -u fi } -------- It should be possible to express this as (untested) ------- update_boot() { if [ "$1" = commented ]; then echo -e "\n$DEF_GRUB_COMMENT\n# $DEF_GRUB_SETTING" >> $DEFAULT_GRUB return fi echo -e "\n$DEF_GRUB_COMMENT\n$DEF_GRUB_SETTING" >>$DEFAULT_GRUB if which update-grub2 > /dev/null ; then sync update-grub2 || true fi if [ -x /usr/sbin/update-initramfs ]; then update-initramfs -u fi } -------- IMHO, considering the use of printf instead of echo -e is also worthwhile, as in printf '\n%s\n#% s\n' "$DEF_GRUB_COMMENT" "$DEF_GRUB_SETTING" >> $DEFAULT_GRUB -------- while read confline; do case confline in $DEF_GRUB_COMMENT) theme_default_conf=true; ;; $DEF_GRUB_SETTING) theme_parm="matching" ;; "#$DEF_GRUB_SETTING"|"# $DEF_GRUB_SETTING") theme_parm="disabled" ---------- missing ;; ---------- GRUB_THEME=*) theme_parm="modified" ;; esac done --------- This reads from stdin which was probably not intended. _______________________________________________ Dng mailing list Dng@lists.dyne.org https://mailinglists.dyne.org/cgi-bin/mailman/listinfo/dng