Allow : in --data-values-file path

[cari-lynn]

UPDATE 3/23:

What

Prior to this PR we were using just : to detect a library reference, but this PR introduces using a prefix of @, and suffix of : to detect it in all flags. This doesn't change any behavior since this syntax was already required. This change avoids ytt confusing the : that can come from a prefix of @libraryReference:, and a : that comes from a file path.

How

These flags are unaffected by this change:

--data-values-env
--data-values-env-yaml
--data-value
--data-value-yaml
--data-value-file

These flags have a flag value in the format @lib:key=value, which is why they are not changed, there is no change needed to allow : in the value because it is not currently ambiguous.

This is the only flag affected: --data-values-file. This is affected because it does not have a key and it is ambiguous where a library reference and a file path value are separated.

It is in the format @lib:value. : can appear in the @lib: and anywhere in the value. So this PR makes a change to allow the : to appear anywhere in the value.

In a case breakdown for --data-values-file is:

• case 1: No library reference. Examples: “path.yml”, “@path.yml”, “c:path.yml”, “:::::path:::::.yml”, “pa@lib:th.yml”
• case 2: Library reference. Examples: “@~libref:path.yml”, “@lib@lib1:”, “@lib:@lib1:path.yml <- o.o
• case 3: Error. Examples: “@:path.yml”, “@:”

[gcheadle-vmware]

This change set took a while for me to digest, and honestly the test is difficult to attempt manually without a windows machine. The implementation was surprising to me since the issue mentions the libraryKeySep, but that const is not altered. After some time, I was able to build a mental model for the switch statement:

• case 1: "path.yml" - no libref case
• case 2: "@libref:path.yml" - normal case, so confirm by checking prefix (@)
• case 2: "c://Users/path.yml" - windows case, return back full string since no prefix (@)
• case 3: "@lib:c://Users/path.yml" - windows case, repair unintentional split on full path

These cases make sense to me and the implementation tracks, and even though there are many ways to supply data values via flags, I think your change will catch all cases.

I do wonder if a Mac user that has multiple ":" in an argument, like trying to (incorrectly) ref two libraries with a single data value file: @lib:@otherLib:path. I believe this arg would return a path like "@otherlib:path" for the file location, instead of reporting an err (err would happen when attempting to read file from system). 🤔

[gcheadle-vmware]

@cari-lynn, I took another look at these changes, and I think they look great. By improving the "Unable to read file error" you are improving the ytt experience across operating systems. I also appreciate how you've done research to inform the decisions being made.

[pivotaljohn]

Left a couple of naming suggestions.

Otherwise, looks great!

[gcheadle-vmware]

LTGM!