Refactor KateViewInternal::mouseDoubleClickEvent(QMouseEvent *e)
ClosedPublic

Authored by shubham on Feb 6 2019, 3:35 PM.

Details

Summary

Removed unncessary switch and replaced it with simple if, more efficient

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
shubham created this revision.Feb 6 2019, 3:35 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptFeb 6 2019, 3:35 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
shubham requested review of this revision.Feb 6 2019, 3:35 PM
sars added a subscriber: sars.Feb 6 2019, 9:12 PM
sars added inline comments.
src/view/kateviewinternal.cpp
2702

does this even compile on OSX? Why did you change this?

2718

Where did e->accept() / e->ignore() go? There is a reason they are there...

I am with Kåre here: this change does not fix any issue nor does it improve the current state significantly. In fact, there is a risk of introducing a regression by removing the accept/reject calls.

I don't think this should go in as is.

Shubham, on bugs.kde.org we have more than 300 open bugs. Maybe you can find something there? Would be really nice :)

shubham added a comment.EditedFeb 7 2019, 3:01 AM

@dhaumann I don't think using switch when there is only a single case a good thought. That's what if is for.

src/view/kateviewinternal.cpp
2702

I can't tell, I don't have one. Btw Q_OS_OSX is deprecated.

2718

By mistake i removed those after seeing their presence in switch, I will add those.

shubham added inline comments.Feb 7 2019, 6:01 AM
src/view/kateviewinternal.cpp
2718

Btw no need to explicitly accept() the event, because isAccepted() function returns true by default. But surely, event should be explicitly ignored.

shubham updated this revision to Diff 51102.Feb 7 2019, 2:30 PM

Accept and ignore the event

Is it okay now?

dhaumann accepted this revision.Feb 9 2019, 9:13 PM

I think it's ok, although it is arguably whether this improves anything. Still, let's move on.

This revision is now accepted and ready to land.Feb 9 2019, 9:13 PM
This revision was automatically updated to reflect the committed changes.