> On mei 23, 2015, 11:21 p.m., 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. > > Kai Uwe Broulik wrote: > I don't get it. I only ever write to the history when you actually invoke > a search result. > > Kai Uwe Broulik wrote: > Though I could probably just do an addToHistory(cons QString &) mthod and > deal with that stuff on the C++ side, without this weird JS dance. Also, I > forgot to enforce a limit on the number of history items.
Ahh, looks like i was wrong. Sorry for the noise there. But the addToHistory method looks much better indeed, much more readable imho. On mei 23, 2015, 11:21 p.m., Kai Uwe Broulik wrote: > > I'm curious (since i don't see that code here). Where do you add items to > > your history? > > Kai Uwe Broulik wrote: > "The unshift() method adds new items to the beginning of an array" unshift.. right, forgot that it was adding an item to the beginning. That function name is so weird.. prepend(...) would have been a lot more intuitive. Weird JavaScript. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123888/#review80757 ----------------------------------------------------------- On mei 23, 2015, 11:40 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123888/ > ----------------------------------------------------------- > > (Updated mei 23, 2015, 11:40 p.m.) > > > 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
