Differential D5037 Diff 22232 src/view/kateviewinternal.cpp

Changeset View

Standalone View

View Options

src/view/kateviewinternal.cpp

Show First 20 Lines • Show All 57 Lines • ▼ Show 20 Line(s)
58#include <QPixmap> 58​#include <QPixmap>
59#include <QApplication> 59​#include <QApplication>
60#include <QLayout> 60​#include <QLayout>
61#include <QStyle> 61​#include <QStyle>
62#include <QToolTip> 62​#include <QToolTip>
63 63​
64static const bool debugPainting = false; 64​static const bool debugPainting = false;
65 65​
66​class ZoomEventFilter
67​{
68​public:
69​ ZoomEventFilter(KateViewInternal *vi)
70​ : m_vi(vi)
71​ {
72​ }
73​
74​ bool detectZoomingEvent(QWheelEvent *e, Qt::KeyboardModifiers modifier = Qt::ControlModifier)
75​ {
76​ Qt::KeyboardModifiers modState = e->modifiers();
anthonyfieroniUnsubmitted
Not Done

About me, it's designed to unset modifiers so param looks unwanted.

anthonyfieroni: About me, it's designed to unset modifiers so param looks unwanted.
rjvbbAuthorUnsubmitted
Not Done

I don't disagree but not for exactly the same reason. I'm not sure the function is or should be *designed* for that task. My first reaction to the suggestion of creating a helper class was that it would have this somewhat unexpected side-effect. That's part of why I added a parameter.

Now the real question is of course if there would ever be use for those parameters. I personally see more use for accelerated wheel-scrolling than for wheel-zooming but there are probably other ways to trigger it, rather than a subtle timing trick.

The modifier argument is less doubtful in my mind; it could be under control of a user setting, which would probably even be a prerequisite if someone wants to allow accelerated scrolling (also tied to the Control modifier, AFAIK hardwired in Qt).

rjvbb: I don't disagree but not for exactly the same reason. I'm not sure the function is or should be…
kfunkUnsubmitted
Done

Style: Use =

kfunk: Style: Use ` = `
77​ if (modState == modifier) {
78​ if (m_lastWheelEvent.isValid()) {
79​ const qint64 deltaT = m_lastWheelEvent.elapsed();
80​ // Pressing the specified modifier key within 200ms of the previous "unmodified"
81​ // wheelevent is not allowed to toggle on text zooming
82​ if (m_lastWheelEventUnmodified && deltaT < 200) {
83​ m_ignoreZoom = true;
84​ }
85​ else if (deltaT > 1000) {
kfunkUnsubmitted
Done

Style: Indent off

kfunk: Style: Indent off
86​ // the protection is kept active for 1s after the last wheel event
87​ // TODO: this value should be tuned, preferrably by someone using
kfunkUnsubmitted
Done

Simplify: else if (...) {?

kfunk: Simplify: `else if (...) {`?
rjvbbAuthorUnsubmitted
Not Done

Why the question mark? Simplification should be perfectly fine here, no?

rjvbb: Why the question mark? Simplification should be perfectly fine here, no?
88​ // Ctrl+Wheel zooming frequently.
89​ m_ignoreZoom = false;
90​ }
91​ } else {
92​ // we can't say anything and have to assume there's nothing
93​ // accidental to the modifier being pressed.
94​ m_ignoreZoom = false;
95​ }
96​ m_lastWheelEventUnmodified = false;
97​ if (m_ignoreZoom) {
98​ // unset the modifier so the view scrollbars can handle the scroll
99​ // event and produce normal, not accelerated scrolling
100​ modState &= ~modifier;
anthonyfieroniUnsubmitted
Done

modeState &= ~modifier;

anthonyfieroni: modeState &= ~modifier;
101​ e->setModifiers(modState);
102​ }
103​ } else {
104​ // state is reset after any wheel event without the zoom modifier
105​ m_lastWheelEventUnmodified = true;
dhaumannUnsubmitted
Done

The comment talks about "ControlModifier", but the parameter allows also other modifies.

dhaumann: The comment talks about "ControlModifier", but the parameter allows also other modifies.
106​ m_ignoreZoom = false;
107​ }
108​ m_lastWheelEvent.start();
109​
110​ // inform the caller whether this event is allowed to trigger text zooming.
111​ return !m_ignoreZoom && modState == modifier;
112​ }
113​
114​protected:
115​ KateViewInternal *m_vi;
116​ QElapsedTimer m_lastWheelEvent;
117​ bool m_ignoreZoom = false;
118​ bool m_lastWheelEventUnmodified = false;
119​};
120​
66KateViewInternal::KateViewInternal(KTextEditor::ViewPrivate *view) 121​KateViewInternal::KateViewInternal(KTextEditor::ViewPrivate *view)
kfunkUnsubmitted
Done

Initialize here

kfunk: Initialize here
rjvbbAuthorUnsubmitted
Not Done

Instead of in the ctor, or in addition to?

The KateViewInternal class doesn't initialise any of its member variables this way.

(http://en.cppreference.com/w/cpp/language/data_members ?)

rjvbb: Instead of in the ctor, or in addition to? The KateViewInternal class doesn't initialise any…
67 : QWidget(view) 122​ : QWidget(view)
kfunkUnsubmitted
Done

Dito

kfunk: Dito
68 , editSessionNumber(0) 123​ , editSessionNumber(0)
69 , editIsRunning(false) 124​ , editIsRunning(false)
70 , m_view(view) 125​ , m_view(view)
71 , m_cursor(doc()->buffer(), KTextEditor::Cursor(0, 0), Kate::TextCursor::MoveOnInsert) 126​ , m_cursor(doc()->buffer(), KTextEditor::Cursor(0, 0), Kate::TextCursor::MoveOnInsert)
72 , m_mouse() 127​ , m_mouse()
73 , m_possibleTripleClick(false) 128​ , m_possibleTripleClick(false)
74 , m_completionItemExpanded(false) 129​ , m_completionItemExpanded(false)
75 , m_bm(doc()->newMovingRange(KTextEditor::Range::invalid(), KTextEditor::MovingRange::DoNotExpand)) 130​ , m_bm(doc()->newMovingRange(KTextEditor::Range::invalid(), KTextEditor::MovingRange::DoNotExpand))
▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Line(s)158{
182 237​
183 // update view if folding ranges change 238​ // update view if folding ranges change
184 connect(&m_view->textFolding(), SIGNAL(foldingRangesChanged()), SLOT(slotRegionVisibilityChanged())); 239​ connect(&m_view->textFolding(), SIGNAL(foldingRangesChanged()), SLOT(slotRegionVisibilityChanged()));
185 240​
186 m_displayCursor.setPosition(0, 0); 241​ m_displayCursor.setPosition(0, 0);
187 242​
188 setAcceptDrops(true); 243​ setAcceptDrops(true);
189 244​
245​ m_zoomEventFilter = new ZoomEventFilter(this);
190 // event filter 246​ // event filter
191 installEventFilter(this); 247​ installEventFilter(this);
192 248​
193 // set initial cursor 249​ // set initial cursor
194 m_mouseCursor = Qt::IBeamCursor; 250​ m_mouseCursor = Qt::IBeamCursor;
195 setCursor(m_mouseCursor); 251​ setCursor(m_mouseCursor);
196 252​
197 // call mouseMoveEvent also if no mouse button is pressed 253​ // call mouseMoveEvent also if no mouse button is pressed
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Line(s)292#endif
240 qDeleteAll(m_imPreeditRangeChildren); 296​ qDeleteAll(m_imPreeditRangeChildren);
241 297​
242 qDeleteAll(m_inputModes); 298​ qDeleteAll(m_inputModes);
243 299​
244 // delete bracket markers 300​ // delete bracket markers
245 delete m_bm; 301​ delete m_bm;
246 delete m_bmStart; 302​ delete m_bmStart;
247 delete m_bmEnd; 303​ delete m_bmEnd;
304​
305​ delete m_zoomEventFilter;
248} 306​}
249 307​
250void KateViewInternal::prepareForDynWrapChange() 308​void KateViewInternal::prepareForDynWrapChange()
251{ 309​{
252 // Which is the current view line? 310​ // Which is the current view line?
253 m_wrapChangeViewLine = cache()->displayViewLine(m_displayCursor, true); 311​ m_wrapChangeViewLine = cache()->displayViewLine(m_displayCursor, true);
254} 312​}
255 313​
▲ Show 20 Lines • Show All 3035 Lines • ▼ Show 20 Line(s)3347{
3291 m_displayCursor = KTextEditor::Cursor(0, 0); 3349​ m_displayCursor = KTextEditor::Cursor(0, 0);
3292 m_cursor.setPosition(0, 0); 3350​ m_cursor.setPosition(0, 0);
3293 cache()->clear(); 3351​ cache()->clear();
3294 updateView(true); 3352​ updateView(true);
3295} 3353​}
3296 3354​
3297void KateViewInternal::wheelEvent(QWheelEvent *e) 3355​void KateViewInternal::wheelEvent(QWheelEvent *e)
3298{ 3356​{
3299 // ctrl pressed -> change font size (only if angle is reported) 3357​ // check if this event should change the font size (Ctrl pressed, angle reported and not accidentally so)
3300 if (e->modifiers() == Qt::ControlModifier) { 3358​ // Note: if detectZoomingEvent() doesn't unset the ControlModifier we'll get accelerated scrolling.
dhaumannUnsubmitted
Done

const qint64 ...

dhaumann: const qint64 ...
3359​ if (m_zoomEventFilter->detectZoomingEvent(e)) {
3301 if (e->angleDelta().y() > 0) { 3360​ if (e->angleDelta().y() > 0) {
3302 slotIncFontSizes(qreal(e->angleDelta().y()) / QWheelEvent::DefaultDeltasPerStep); 3361​ slotIncFontSizes(qreal(e->angleDelta().y()) / QWheelEvent::DefaultDeltasPerStep);
3303 } else if (e->angleDelta().y() < 0) { 3362​ } else if (e->angleDelta().y() < 0) {
3304 slotDecFontSizes(qreal(-e->angleDelta().y()) / QWheelEvent::DefaultDeltasPerStep); 3363​ slotDecFontSizes(qreal(-e->angleDelta().y()) / QWheelEvent::DefaultDeltasPerStep);
3305 } 3364​ }
3306 3365​
3307 // accept always and be done for zooming 3366​ // accept always and be done for zooming
3308 e->accept(); 3367​ e->accept();
Show All 12 Lines
3321 // handle horizontal scrolling via the scrollbar 3380​ // handle horizontal scrolling via the scrollbar
3322 if (e->orientation() == Qt::Horizontal) { 3381​ if (e->orientation() == Qt::Horizontal) {
3323 // if we have dyn word wrap, we should ignore the scroll events 3382​ // if we have dyn word wrap, we should ignore the scroll events
3324 if (m_view->dynWordWrap()) { 3383​ if (m_view->dynWordWrap()) {
3325 e->accept(); 3384​ e->accept();
3326 return; 3385​ return;
3327 } 3386​ }
3328 3387​
3329 QWheelEvent copy = *e; 3388​ QWheelEvent copy = *e;
dhaumannUnsubmitted
Done

Note: instead of NB (had to look it up)

dhaumann: Note: instead of NB (had to look it up)
3330 QApplication::sendEvent(m_columnScroll, &copy); 3389​ QApplication::sendEvent(m_columnScroll, &copy);
3331 if (copy.isAccepted()) { 3390​ if (copy.isAccepted()) {
3332 e->accept(); 3391​ e->accept();
3333 } 3392​ }
3334 } 3393​ }
3335} 3394​}
3336 3395​
3337void KateViewInternal::startDragScroll() 3396​void KateViewInternal::startDragScroll()
▲ Show 20 Lines • Show All 441 Lines • Show Last 20 Lines