[Differential] [Updated] D3040: Allow scaling documentation view (BUG 285162)

[Igor Kushnir]

igorkushnir marked 2 inline comments as done.
igorkushnir added a comment.


  I guess everyone is OK with Ctrl+0 working only when the documentation widget has focus. This widget actually gets focus on scrolling, even though individual windows on my system don't get focus in such cases. So this should not be a problem.
  
  I've noticed an issue with my own shortcut implementation: the shortcut does not work in the Programmer Dvorak layout, because this layout requires pressing Shift to access digits. It also does not work with numpad 0. So I'd like to replace `event->modifiers() == Qt::ControlModifier` with `(event->modifiers() & Qt::ControlModifier)`. This will reset zoom level even if Ctrl+Shift+0 or Ctrl+Alt+0 is pressed. Please let me know if this is likely to cause conflicts with some global shortcuts. If it is, I could write even more specific code: `(event->modifiers() & Qt::ControlModifier) && !(event->modifiers() & ~(Qt::ControlModifier | Qt::ShiftModifier | Qt::KeypadModifier))`.

INLINE COMMENTS

> rjvbb wrote in standarddocumentationview.cpp:84
> Not my review, but FWIW, that's turning a function that only does something the 1st time into one that either crashes or else leaks ZoomController instances.
> 
> IMHO asserts like this should only be used for conditions that cannot be handled gracefully regardless of whether they should never happen.

This code mimics what QWidget::insertAction <https://code.woboq.org/qt5/qtbase/src/widgets/kernel/qwidget.cpp.html#_ZN7QWidget12insertActionEP7QActionS1_> does with a nullptr parameter.

@mwolff, please confirm that you still prefer assertion. I'll make the change then.

REPOSITORY
  R33 KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D3040

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: igorkushnir, mwolff, #kdevelop
Cc: rjvbb, mwolff, apol, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170221/1dac8dee/attachment.html>