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

Add regex support in Auto Color for track, marker, region name filters #1291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daleonov
Copy link

Exact matching didn't really work for track names for me. So I added regex support to define a pattern, and keywords like "gtr", "bass", "synth" won't conflict with "lead", "rhythm", "double" etc and each combo would be able to have its own color.
sws regex

@daleonov
Copy link
Author

I can also look into #1257 since it's about similar feature.

@AppVeyorBot
Copy link

Build sws 202-master completed (commit cb66149b07 by @daleonov)

Environment: APPVEYOR_BUILD_WORKER_IMAGE=Ubuntu1804, ARCH=aarch64
Environment: APPVEYOR_BUILD_WORKER_IMAGE=Ubuntu1804, ARCH=i686
Environment: APPVEYOR_BUILD_WORKER_IMAGE=Ubuntu1804, ARCH=armv7l
Environment: APPVEYOR_BUILD_WORKER_IMAGE=Ubuntu1804, ARCH=x86_64
Environment: APPVEYOR_BUILD_WORKER_IMAGE=Visual Studio 2019, ARCH=x86
Environment: APPVEYOR_BUILD_WORKER_IMAGE=Visual Studio 2019, ARCH=x64

@nofishonfriday
Copy link
Collaborator

nofishonfriday commented Jan 17, 2020

I was unaware of this either until shortly ago but since SWS targets macOS 10.5. the C++11 regex features are not available for the Mac version, so we can't use them unfortunately (afaik).
https://travis-ci.com/reaper-oss/sws/jobs/276788893#L546

edit:
More comprehensive explaination here.

edit2;
Sorry, this should have been mentioned in the Submitting pull requests section of the Wiki. I'll add that.

@cfillion
Copy link
Member

cfillion commented Jan 17, 2020

In addition to the above, the std::regex constructor throws an std::regex_error exception when the input is invalid. Since it comes from users, the possibility of an invalid regex should be handled without crashing.

Currently it bricks the REAPER install by making it crash on startup until the bad regex is manually removed from sws-autocoloricon.ini.

I find the $ prefix somewhat confusing, as it is usually the end of string/line anchor. Perhaps surrounding the regexes with slashes would make it clearer? It is a very widely used delimiter for regexes.

Additional concerns I'm not sure about (needs testing/research):

  • Cost of compiling regexes over and over with lots of tracks (vs storing a compiled std::regex and reusing it)
  • Possible hangs with user input accidentally resulting in overly complex regexes?
  • What are the advantages of using the POSIX extended syntax vs the default ECMAScript?
  • Wouldn't the collate option introduce behavior differences from an install to the next depending on the current system locale?

#1257 (and the related #1119) could be done by reusing ReaPack's filter implementation (but with its use of the C++17 stdlib removed) as discussed here.

@daleonov
Copy link
Author

daleonov commented Jan 17, 2020

the C++11 regex features are not available for the Mac version, so we can't use them unfortunately (afaik).

Can't use them at all or still can use as a platform-specific feature? The code is easy to isolate so I can do something like

#ifndef GRANDPA_TECH
do_regex();
#endif 

...if such thing is acceptable here, of course.
Other way is using third-party library, but I don't think it's welcome here unless extremely necessary?

the std::regex constructor throws an std::regex_error exception when the input is invalid

Fair enough. No elegant way to address it other than catching it? Good news is std::regex_error is easy to isolate.

Perhaps surrounding the regexes with slashes would make it clearer?

Good idea!

Cost of compiling regexes over and over with lots of tracks (vs storing a compiled std::regex and reusing it)

Fair enough. It's a lower priority concern than the rest of those. But if I go with regex exception handling (as I discussed above) then I'll have one more reason to move it outside the loop.

Possible hangs with user input accidentally resulting in overly complex regexes?

Possible... But it's their DAW, right? I think it's up to them to decide how complex is too complex :)

What are the advantages of using the POSIX extended syntax vs the default ECMAScript?

There was something I didn't like, so I always used the extended one. Don't remember exactly, I'll check later. As long as it can do what's on screenshot in first post, I'm happy.

Wouldn't the collate option introduce behavior differences from an install to the next depending on the current system locale?

And I think it should. What I had in mind is stuff like \w including user's alphabet. I haven't tested it yet but I assumed it's what this flag does.

Also, I've hardcoded it to be case insensitive, not quite happy with it. I don't recall anything in actual regex syntax to specify case sensitivity, and I don't see any elegant way to specify it in the sws enviroment. Other than adding another meta language thing to filter string, but that's clunky too.


Okay, so let's first decide if it's possible at all to add this feature (since there are some macOS compatibility issues) and if it is, I'll proceed with addressing the points above. Hopefully I won't have to fork it just for that little feature :)

@nofishonfriday nofishonfriday mentioned this pull request Jan 19, 2020
@cfillion
Copy link
Member

cfillion commented Jan 25, 2020

Can't use them at all or still can use as a platform-specific feature?

It wouldn't be the first platform-specific feature in SWS (there are some Windows-only ones). It would be a shame to have this everywhere except on macOS though...

Other way is using third-party library, but I don't think it's welcome here unless extremely necessary?

Pretty much. Regexes in auto color would be a pretty niche feature so I don't think it's enough to warrant bundling an entire regex library. From what I could find (ignoring compatibility concerns regarding the regex flavor, which would have to be addressed), Apple's Foundation framework added regexes in v10.7 and libpcre is bundled with the OS since 10.6. 10.5 doesn't seem to ship with one.

No elegant way to address it other than catching it?

That's correct.

Possible... But it's their DAW, right?

If they can accidentally brick their DAW (eg. freeze at startup), it might become an issue. (I haven't looked into it so it might be very hard/impossible to so non-intentionally. In which case this is a non-issue.)

And I think it should. What I had in mind is stuff like \w including user's alphabet.

That would cause issues when using an existing config one a computer that happens to have a different locale than the one it was tested on (eg. cool Auto Color regex tips shared on the forum mysteriously doesn't work right, portable install on a thumb drive appears broken at the studio's PC...).

I don't recall anything in actual regex syntax to specify case sensitivity, and I don't see any elegant way to specify it in the sws enviroment.

How about /regex string/i for a case-insensitive option? Since it's a common syntax used in Perl, JavaScript, Ruby, PHP...


Alternatively, instead of regular expressions, perhaps using a REAPER Action List-like syntax would be helpful in Auto Color? It defaults to AND matching, has OR/NOT operators, grouping with parentheses, full-word/exact matching using quotes and (in ReaPack's implementation) start/end of string anchoring with ^ and $.

It's not a very useful alternative with patterns such as V[0-9] (becoming the very ugly V0 OR V1 OR V2 OR...), but things like (GTR|guitar).*lead can be written as ( GTR OR guitar ) lead (except it allows lead to appear anywhere including before GTR).

@daleonov
Copy link
Author

Using simpler syntax would be great! I agree, regex is a bit too geeky even for reaper users.
But parsing it without regex and not being able to use regex under the hood would be one hell of a chore... Is something similar already implemented somewhere in sws?

@cfillion
Copy link
Member

cfillion commented Jan 27, 2020

In that case this PR, along with #1119 (where it was originally suggested), #1257 and #1285 could all be solved by adding ReaPack's implementation of REAPER's filter syntax into SWS (with minor tweaks to remove uses of std::unique_ptr and boost::algorithm::to_lower) .

@cfillion cfillion mentioned this pull request Dec 16, 2021
@odedd
Copy link

odedd commented Jul 18, 2022

Any chance of getting this thing to work? This could be a really powerful addition

@sychron
Copy link

sychron commented Apr 13, 2023

Unfortunately, my C++ is not good enaugh to resolve the conflicts.

But I would really like to have this feature, as it's very handy if you have track names that have the description in the front and a labelling in the back, for example: "FX: A", "FX: B" etc. With Regex, one could assign track icons like FX:A, FX:B, ignoring the role name.
So ... please count this as an upvote for this request :)

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.

6 participants