Differential D19408

[RFC] Avoid deadlock in use of vi mode searchbar
Needs ReviewPublic
Actions

Authored by loh.tar on Feb 28 2019, 4:45 AM.

Details

Reviewers
None
Group Reviewers
KTextEditor
Summary

BUG:368130

Test Plan

I'm not a vi mode user, so I can't say what is to be expect.
I guess after press Ctrl-F again they should disappear. Right now is the search field cleared.
It would be nice someone take over this patch and finish it or give useful hints how to fix that further, if needed.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.Feb 28 2019, 4:45 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptFeb 28 2019, 4:45 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Feb 28 2019, 4:45 AM
dhaumann added subscribers: cullmann, dhaumann.Mar 12 2019, 9:14 PM

Hm, I currently cannot say anything to this patch. I wonder where the \r comes from. And also, I wonder whether this if now is ever true? ...but if it fixes the issue... @cullmann: objections to a blind commit?

loh.tar added a comment.Mar 13 2019, 5:26 AM
  • Assume you add "qDebug() << keyEvent;" on top of this function
  • For whatever reason is by Ctrl-F this code running with

KateVi::Completer::completerHandledKeypress: QKeyEvent(KeyPress, Key_Enter)

  • When you you hint return it looks like this...

KateVi::Completer::completerHandledKeypress: QKeyEvent(KeyPress, Key_Return, text="\r")

  • ...and the search bar disappears now
cullmann added a comment.Mar 13 2019, 1:34 PM

I am unsure about the toplevel &&, wouldn't one just want that for the return case?

loh.tar added a comment.Mar 13 2019, 1:52 PM

hm, it may better to avoid to call that function at all in case of Ctrl-F (?) Will take a look.

loh.tar added a comment.Mar 13 2019, 7:36 PM

OK. I spend some time again, but give up now.
I've got the feeling that these key event handling is not very well engineered.
May someone else take a closer look for some better solution, or at least try this patch, if it breaks some other functionality.

Revision Contents
Changeset List

Diff 52791

View Options

src/vimode/emulatedcommandbar/completer.cpp

Loading...