> On Mai 23, 2015, 11:21 nachm., Mark Gaiser wrote: > > krunner/view.h, line 48 > > <https://git.reviewboard.kde.org/r/123888/diff/1/?file=370580#file370580line48> > > > > const QStringList &history() const; > > > > Saves a copy and prevents users from manipulating this object. > > > > You do manipulate this, but that in the QML and you put it in a "var > > history" which probably copies it also in a javascript object? > > > > Or i'm wrong... In that case you can ignore this comment :)
QML cannot track changes inside JS objects, so I need to do an explicit assignment for it to notice hence the indirection. > On Mai 23, 2015, 11:21 nachm., Mark Gaiser wrote: > > lookandfeel/contents/runcommand/RunCommand.qml, lines 98-102 > > <https://git.reviewboard.kde.org/r/123888/diff/1/?file=370582#file370582line98> > > > > Hmm, this looks interesting. > > Suppose my history looks like this: > > - plasmashell > > - konsole > > - dolphin > > > > now imagine i type "plasma". It should then show: > > - plasmashell > > > > Thus far it probably works fine. > > Now imagine i clear whatever i typed (so a clean inputfield). All > > without closing krunner! When i clear it the history probably shows one > > entry: > > - plasmashell > > > > What happened to the other two? > > Well, you've rewritten history when i typed a search query. > > And because you call "runnerWindow.history = history" it will call the > > C++ setHistory method which will overwrite your history value in the > > config.. Ouch! > > > > I could be wrong, but this is how i think it behaves when reading the > > code. > > I think you should use something with a tree. A radix tree would be the > > easiest, fastest and cheapest in memory, but that doesn't exist in Qt nor > > the C++ STL. I don't get it. I only ever write to the history when you actually invoke a search result. On Mai 23, 2015, 11:21 nachm., Kai Uwe Broulik wrote: > > I'm curious (since i don't see that code here). Where do you add items to > > your history? "The unshift() method adds new items to the beginning of an array" - Kai Uwe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123888/#review80757 ----------------------------------------------------------- On Mai 23, 2015, 10:01 nachm., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123888/ > ----------------------------------------------------------- > > (Updated Mai 23, 2015, 10:01 nachm.) > > > Review request for Plasma, KDE Usability and Vishesh Handa. > > > Bugs: 335731 > https://bugs.kde.org/show_bug.cgi?id=335731 > > > Repository: plasma-workspace > > > Description > ------- > > This turns KRunner's TextField into an editable ComboBox to provide a history. > > When a result is invoked, the query string is prepended to the history, query > strings are only added once. ComboBox provides letter-by-letter auto > completion. > > > Diffs > ----- > > krunner/view.h 1ad5075 > krunner/view.cpp 8640e1d > lookandfeel/contents/runcommand/RunCommand.qml 4c6eb30 > > Diff: https://git.reviewboard.kde.org/r/123888/diff/ > > > Testing > ------- > > Somehow I have a feeling it doesn't always save the history or nukes it at > times. It also has some shortcomings due to ComboBox: > > 1.) You cannot use the arrow keys to cycle between entries (when the popup's > not opened) because arrow keys navigate through results > 2.) forceActiveFocus() on the ComboBox will not activate the embedded > TextField - when you had opened the popup there's a slight chance the input > field won't get focussed I'll prepare a Qt patch for this. > 3.) Before Qt 5.4.2 (not sure if my patch ended up in 5.4.1) pressing space > in the edit combobox will open the popup, not insert a space (nasty show > stopper) > 4.) Plasma's edtiable ComboBox looks a bit strange imho > 5.) Plasma's editable ComboBox doesn't support clearButtonShown > 6.) Plasma's ComboBox has strange bullets and margins in it, that's probably > a bug in Plasma Style (need to look what Desktop style does differently from > us) > 7.) ComboBox doesn't have a cursorPosition, I'll prepare a Qt patch for this. > > > File Attachments > ---------------- > > History popup > > https://git.reviewboard.kde.org/media/uploaded/files/2015/05/23/7ad7e5eb-4874-4f9f-9796-738fa2ac9ed5__krunnerhistory.png > Auto completion > > https://git.reviewboard.kde.org/media/uploaded/files/2015/05/23/18714844-ef28-4cdd-af00-e6685caece9b__krunnerautocompletion.png > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
