----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105563/#review16042 -----------------------------------------------------------
tired or not ... kdm/backend/bootman.c <http://git.reviewboard.kde.org/r/105563/#comment12619> parseGrubTitle() would be a better name kdm/backend/bootman.c <http://git.reviewboard.kde.org/r/105563/#comment12622> you don't seem to be actually counting opening braces, so detaching the two counts makes little sense. kdm/backend/bootman.c <http://git.reviewboard.kde.org/r/105563/#comment12621> use strApp() kdm/backend/bootman.c <http://git.reviewboard.kde.org/r/105563/#comment12620> use extStrArr() kdm/backend/bootman.c <http://git.reviewboard.kde.org/r/105563/#comment12623> 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). ksmserver/shutdowndlg.cpp <http://git.reviewboard.kde.org/r/105563/#comment12625> no way. the backend should directly communicate the hierarchy separator. also to ksmserver (this is just the start): --- a/kdm/backend/ctrl.c +++ b/kdm/backend/ctrl.c @@ -769,9 +769,10 @@ processCtrl(const char *string, int len, int fd, struct display *d) } else if (!strcmp(ar[0], "listbootoptions")) { char **opts; + const char *sep; int def, cur, i, j; if (ar[1]) goto exce; - switch (getBootOptions(&opts, &def, &cur)) { + switch (getBootOptions(&opts, &def, &cur, &sep)) { case BO_NOMAN: fLog(d, fd, "notsup", "boot options unavailable"); @@ -796,5 +797,5 @@ processCtrl(const char *string, int len, int fd, struct display *d) } freeStrArr(opts); - writer(fd, cbuf, sprintf(cbuf, "\t%d\t%d\n", def, cur)); + writer(fd, cbuf, sprintf(cbuf, "\t%d\t%d\t%s\n", def, cur, sep)); goto bust; } else if (d) { ossi ksmserver/shutdowndlg.cpp <http://git.reviewboard.kde.org/r/105563/#comment12624> way too much code duplication. a leaf is just a special case of a general node. ksmserver/shutdowndlg.cpp <http://git.reviewboard.kde.org/r/105563/#comment12626> don't fix the style of code you didn't touch otherwise ksmserver/shutdowndlg.cpp <http://git.reviewboard.kde.org/r/105563/#comment12627> missing space after keyword execess braces (kdm uses qt style, not kdelibs style) - Oswald Buddenhagen 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 > >