Review Request: Support for GRUB2 submenus

[Konstantinos Smanis]

> On July 17, 2012, 10:23 p.m., Oswald Buddenhagen wrote:
> > kdm/backend/bootman.c, line 231
> > <http://git.reviewboard.kde.org/r/105563/diff/2/?file=73175#file73175line231>
> >
> >     you don't seem to be actually counting opening braces, so detaching the two counts makes little sense.
> 
> Konstantinos Smanis wrote:
>     True, but we still have to know if a closing brace is from a 'menuentry' or a 'submenu'.

I could drop both counters and use an unsigned char as bool for marking when we are inside a menuentry. Would you prefer that?


> On July 17, 2012, 10:23 p.m., Oswald Buddenhagen wrote:
> > kdm/backend/bootman.c, line 265
> > <http://git.reviewboard.kde.org/r/105563/diff/2/?file=73175#file73175line265>
> >
> >     use extStrArr()

extStrArr is static in util.c and btw addStrArr makes use of it. Do we really have to?


> On July 17, 2012, 10:23 p.m., Oswald Buddenhagen wrote:
> > kdm/backend/bootman.c, line 282
> > <http://git.reviewboard.kde.org/r/105563/diff/2/?file=73175#file73175line282>
> >
> >     as you are putting the submenus into the selection list anyway, "menus" needs to be only a stack of integers (and you can just allocate a fixed-size array for that - nobody is going to nest beyond 3 (does grub even support nesting? i didn't gather that impression).
> 
> Konstantinos Smanis wrote:
>     Grub2 now supports nesting (dunno up to which level but I have tested with 3 levels and it worked). Distributions create only one nesting level to my knowledge but that doesn't mean a user can't have custom generation scripts.

Another reason we can't use integers is because the submenus' names are prepended to nested menuentries and this list will be shown to the user. We could use integers as internal representation but imho that would be unfair memory-to-code-clarity tradeoff.


- Konstantinos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105563/#review16042
-----------------------------------------------------------


On July 17, 2012, 10 p.m., Konstantinos Smanis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105563/
> -----------------------------------------------------------
> 
> (Updated July 17, 2012, 10 p.m.)
> 
> 
> Review request for KDE Runtime and Oswald Buddenhagen.
> 
> 
> Description
> -------
> 
> Recent versions of GRUB2 introduce submenus which allow for nesting levels to appear (instead of the flat list in the past).
> This patch consists of two parts: the parsing part in KDM (bootman.c) and creating a menu structure from the parsed list in ksmserver (shutdowndlg.*)
> The parsing part produces a list like this:
> 
> Gentoo GNU/Linux
> Advanced options for Gentoo GNU/Linux
> Advanced options for Gentoo GNU/Linux>Gentoo GNU/Linux, with Linux 3.4.4
> Advanced options for Gentoo GNU/Linux>Gentoo GNU/Linux, with Linux 3.4.4 (recovery mode)
> Windows 7 (loader) (on /dev/sda2)
> ???? ?????????
> 
> which is then converted into the menu structure. These full identifiers can be properly used with `grub2-reboot`.
> 
> More info about submenus: http://ubuntuforums.org/showthread.php?p=10720316
> Also check the related bug.
> 
> The parsing part of the patch can be applied in the KDE/4.9 and master branches as well (tested and working in KDE 4.8.95 and 4.9.x). ksmserver has migrated to QML since then however, and the menu structure has to wait. Currently it looks like this: http://i50.tinypic.com/96bw35.png
> Related ML-discussion: http://lists.kde.org/?l=kde-core-devel&m=134160279511422&w=2
> Update: There is a proper fix now for KDE/4.9 and master: https://git.reviewboard.kde.org/r/105568/
> 
> 
> This addresses bug 297209.
>     http://bugs.kde.org/show_bug.cgi?id=297209
> 
> 
> Diffs
> -----
> 
>   kdm/backend/bootman.c 8b834d2 
>   ksmserver/shutdowndlg.h e5f0942 
>   ksmserver/shutdowndlg.cpp a09a1a7 
> 
> Diff: http://git.reviewboard.kde.org/r/105563/diff/
> 
> 
> Testing
> -------
> 
> Works with the menu file produced in my system with `grub2-mkconfig`.
> Also works with a custom menu file I made (as shown in the second screenshot).
> 
> 
> Screenshots
> -----------
> 
> Distribution's stock menu file
>   http://git.reviewboard.kde.org/r/105563/s/633/
> A custom menu file for testing
>   http://git.reviewboard.kde.org/r/105563/s/634/
> 
> 
> Thanks,
> 
> Konstantinos Smanis
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120729/7849a221/attachment.htm>