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

Support colon in set expressions #2070

Merged

Conversation

mattigruener
Copy link
Contributor

The names of our sets often contain colons - this should fix parser errors we were seeing. I have also added support for colons to object names.

I guess I have two questions about this:

  1. What other special characters do we want to support?
  2. How do we want to handle set and object names that contain our set expression operators?

Maybe those two remaining questions boil down to: Do we want object names in gaffer to be more restricted than they currently are? "sphere*&|" is a perfectly valid name at the moment, but it feels like we should only allow [_a-zA-Z0-9][_a-zA-Z0-9:]* maybe? Having a convention like that would make it easier to keep the parser code in sync, I guess...

@johnhaddon
Copy link
Member

johnhaddon commented May 2, 2017

Thanks Matti,

You might be right that we should be more restrictive in what names we allow to appear in the scene hierarchy, but I think [a-zA-Z0-9:] is probably too restrictive. Minimally we need to bear in mind support for languages other than English, and also what names are allowed in common cache formats and other DCC apps. The core of GafferScene really can support anything because it's just using vector<InternedString>, and treats each path component as just an arbitrary token. The problems really only appear in things like PathMatcher and SetAlgo where we allow paths to be defined as '/' separated strings, and assign special meaning to certain characters. We already support escape characters in StringAlgo::match(), which goes some way to solving the ambiguity ("\*" is a literal '*', not a wildcard for instance), and it could be that using escape characters in SetAlgo and ScenePlug::stringToPath() actually allows us to continue supporting totally general names in the core. It might be nice to keep that generality rather than one day find we've accidentally excluded a particular cache format which is more liberal.

We actually have a Cortex issue open about this already, so I suggest we continue the discussion there. In the meantime I'll merge this...

Cheers...
John

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants