> On Aug. 1, 2012, 6:25 a.m., Oswald Buddenhagen wrote: > > ksmserver/shutdowndlg.cpp, line 485 > > <http://git.reviewboard.kde.org/r/105563/diff/11/?file=75571#file75571line485> > > > > why do you do that? the non-nested case should be just the simple > > solution of the general nested case. > > Konstantinos Smanis wrote: > I noticed that my menuentry-case code kinda duplicates the old code but > could you please elaborate a little more on this? I am not sure I get how you > want the implementation. If-nesting for submenus and simple actions outside > of it? > > Oswald Buddenhagen wrote: > huh? i don't see why you need a special code path for "no submenus at > all". it's just a normal submenu structure with all leaves hanging off the > top-level menu.
OK, I got you now. > On Aug. 1, 2012, 6:25 a.m., Oswald Buddenhagen wrote: > > kdm/backend/bootman.c, line 281 > > <http://git.reviewboard.kde.org/r/105563/diff/11/?file=75565#file75565line281> > > > > if a closing brace can appear only alone, this line should become an > > else-if. if it can come after another statement, we need to think some more. > > Konstantinos Smanis wrote: > What do you mean here? A single if-else would require ANDing the external > condition (line ending in '}'). > > Oswald Buddenhagen wrote: > i don't quite understand what you mean, either. > > anyway, even if the closing brace can follow other statements on the same > line, its effect should not be visible until the other statements have been > processed. Ah, I thought you were referring to C's closing brace, not GRUB's one. I will sort this out! - Konstantinos ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105563/#review16732 ----------------------------------------------------------- On Aug. 1, 2012, 8:54 a.m., Konstantinos Smanis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105563/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2012, 8:54 a.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 > kdm/backend/ctrl.c 5d219fe > kdm/backend/dm.h 13e7b45 > kdm/backend/dm.c e0f1366 > kdm/backend/util.c 6cd93ef > ksmserver/shutdowndlg.h e5f0942 > ksmserver/shutdowndlg.cpp a09a1a7 > libs/kworkspace/kdisplaymanager.h 76f25a7 > libs/kworkspace/kdisplaymanager.cpp 28fabfc > > 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 > >
