feat: add improved parsing for labels and support for multiple fallbacks#4977
feat: add improved parsing for labels and support for multiple fallbacks#4977robertcoltheart wants to merge 13 commits into
Conversation
robertcoltheart
force-pushed
the
feature/improved-regex-labelling
branch
from
cf6ffb9 to
56ec6f8
Compare
|
hey @robertcoltheart, do you think you can get this finalized and ready for review in soon? I'd want to prepare a new release, and I could wait to get this in. |
|
Yes this is basically done now. Just running final tests and will mark it ready hopefully today. |
|
@robertcoltheart make sure to rebase onto main |
robertcoltheart
changed the title
robertcoltheart
force-pushed
the
feature/improved-regex-labelling
branch
from
9cfca75 to
8663be0
Compare
|
@arturcic Waiting on tests to pass now, but this is ready for a review |
|
@HHobeck could would have a look on this one? |
asbjornu
left a comment
asbjornu
left a comment
There was a problem hiding this comment.
This is really great stuff! I especially like the increased test coverage as well as the clarity this brings to the parsing algorithm (moving logic from the Regex and into the LabelTokenizer). Good job!
| Should.Throw<ArgumentException>(() => | ||
| effectiveConfiguration.GetBranchSpecificLabel(ReferenceName.FromBranchName(BranchName), null, environment)); | ||
| var actual = effectiveConfiguration.GetBranchSpecificLabel(ReferenceName.FromBranchName(BranchName), null, environment); | ||
| actual.ShouldBe("pr-"); |
There was a problem hiding this comment.
This change in behavior can be considered to be a breaking change. Perhaps not enough to warrant a major version bump, but worth mentioning in BREAKING_CHANGES.md? Otherwise we should consider supporting the previous behavior.
There was a problem hiding this comment.
Added a comment to breaking changes
There was a problem hiding this comment.
Hmm. But when you want to have a fallback why do you not specify {env:MISSING_VAR ?? ""}? Don't understand the reason why this behavior change.
There was a problem hiding this comment.
Pull request overview
This PR updates GitVersion’s custom string/label formatting to support multi-step fallback chains (e.g. {env:ENV_VAR ?? Prop ?? "fallback"}) by replacing the previous strict regex-based token parsing with a dedicated tokenizer/parser. This is aimed at fixing label formatting scenarios like {BranchName:c} (Issue #4976) and enabling more expressive fallback behavior across environment variables, properties, and literals.
Changes:
- Replace the previous
ExpandTokensRegex-driven parsing with aLabelTokenizer-based parser that supports chained??fallbacks and quoted literals. - Adjust placeholder resolution to avoid throwing on missing properties (and update tests to reflect new behavior).
- Update docs/tests to cover the new fallback syntax and stricter malformed-token handling.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/GitVersion.Core/Formatting/StringFormatWithExtension.cs | Switches evaluation to tokenizer-driven parsing and implements chained fallback evaluation + formatting. |
| src/GitVersion.Core/Formatting/MemberResolver.cs | Changes missing-member behavior from throwing to returning an empty path to support fallback chains. |
| src/GitVersion.Core/Formatting/LabelTokenType.cs | Adds token-type enum for the new parser. |
| src/GitVersion.Core/Formatting/LabelTokenizer.cs | Implements the new parser/tokenizer for label expressions and separators. |
| src/GitVersion.Core/Formatting/LabelToken.cs | Adds a token model used by the tokenizer/evaluator. |
| src/GitVersion.Core/Extensions/ConfigurationExtensions.cs | Updates branch regex placeholder extraction (group name handling). |
| src/GitVersion.Core/Core/RegexPatterns.cs | Broadens the token-matching regex to hand off parsing/validation to the tokenizer. |
| src/GitVersion.Core.Tests/Formatting/LabelTokenizerTests.cs | Adds unit tests for token parsing scenarios (literals, identifiers, malformed inputs). |
| src/GitVersion.Core.Tests/Extensions/StringFormatWithExtensionTests.cs | Expands tests to cover chained fallbacks and updated malformed-token behavior. |
| src/GitVersion.Configuration.Tests/Configuration/ConfigurationExtensionsTests.cs | Updates expectations for missing env-var placeholder behavior in branch labels. |
| docs/input/docs/reference/mdsource/configuration.source.md | Documents cascading fallbacks in label/environment placeholders. |
|
@robertcoltheart could you please rebase once again onto |
robertcoltheart
force-pushed
the
feature/improved-regex-labelling
branch
from
f885834 to
2c308a8
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
arturcic
force-pushed
the
feature/improved-regex-labelling
branch
from
2c308a8 to
0aa9be8
Compare
I had one more comment on that. |
@robertcoltheart can you check this one? |
arturcic
force-pushed
the
feature/improved-regex-labelling
branch
2 times, most recently
from
174ffe8 to
93b8ed5
Compare
arturcic
force-pushed
the
feature/improved-regex-labelling
branch
from
93b8ed5 to
8a0f8d5
Compare
|
|
@robertcoltheart I rebased and fixed some small conflict, could you please check with @HHobeck what needs to be done to have this PR complete? |
Description
Adds advanced label parsing to support multiple fallbacks.
Related Issue
Resolves #4976
Motivation and Context
This supports multiple fallbacks eg
{env:ENV_VAR ?? Prop ?? "fallback"}which wasn't possible with the existing regex parsing. Any combination or number of properties, environment variables or literal values are supported. Property values can be considered as literals if they appear last in the chain.There are some changes in behavior as below:
{Prop1 ?? Prop2}will yieldProp2if neither property is found.???as a separator) whereas previously it would have just shown the unparsed syntax in the output. This is because we can't reliably determine which output to use, and using a manual parser means we can be stricter on the format than when using regex.How Has This Been Tested?
Additional unit tests added for refactored parsing, and testing manually.
Screenshots (if appropriate):
Checklist: