Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify string matching code #707

Open
johnhaddon opened this issue Jan 9, 2014 · 2 comments
Open

Unify string matching code #707

johnhaddon opened this issue Jan 9, 2014 · 2 comments
Labels
core Issues with core Gaffer functionality

Comments

@johnhaddon
Copy link
Member

We're performing various different styles of string matching in Gaffer :

  • Python regexes in python registries, with automatic mapping of glob strings to regexes via fnmatch. PlugValueWidget.registerCreator() is one example of this.
  • Custom glob style matching in C++. PathFilter is an example of this.
  • Regexes in C++ with some odd mapping from glob strings via python bindings. Metadata is an example of this.

We should reimplement all such code to use a single Gaffer class intended for the purpose. The basis for this should be the glob matching implemented in PathFilter - although regexes are more powerful, they're also way more confusing for users, and we haven't had a use case for them yet, despite their availability.

@johnhaddon
Copy link
Member Author

Another argument for glob style matching is that we can avoid having to construct objects as we would for regexes - this is very useful in that we can just have a match function taking const char *, and directly pass in the values we get from StringPlug::getValue(). Otherwise we'd be left having to convert to a regex every single time we accessed a plug, which would be a lot, or having to introduce yet another plug type which stores regexes internally, or some other overly complex thing.

In that case I don't think we really want a string matching object - just a function is sufficient, and allows callers to manage their own storage as appropriate.

johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Feb 28, 2014
This is extracted from PathMatcher and will be used as the basis for unifying string matching across Gaffer as required by GafferHQ#707.
johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Mar 10, 2014
This is extracted from PathMatcher and will be used as the basis for unifying string matching across Gaffer as required by GafferHQ#707.
johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Mar 12, 2014
This is extracted from PathMatcher and will be used as the basis for unifying string matching across Gaffer as required by GafferHQ#707.
johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Mar 26, 2014
This is extracted from PathMatcher and will be used as the basis for unifying string matching across Gaffer as required by GafferHQ#707.
johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Apr 1, 2014
This is extracted from PathMatcher and will be used as the basis for unifying string matching across Gaffer as required by GafferHQ#707.
johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Apr 6, 2018
This has been deprecated for almost 3 years, and recent improvements in metadata matching should have rendered it totally unnecessary. This removes one of the main hurdles in completing GafferHQ#707.

Breaking Change :

- Removed `PlugValueWidget.registerCreator()`. Use metadata instead.
@johnhaddon
Copy link
Member Author

johnhaddon commented Apr 6, 2018

I think we're within shooting distance of having this done now. Here's the list of places where we still expose regexes publicly rather than use the syntax from StringAlgo::match() :

  • PlugValueWidget.registerCreator() Fixed in Metadata matching improvements #2536
  • InfoPathFilter and FileNamePathFilter. These are both deprecated already.
  • NodeFinderDialogue. Here we're using fnmatch.translate() so the syntax is practically the same as StringAlgo. This is a trivial change.
  • Layouts. Regex taken as a parameter to save(), which we have a todo to remove anyway.
  • ShaderUI.appendShaders(). We're using some pretty nasty regexes with this, so it might be tricky.
  • ParameterisedHolderUI.appendParameterisedHolders()
  • ConnectionGadget.registerConnectionGadget(). We should remove this and use metadata.
  • View.registerView(). We should remove this and use metadata.

johnhaddon added a commit to johnhaddon/gaffer that referenced this issue Apr 6, 2018
This has been deprecated for almost 3 years, and recent improvements in metadata matching should have rendered it totally unnecessary. This removes one of the main hurdles in completing GafferHQ#707.

Breaking Change :

- Removed `PlugValueWidget.registerCreator()`. Use metadata instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues with core Gaffer functionality
Projects
None yet
Development

No branches or pull requests

2 participants