More Search UI/UX Enhancements#7612
Conversation
JohnAFernandez
added
the
enhancement
MjnMixael
left a comment
MjnMixael
left a comment
There was a problem hiding this comment.
Minor things. Looks overall pretty good.
| void sexp_tree_view::filterOperatorPopup(const QString& text) | ||
| { | ||
| _opList->clear(); | ||
| SCP_set<QString> found_list; |
There was a problem hiding this comment.
Unused variable? Alternatively if you used it for what it seems like was intended you can change this whole lookup from O(n2) into just O(n), I think.
for (const auto& s : _opAll) {
if (s.startsWith(text, Qt::CaseInsensitive)) {
_opList->addItem(s);
found_list.insert(s);
}
}
for (const auto& s : _opAll) {
if (s.contains(text, Qt::CaseInsensitive) && found_list.count(s) == 0)
_opList->addItem(s);
}
There was a problem hiding this comment.
Thanks for pointing this out. Again, I think my mental state was a little frazzled. Not that we need a ton of speed here, but I added the check before comparing the text.
|
|
||
| // find the best operator | ||
| int best = sexp_match_closest_operator(str, opf); | ||
| int best = sexp_match_closest_operator(str, opf, 10); |
There was a problem hiding this comment.
Should this magic number be a constant? Personally I'd put it at the top of the file with TREE_NODE_INCREMENT and a comment explaining it.
There was a problem hiding this comment.
Definitely, I think my ADHD was flaring when writing this.
|
|
||
| item_index = get_node(item); | ||
| // No longer gated to just certain types. | ||
| openNodeEditor(currentItem()); |
There was a problem hiding this comment.
Could pass item here as you've already got currentItem()
|
|
||
| // This keyboard shortcut does not need as much state checking because the helper | ||
| // handles security checks and what to call, and any node should work here. | ||
| auto* shortcut = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_S), this); |
There was a problem hiding this comment.
Just a question.. did you verify this doesn't fall through to the editor and call a save action? I don't think it will but just want to be sure.
There was a problem hiding this comment.
It still saves the mission after the dialog is closed.
github-project-automation
Bot
moved this from Work In Progress (PRs)
to In Review (PRs)
in qtFRED2
Items that begin with the search string should be prioritized before those that simply contain the search string.
I didn't see any crashes, but this matches the pattern of other keyboard shortcuts.
The working code existed already
This limits matches on operator searches by setting a minimum size that a string match has to meet. Function returns -1 on failure, and the warning has been removed to allow this behavior.
This allows ctrl+s (which can be listed instead of spacebar) to initiate search. There is a new menu item added as well.
d46fd2a to
a26667e
Compare
We want the item to be inserted here, not ignored.
2 participants
This has a few upgrades to the sexp search system, including:
Each feature was individually tested, and I'm open to feedback on adjustments.
Fixes #7604