ngraham added a comment.
Thanks! Please also remove ` - Bug 395349` from the title and replace it with `BUG: 395349` in the Summary section. --- Now the patch looks better, applies cleanly, and works in my testing. However--and this is a complaint with other KDE software as well--if you remove the menubar, there is no GUI method to get it back. You have to have already known about the [Ctrl] + [M] shortcut, because once the menubar is hidden, there's no way to learn it from ksysguard itself. Different KDE apps handle this in different ways: - Dolphin puts most of the menubar's functionality under a Control button that appears in the toolbar when the menubar is hidden (not bad) - Kate displays a dialog warning you and including a reminder about the [Ctrl] + [M] shortcut (better than nothing, but nobody will read it or remember the lesson): F6277660: Hide.png <https://phabricator.kde.org/F6277660> - Gwenview does nothing, leading to bug reports: https://bugs.kde.org/show_bug.cgi?id=210620 My worry is that if we implement this patch as-is, with no warning or safety valve or obvious way to restore the menubar or access the lost functionality, we will get bug reports like https://bugs.kde.org/show_bug.cgi?id=210620. Thoughts on how we can resolve this? INLINE COMMENTS > ksysguard.cpp:73 > > + > //Comment out to stop ksysguard from forking. Good for debugging Unrelated whitespace change > ksysguard.cpp:148 > connect(mConfigureSheetAction, &QAction::triggered, this, > &TopLevel::configureCurrentSheet); > - > + > + // setup 'Settings' menu Extra whitespace > ksysguard.h:78 > void updateProcessCount(); > - void configureCurrentSheet(); > + void configureCurrentSheet(); > + void toggleShowMenuBar(); Accidental trailing whitespace REVISION DETAIL https://phabricator.kde.org/D15644 To: lsartorelli, ngraham, #plasma, #frameworks Cc: acrouthamel, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
