This revision was automatically updated to reflect the committed changes.
Closed by commit R39:b9a52834ec45: Refactor
KateViewInternal::mouseDoubleClickEvent(QMouseEvent *e) (authored by shubham).
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18788?vs=5110
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
I think it's ok, although it is arguably whether this improves anything.
Still, let's move on.
REPOSITORY
R39 KTextEditor
BRANCH
arcpatch-D18788
REVISION DETAIL
https://phabrica
shubham added a comment.
Is it okay now?
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D18788
To: shubham, cullmann
Cc: dhaumann, sars, kwrite-devel, kde-frameworks-devel, michaelh, ngraham,
bruns, demsking, cullmann
shubham updated this revision to Diff 51102.
shubham added a comment.
Accept and ignore the event
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D18788?vs=51039&id=51102
BRANCH
arcpatch-D18788
REVISION DETAIL
https://phabricator.kde.org/D18788
AFFE
shubham added inline comments.
INLINE COMMENTS
> sars wrote in kateviewinternal.cpp:2718
> Where did e->accept() / e->ignore() go? There is a reason they are there...
Btw no need to explicitly accept() the event, because isAccepted() function
returns true by default. But surely, event should be
shubham added a comment.
@dhaumann I don't think using switch when there is only a single case good
thought
INLINE COMMENTS
> sars wrote in kateviewinternal.cpp:2702
> does this even compile on OSX? Why did you change this?
I can't tell, I don't have one. Btw Q_OS_OSX is deprecated.
> sar
dhaumann added a comment.
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.kd
sars added inline comments.
INLINE COMMENTS
> kateviewinternal.cpp:2702
> // Move cursor to end (or beginning) of selected word
> +#ifndef Q_OS_MACOS
> if (view()->selection()) {
does this even compile on OSX? Why did you change this?
> kateviewinternal.cpp:2718
> -
> -
shubham created this revision.
shubham added a reviewer: cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
shubham requested review of this revision.
REVISION SUMMARY
Removed unncessary switch and replaced it with simple if, more eff
