Skip to content

Commit 0c05d92

Browse files
Copilotarturcic
andcommitted
Remove legacy forward slash syntax support from SpectreArgumentParser and update tests to use POSIX syntax
Co-authored-by: arturcic <[email protected]>
1 parent 42b74b1 commit 0c05d92

File tree

2 files changed

+27
-180
lines changed

2 files changed

+27
-180
lines changed

src/GitVersion.App.Tests/ArgumentParserTests.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ public void WrongNumberOfArgumentsShouldThrow()
220220
}
221221

222222
[TestCase("targetDirectoryPath -x logFilePath")]
223-
[TestCase("/invalid-argument")]
223+
[TestCase("--invalid-argument")]
224224
public void UnknownArgumentsShouldThrow(string arguments)
225225
{
226226
var exception = Assert.Throws<WarningException>(() => this.argumentParser.ParseArguments(arguments));
@@ -370,14 +370,14 @@ public void UpdateAssemblyInfoWithRelativeFilename()
370370
[Test]
371371
public void OverrideconfigWithNoOptions()
372372
{
373-
var arguments = this.argumentParser.ParseArguments("/overrideconfig");
373+
var arguments = this.argumentParser.ParseArguments("--override-config");
374374
arguments.OverrideConfiguration.ShouldBeNull();
375375
}
376376

377377
[TestCaseSource(nameof(OverrideconfigWithInvalidOptionTestData))]
378378
public string OverrideconfigWithInvalidOption(string options)
379379
{
380-
var exception = Assert.Throws<WarningException>(() => this.argumentParser.ParseArguments($"/overrideconfig {options}"));
380+
var exception = Assert.Throws<WarningException>(() => this.argumentParser.ParseArguments($"--override-config {options}"));
381381
exception.ShouldNotBeNull();
382382
return exception.Message;
383383
}
@@ -386,18 +386,18 @@ private static IEnumerable<TestCaseData> OverrideconfigWithInvalidOptionTestData
386386
{
387387
yield return new TestCaseData("tag-prefix=sample=asdf")
388388
{
389-
ExpectedResult = "Could not parse /overrideconfig option: tag-prefix=sample=asdf. Ensure it is in format 'key=value'."
389+
ExpectedResult = "Could not parse --override-config option: tag-prefix=sample=asdf. Ensure it is in format 'key=value'."
390390
};
391391
yield return new TestCaseData("unknown-option=25")
392392
{
393-
ExpectedResult = "Could not parse /overrideconfig option: unknown-option=25. Unsupported 'key'."
393+
ExpectedResult = "Could not parse --override-config option: unknown-option=25. Unsupported 'key'."
394394
};
395395
}
396396

397397
[TestCaseSource(nameof(OverrideConfigWithSingleOptionTestData))]
398398
public void OverrideConfigWithSingleOptions(string options, IGitVersionConfiguration expected)
399399
{
400-
var arguments = this.argumentParser.ParseArguments($"/overrideconfig {options}");
400+
var arguments = this.argumentParser.ParseArguments($"--override-config {options}");
401401

402402
ConfigurationHelper configurationHelper = new(arguments.OverrideConfiguration);
403403
configurationHelper.Configuration.ShouldBeEquivalentTo(expected);
@@ -551,23 +551,23 @@ public void OverrideConfigWithMultipleOptions(string options, IGitVersionConfigu
551551
private static IEnumerable<TestCaseData> OverrideConfigWithMultipleOptionsTestData()
552552
{
553553
yield return new TestCaseData(
554-
"/overrideconfig tag-prefix=sample /overrideconfig assembly-versioning-scheme=MajorMinor",
554+
"--override-config tag-prefix=sample --override-config assembly-versioning-scheme=MajorMinor",
555555
new GitVersionConfiguration
556556
{
557557
TagPrefixPattern = "sample",
558558
AssemblyVersioningScheme = AssemblyVersioningScheme.MajorMinor
559559
}
560560
);
561561
yield return new TestCaseData(
562-
"/overrideconfig tag-prefix=sample /overrideconfig assembly-versioning-format=\"{Major}.{Minor}.{Patch}.{env:CI_JOB_ID ?? 0}\"",
562+
"--override-config tag-prefix=sample --override-config assembly-versioning-format=\"{Major}.{Minor}.{Patch}.{env:CI_JOB_ID ?? 0}\"",
563563
new GitVersionConfiguration
564564
{
565565
TagPrefixPattern = "sample",
566566
AssemblyVersioningFormat = "{Major}.{Minor}.{Patch}.{env:CI_JOB_ID ?? 0}"
567567
}
568568
);
569569
yield return new TestCaseData(
570-
"/overrideconfig tag-prefix=sample /overrideconfig assembly-versioning-format=\"{Major}.{Minor}.{Patch}.{env:CI_JOB_ID ?? 0}\" /overrideconfig update-build-number=true /overrideconfig assembly-versioning-scheme=MajorMinorPatchTag /overrideconfig mode=ContinuousDelivery /overrideconfig tag-pre-release-weight=4",
570+
"--override-config tag-prefix=sample --override-config assembly-versioning-format=\"{Major}.{Minor}.{Patch}.{env:CI_JOB_ID ?? 0}\" --override-config update-build-number=true --override-config assembly-versioning-scheme=MajorMinorPatchTag --override-config mode=ContinuousDelivery --override-config tag-pre-release-weight=4",
571571
new GitVersionConfiguration
572572
{
573573
TagPrefixPattern = "sample",
@@ -711,7 +711,7 @@ public void LogPathCanContainForwardSlash()
711711
[Test]
712712
public void BooleanArgumentHandling()
713713
{
714-
var arguments = this.argumentParser.ParseArguments("/nofetch /updateassemblyinfo true");
714+
var arguments = this.argumentParser.ParseArguments("--no-fetch --update-assembly-info true");
715715
arguments.NoFetch.ShouldBe(true);
716716
arguments.UpdateAssemblyInfo.ShouldBe(true);
717717
}

src/GitVersion.App/SpectreArgumentParser.cs

Lines changed: 17 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -90,161 +90,8 @@ public Arguments ParseArguments(string[] commandLineArguments)
9090
}
9191
catch (Exception)
9292
{
93-
// If parsing fails, try to handle legacy forward slash syntax
94-
return HandleLegacySyntax(commandLineArguments);
95-
}
96-
97-
return CreateDefaultArguments();
98-
}
99-
100-
private Arguments HandleLegacySyntax(string[] commandLineArguments)
101-
{
102-
// Convert legacy forward slash syntax to hyphen syntax for Spectre.Console.Cli
103-
var convertedArgs = new List<string>();
104-
105-
for (int i = 0; i < commandLineArguments.Length; i++)
106-
{
107-
var arg = commandLineArguments[i];
108-
109-
// Handle legacy forward slash options
110-
if (arg.StartsWith('/'))
111-
{
112-
var option = arg.Substring(1).ToLowerInvariant();
113-
switch (option)
114-
{
115-
case "output":
116-
convertedArgs.Add("--output");
117-
break;
118-
case "outputfile":
119-
convertedArgs.Add("--output-file");
120-
break;
121-
case "showvariable":
122-
convertedArgs.Add("--show-variable");
123-
break;
124-
case "format":
125-
convertedArgs.Add("--format");
126-
break;
127-
case "l":
128-
convertedArgs.Add("--log-file");
129-
break;
130-
case "config":
131-
convertedArgs.Add("--config");
132-
break;
133-
case "showconfig":
134-
convertedArgs.Add("--show-config");
135-
break;
136-
case "overrideconfig":
137-
convertedArgs.Add("--override-config");
138-
// Handle the key=value format for override config
139-
if (i + 1 < commandLineArguments.Length)
140-
{
141-
var nextArg = commandLineArguments[i + 1];
142-
if (nextArg.Contains('=') && !nextArg.StartsWith('/') && !nextArg.StartsWith('-'))
143-
{
144-
// This is the key=value pair for override config
145-
convertedArgs.Add(nextArg);
146-
i++; // Skip the next argument since we consumed it
147-
}
148-
}
149-
break;
150-
case "nocache":
151-
convertedArgs.Add("--no-cache");
152-
break;
153-
case "nonormalize":
154-
convertedArgs.Add("--no-normalize");
155-
break;
156-
case "allowshallow":
157-
convertedArgs.Add("--allow-shallow");
158-
break;
159-
case "verbosity":
160-
convertedArgs.Add("--verbosity");
161-
break;
162-
case "updateassemblyinfo":
163-
convertedArgs.Add("--update-assembly-info");
164-
break;
165-
case "updateprojectfiles":
166-
convertedArgs.Add("--update-project-files");
167-
break;
168-
case "ensureassemblyinfo":
169-
convertedArgs.Add("--ensure-assembly-info");
170-
break;
171-
case "updatewixversionfile":
172-
convertedArgs.Add("--update-wix-version-file");
173-
break;
174-
case "url":
175-
convertedArgs.Add("--url");
176-
break;
177-
case "b":
178-
convertedArgs.Add("--branch");
179-
break;
180-
case "u":
181-
convertedArgs.Add("--username");
182-
break;
183-
case "p":
184-
convertedArgs.Add("--password");
185-
break;
186-
case "c":
187-
convertedArgs.Add("--commit");
188-
break;
189-
case "dynamicrepolocation":
190-
convertedArgs.Add("--dynamic-repo-location");
191-
break;
192-
case "nofetch":
193-
convertedArgs.Add("--no-fetch");
194-
break;
195-
case "targetpath":
196-
convertedArgs.Add("--target-path");
197-
break;
198-
case "diag":
199-
convertedArgs.Add("--diag");
200-
break;
201-
default:
202-
// Unknown option, keep as is
203-
convertedArgs.Add(arg);
204-
break;
205-
}
206-
}
207-
else if (!arg.StartsWith('-') && i == 0)
208-
{
209-
// First non-option argument is likely the target path
210-
convertedArgs.Add(arg);
211-
}
212-
else
213-
{
214-
// Regular argument or already in correct format
215-
convertedArgs.Add(arg);
216-
}
217-
}
218-
219-
// Try parsing again with converted arguments
220-
try
221-
{
222-
var app = new CommandApp<GitVersionCommand>();
223-
app.Configure(config =>
224-
{
225-
config.SetApplicationName("gitversion");
226-
config.PropagateExceptions();
227-
});
228-
229-
var resultStorage = new ParseResultStorage();
230-
var interceptor = new ArgumentInterceptor(resultStorage, this.environment, this.fileSystem, this.buildAgent, this.console, this.globbingResolver);
231-
app.Configure(config => config.Settings.Interceptor = interceptor);
232-
233-
var parseResult = app.Run(convertedArgs.ToArray());
234-
235-
var result = resultStorage.GetResult();
236-
if (result != null)
237-
{
238-
return result;
239-
}
240-
}
241-
catch (Exception)
242-
{
243-
// Final fallback - if it's a single argument and not an option, treat as target path
244-
if (commandLineArguments.Length == 1 && !commandLineArguments[0].StartsWith('-') && !commandLineArguments[0].StartsWith('/'))
245-
{
246-
return CreateArgumentsWithTargetPath(commandLineArguments[0]);
247-
}
93+
// If parsing fails, return default arguments
94+
return CreateDefaultArguments();
24895
}
24996

25097
return CreateDefaultArguments();
@@ -262,18 +109,6 @@ private Arguments CreateDefaultArguments()
262109
return args;
263110
}
264111

265-
private Arguments CreateArgumentsWithTargetPath(string targetPath)
266-
{
267-
var args = new Arguments
268-
{
269-
TargetPath = targetPath.TrimEnd('/', '\\')
270-
};
271-
args.Output.Add(OutputType.Json);
272-
AddAuthentication(args);
273-
args.NoFetch = this.buildAgent.PreventFetch();
274-
return args;
275-
}
276-
277112
private void AddAuthentication(Arguments arguments)
278113
{
279114
var username = this.environment.GetEnvironmentVariable("GITVERSION_REMOTE_USERNAME");
@@ -437,12 +272,24 @@ private static Arguments ConvertToArguments(GitVersionSettings settings)
437272
// Handle override configuration
438273
if (settings.OverrideConfiguration != null && settings.OverrideConfiguration.Any())
439274
{
440-
var overrideConfig = new Dictionary<object, object?>();
275+
var parser = new OverrideConfigurationOptionParser();
276+
441277
foreach (var kvp in settings.OverrideConfiguration)
442278
{
443-
overrideConfig[kvp.Key] = kvp.Value;
279+
// Validate the key format - Spectre.Console.Cli should have already parsed key=value correctly
280+
// but we still need to validate against supported properties
281+
var keyValueOption = $"{kvp.Key}={kvp.Value}";
282+
283+
var optionKey = kvp.Key.ToLowerInvariant();
284+
if (!OverrideConfigurationOptionParser.SupportedProperties.Contains(optionKey))
285+
{
286+
throw new WarningException($"Could not parse --override-config option: {keyValueOption}. Unsupported 'key'.");
287+
}
288+
289+
parser.SetValue(optionKey, kvp.Value);
444290
}
445-
arguments.OverrideConfiguration = overrideConfig;
291+
292+
arguments.OverrideConfiguration = parser.GetOverrideConfiguration();
446293
}
447294
else
448295
{

0 commit comments

Comments
 (0)