Page MenuHome
Differential D8232

Fix T78084: Search does not accept text fragments
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Jul 7 2020, 10:15 AM.

Details

Reviewers
Julian Eisel (Severin)
Campbell Barton (campbellbarton)
Jacques Lucke (JacquesLucke)
Maniphest Tasks
T78084: Search in "Add Node" does not accept text fragments
Commits
rB2a24b3aaf4c8: Fix T78084: Search does not accept text fragments everywhere
Summary

This was reported for the "Add Node" search functionality, but is relevant in other searches as well.

So e.g. when searching for "Separate XYZ", typing "sep", then " " (with the intention to type "X" next) would clear the search field.
Now use the same method (matching against all search words) as in F3 searching ('menu_search_update_fn') in other searches as well [ID search template, objects as properties, finding nodes,...]

This should give a much nicer search experience in general.
Note: this doenst touch other searches in the Dopesheet, Outliner, Filebrowser or User Preferences that have other search implementations.

Diff Detail

Repository
rB Blender
Branch
T78084 (branched from master)
Build Status
Buildable 9942
Build 9942: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Jul 7 2020, 10:15 AM
Harbormaster completed remote builds in B8879: Diff 26579.Jul 7 2020, 10:15 AM
Philipp Oeser (lichtwerk) requested review of this revision.Jul 7 2020, 10:15 AM
Philipp Oeser (lichtwerk) added projects: User Interface, Nodes.Jul 7 2020, 10:16 AM
Jacques Lucke (JacquesLucke) accepted this revision.Jul 7 2020, 12:42 PM
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/editors/interface/interface.c
6623

Took me a while to understand what this is checking exactly. Maybe this change makes it easier to understand.

const bool all_words_matched = (index == words_len);
if (all_words_matched) { ... }
This revision is now accepted and ready to land.Jul 7 2020, 12:42 PM
RedMser (RedMser) awarded a token.Jul 7 2020, 8:39 PM
Campbell Barton (campbellbarton) accepted this revision.Jul 8 2020, 10:29 AM

This could mention the same method is used in menu_search_update_fn, so any changes are made in both places.

Julian Eisel (Severin) accepted this revision.Jul 8 2020, 10:36 AM

Assuming above's points were addressed, looks good.

Philipp Oeser (lichtwerk) added a comment.Jul 8 2020, 10:55 AM

Just talked to @Campbell Barton (campbellbarton) and we agreed this could also be good for ID search, will add that in a bit as well...

Philipp Oeser (lichtwerk) updated this revision to Diff 26626.Jul 8 2020, 3:36 PM

also use this multi-word method in other places [ID search template, objects as properties, node finding operator] and make this a general function

Harbormaster completed remote builds in B8904: Diff 26626.Jul 8 2020, 3:36 PM
Philipp Oeser (lichtwerk) retitled this revision from Fix T78084: Search in enums does not accept text fragments to Fix T78084: Search does not accept text fragments.Jul 8 2020, 3:39 PM
Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

Since I made this a bit more general and used in more places, would appreciate a second review pass, thx in advance!

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Jul 8 2020, 5:48 PM
Jacques Lucke (JacquesLucke) added inline comments.Jul 8 2020, 5:55 PM
source/blender/blenlib/intern/string.c
527

You might want to add unit tests in BLI_string_test.cc.

source/blender/editors/interface/interface.c
6614

Since you are refactoring now, might be nice to extract this (str_len / 2) + 1 expression into a function like uint BLI_string_max_possible_word_count(const uint len).

ogierm awarded a token.Jul 8 2020, 6:39 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 26711.Jul 10 2020, 7:37 PM

from review: make max word count a function

Harbormaster completed remote builds in B8961: Diff 26711.Jul 10 2020, 7:38 PM
Philipp Oeser (lichtwerk) marked 2 inline comments as done.Jul 10 2020, 7:39 PM
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).Aug 12 2020, 6:11 AM

If you commit this I can use it for property search : )

It may make sense to look at the use of uint again given the section recently added to the style guide: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Integer_Types

Philipp Oeser (lichtwerk) added a comment.Aug 12 2020, 9:50 AM
In D8232#208182, @Hans Goudey (HooglyBoogly) wrote:

If you commit this I can use it for property search : )

It may make sense to look at the use of uint again given the section recently added to the style guide: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Integer_Types

on my list for today

Philipp Oeser (lichtwerk) added a comment.Aug 12 2020, 5:19 PM
In D8232#208204, @Philipp Oeser (lichtwerk) wrote:

on my list for today

make this tomorrow, sorry

Hans Goudey (HooglyBoogly) added a comment.Aug 12 2020, 5:26 PM

No hurry at all! I just saw it and thought it might be useful

Hans Goudey (HooglyBoogly) mentioned this in T77824: Properties Search Implementation.Aug 20 2020, 9:57 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 28459.Sep 2 2020, 6:14 PM

rebase on master

Harbormaster completed remote builds in B9941: Diff 28459.Sep 2 2020, 6:14 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 28462.Sep 2 2020, 6:50 PM

respect Integer Types Style Guide

Harbormaster completed remote builds in B9942: Diff 28462.Sep 2 2020, 6:50 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 28498.Sep 3 2020, 4:49 PM

added a test for BLI_string_max_possible_word_count

Harbormaster completed remote builds in B9963: Diff 28498.Sep 3 2020, 4:49 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 28508.Sep 3 2020, 6:34 PM

correct test

Harbormaster completed remote builds in B9967: Diff 28508.Sep 3 2020, 6:34 PM
Philipp Oeser (lichtwerk) marked an inline comment as done.Sep 3 2020, 6:39 PM

So this has seen some (minor) adjustments based on @Jacques Lucke (JacquesLucke) and @Hans Goudey (HooglyBoogly) feedback.
Just to make sure: could you share your pair of eyes another time for final green light? thx in advance!

Jacques Lucke (JacquesLucke) accepted this revision.Sep 4 2020, 9:56 AM
Closed by commit rB2a24b3aaf4c8: Fix T78084: Search does not accept text fragments everywhere (authored by Philipp Oeser (lichtwerk)). · Explain WhySep 4 2020, 10:13 AM
This revision was automatically updated to reflect the committed changes.
Philipp Oeser (lichtwerk) added a commit: rB2a24b3aaf4c8: Fix T78084: Search does not accept text fragments everywhere.

Revision Contents
Changeset List

PathSize
source/
blender/
blenlib/
7 lines
intern/
33 lines
editors/
animation/
2 lines
interface/
9 lines
3 lines
16 lines
15 lines
9 lines
22 lines
space_node/
11 lines

Diff 28462

View Options

source/blender/blenlib/BLI_string.h

Loading...
View Options

source/blender/blenlib/intern/string.c

Loading...
View Options

source/blender/editors/animation/anim_filter.c

Loading...
View Options

source/blender/editors/interface/interface.c

Loading...
View Options

source/blender/editors/interface/interface_intern.h

Loading...
View Options

source/blender/editors/interface/interface_template_search_menu.c

Loading...
View Options

source/blender/editors/interface/interface_template_search_operator.c

Loading...
View Options

source/blender/editors/interface/interface_templates.c

Loading...
View Options

source/blender/editors/interface/interface_utils.c

Loading...
View Options

source/blender/editors/space_node/node_select.c

Loading...