feat: SG-41979: Added env var with a pattern for opening files without forming sequences

[chenlj1]

SG-41979: Add env var with a pattern to prevent RV from loading images as an image sequence

Summarize your change.

Added the ability to set an environment variable called RV_NO_SEQUENCE_SUBSTRING to make all files containing that pattern no longer be able to form a sequence. (i.e. export RV_NO_SEQUENCE_SUBSTRING=".some_pattern.")

Describe the reason for the change.

This change was requested by a client where they wanted a better way to control when a list of file should form a sequence.

Describe what you have tested and on which operating system.

This fix was tested on macOS by setting the environment variable RV_NO_SEQUENCE_SUBSTRING to ".nosequence." (export RV_NO_SEQUENCE_SUBSTRING=".nosequence.") and trying to add more than 5 file with that pattern at once to make sure no sequence would form. (RV was opened using the same terminal used to set the environment variable.)

[cedrik-fuoco-adsk]

I am not sure that I liked adding Qt::Gui dependcy to TwkUtil. Is there a way you could detect the key outside of sequencesInFileList and pass a boolean, or something along those lines? @chenlj1 @bernie-laberge

[chenlj1]

I am not sure that I liked adding Qt::Gui dependcy to TwkUtil. Is there a way you could detect the key outside of sequencesInFileList and pass a boolean, or something in those line? @chenlj1 @bernie-laberge

I will look into this.

[eloisebrosseau]

I am not sure that I liked adding Qt::Gui dependcy to TwkUtil. Is there a way you could detect the key outside of sequencesInFileList and pass a boolean, or something in those line? @chenlj1 @bernie-laberge

I was about to add the same comment as @cedrik-fuoco-adsk. It would be cleaner if we could simply pass the information needed to sequencesInFileList than to add QGuiApplication as this is a utility library.

[eloisebrosseau]

Also @chenlj1, if you don't have it enabled already, I encourage you to add clang-tidy to your IDE by following the Open RV documentation.

[eloisebrosseau]

Was the intent of the environment variable to be an actual pattern like for a regex or simply a substring to match? If it's the latter, I would rename "RV_NO_SEQUENCE_PATTERN" to "RV_NO_SEQUENCE_SUBSTRING" for clarity.

[chenlj1]

I am not sure that I liked adding Qt::Gui dependcy to TwkUtil. Is there a way you could detect the key outside of sequencesInFileList and pass a boolean, or something in those line? @chenlj1 @bernie-laberge

I was about to add the same comment as @cedrik-fuoco-adsk. It would be cleaner if we could simply pass the information needed to sequencesInFileList than to add QGuiApplication as this is a utility library.

@cedrik-fuoco-adsk @eloisebrosseau If I add the flag to the sequencesInFileList, would it be ok to add QGuiApplication to the file that will be calling sequenceInFileList. Or should I find another way to determine whether Alt/Option is held down. I am asking because my proposed fix was to add the flag. Then, in the file that handles the drag and drop, use the QGuiApplication to detect keyboard input to know if we should pass true or false for that newly added flag. Let me know if this is the what you meant and please correct me if this was not the original intention.

[chenlj1]

Nit: You could look into using std::optional instead of your empty string pattern. Something like:

std::optional<std::string> noSequencePattern;
if (auto* pattern = getenv("RV_NO_SEQUENCE_PATTERN"))
{
    noSequencePattern = pattern;
}

But what you did works too.

Also, was the intent of the environment variable to be an actual pattern like for a regex or simply a substring to match? If it's the latter, I would rename "RV_NO_SEQUENCE_PATTERN" to "RV_NO_SEQUENCE_SUBSTRING" for clarity.

You are right, renaming it to "RV_NO_SEQUENCE_SUBSTRING" is in fact better for clarity as I am not actually using regex.

[chenlj1]

@cedrik-fuoco-adsk @eloisebrosseau After discussing with @bernie-laberge, we have decided to remove the alt/option key option. This means we will no longer need the extra dependency in TwkUtil.

[bernie-laberge]

LGTM

[cedrik-fuoco-adsk]

Does #1139 replace this PR ? @chenlj1

[chenlj1]

Does #1139 replace this PR ? @chenlj1

Yes, that new PR will be replacing this one and I will be closing this PR.