Skip to content

Simplify command and parse result handling #2261

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.

Sign up for GitHub

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

Merged
alzimmermsft merged 8 commits into from
Apr 17, 2026
Merged

Simplify command and parse result handling #2261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
390bf31
Small PoC on simplifying command and parse result handling
alzimmermsft Mar 27, 2026
40868e5
Merge branch 'main' into ChangeCommandParsingExtensions
alzimmermsft Apr 10, 2026
8eddcf2
PR feedback
alzimmermsft Apr 10, 2026
e952a40
Test updates
alzimmermsft Apr 10, 2026
e81fab0
Add more tests
alzimmermsft Apr 10, 2026
b4b9aba
Merge branch 'main' into ChangeCommandParsingExtensions
alzimmermsft Apr 13, 2026
df91f90
Merge branch 'main' into ChangeCommandParsingExtensions
alzimmermsft Apr 16, 2026
7b9be56
PR feedback
alzimmermsft Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
535 changes: 218 additions & 317 deletions .../Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Extensions/CommandResultExtensionsTests.cs
View file
Open in desktop

Large diffs are not rendered by default.

86 changes: 48 additions & 38 deletions core/Microsoft.Mcp.Core/src/Extensions/CommandResultExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,25 @@ namespace Microsoft.Mcp.Core.Extensions;
public static class CommandResultExtensions
{
public static bool HasOptionResult(this CommandResult commandResult, Option option)
=> HasOptionResult(commandResult, option.Name);

public static bool HasOptionResult<T>(this CommandResult commandResult, Option<T> option)
=> HasOptionResult(commandResult, option.Name);

/// <summary>
/// Checks if an option with the specified name or alias has a result in the command result.
/// </summary>
/// <param name="commandResult">The command result to check.</param>
/// <param name="optionName">The option name or any configured alias to check for.</param>
/// <returns>True if the option has a result, false otherwise.</returns>
public static bool HasOptionResult(this CommandResult commandResult, string optionName)
{
// Find the option by name in the command
var option = commandResult.Command.Options
.FirstOrDefault(o => o.Name == optionName || o.Aliases.Contains(optionName));
Comment thread
alzimmermsft marked this conversation as resolved.
if (option is null)
return false;

var result = commandResult.GetResult(option);
if (result is null)
return false;
Expand Down Expand Up @@ -36,26 +54,20 @@ public static bool HasOptionResult(this CommandResult commandResult, Option opti
return hasNonEmptyValue;
}

public static bool HasOptionResult<T>(this CommandResult commandResult, Option<T> option)
=> HasOptionResult(commandResult, (Option)option);
public static bool TryGetValue<T>(this CommandResult commandResult, Option<T> option, out T? value)
=> TryGetValue(commandResult, option.Name, out value);

/// <summary>
/// Checks if an option with the specified name has a result in the command result.
/// </summary>
/// <param name="commandResult">The command result to check.</param>
/// <param name="optionName">The name of the option to check for (including -- prefix).</param>
/// <returns>True if the option has a result, false otherwise.</returns>
public static bool HasOptionResult(this CommandResult commandResult, string optionName)
public static bool TryGetValue<T>(this CommandResult commandResult, string optionName, out T? value)
{
// Find the option by name in the command
var option = commandResult.Command.Options
.FirstOrDefault(o => o.Name == optionName || o.Aliases.Contains(optionName));
var option = FindOptionTByName<T>(commandResult, optionName);

return option != null && HasOptionResult(commandResult, option);
}
if (option is null)
{
value = default;
return false;
}

public static bool TryGetValue<T>(this CommandResult commandResult, Option<T> option, out T? value)
{
// If the option has any result (explicit or implicit), attempt to read its value.
var result = commandResult.GetResult(option);
if (result is not null)
Expand Down Expand Up @@ -92,21 +104,26 @@ public static bool TryGetValue<T>(this CommandResult commandResult, Option<T> op
return false;
}

public static bool TryGetValue<T>(this CommandResult commandResult, string optionName, out T? value)
public static T? GetValueOrDefault<T>(this CommandResult commandResult, Option<T> option)
{
ArgumentNullException.ThrowIfNull(commandResult);
ArgumentNullException.ThrowIfNull(option);
return GetValueOrDefault<T>(commandResult, option.Name);
}

Comment thread
alzimmermsft marked this conversation as resolved.
public static T? GetValueOrDefault<T>(this CommandResult commandResult, string optionName)
{
// Find the option by name in the command
var option = FindOptionTByName<T>(commandResult, optionName);

if (option is null)
{
value = default;
return false;
return default;
}

return TryGetValue(commandResult, option, out value);
return GetValueOrDefaultImpl(commandResult, option);
}

public static T? GetValueOrDefault<T>(this CommandResult commandResult, Option<T> option)
private static T? GetValueOrDefaultImpl<T>(CommandResult commandResult, Option<T> option)
{
ArgumentNullException.ThrowIfNull(commandResult);
ArgumentNullException.ThrowIfNull(option);
Expand Down Expand Up @@ -140,20 +157,26 @@ public static bool TryGetValue<T>(this CommandResult commandResult, string optio
return optionResult.GetValueOrDefault<T>();
}

public static T? GetValueOrDefault<T>(this CommandResult commandResult, string optionName)
public static T? GetValueWithoutDefault<T>(this CommandResult commandResult, Option<T> option)
{
ArgumentNullException.ThrowIfNull(commandResult);
ArgumentNullException.ThrowIfNull(option);
return GetValueWithoutDefault<T>(commandResult, option.Name);
}

public static T? GetValueWithoutDefault<T>(this CommandResult commandResult, string optionName)
{
// Find the option by name in the command
var option = FindOptionTByName<T>(commandResult, optionName);

if (option is null)
{
return default;
}

return GetValueOrDefault(commandResult, option);
return GetValueWithoutDefaultImpl(commandResult, option);
}

public static T? GetValueWithoutDefault<T>(this CommandResult commandResult, Option<T> option)
private static T? GetValueWithoutDefaultImpl<T>(CommandResult commandResult, Option<T> option)
{
ArgumentNullException.ThrowIfNull(commandResult);
ArgumentNullException.ThrowIfNull(option);
Expand All @@ -173,19 +196,6 @@ public static bool TryGetValue<T>(this CommandResult commandResult, string optio
return optionResult.GetValueOrDefault<T>();
}

public static T? GetValueWithoutDefault<T>(this CommandResult commandResult, string optionName)
{
// Find the option by name in the command
var option = FindOptionTByName<T>(commandResult, optionName);

if (option is null)
{
return default;
}

return GetValueWithoutDefault(commandResult, option);
}

private static Option<T>? FindOptionTByName<T>(CommandResult commandResult, string optionName)
=> commandResult.Command.Options.OfType<Option<T>>()
.FirstOrDefault(o => o.Name == optionName || o.Aliases.Contains(optionName));
Expand Down
73 changes: 9 additions & 64 deletions core/Microsoft.Mcp.Core/src/Extensions/ParseResultExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,86 +1,31 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.CommandLine.Parsing;
using Microsoft.Mcp.Core.Helpers;
using Microsoft.Mcp.Core.Models.Option;

namespace Microsoft.Mcp.Core.Extensions;

public static class ParseResultExtensions
{
public static bool TryGetValue<T>(this ParseResult parseResult, Option<T> option, out T? value)
=> parseResult.CommandResult.TryGetValue(option, out value);
=> TryGetValue(parseResult, option.Name, out value);

Comment thread
alzimmermsft marked this conversation as resolved.
public static bool TryGetValue<T>(this ParseResult parseResult, string optionName, out T? value)
{
// Find the option by name in the command
var command = parseResult.CommandResult.Command;
var option = command.Options.OfType<Option<T>>()
.FirstOrDefault(o => o.Name == optionName || o.Aliases.Contains(optionName));

if (option != null)
{
return parseResult.CommandResult.TryGetValue(option, out value);
}

value = default;
return false;
}
=> parseResult.CommandResult.TryGetValue(optionName, out value);

public static T? GetValueOrDefault<T>(this ParseResult parseResult, Option<T> option)
=> parseResult.CommandResult.GetValueOrDefault(option);
=> GetValueOrDefault<T>(parseResult, option.Name);

/// <summary>
/// Gets the value of an option by name, returning default if not found or not set
/// </summary>
public static T? GetValueOrDefault<T>(this ParseResult parseResult, string optionName)
{
// Find the option by name in the command
var command = parseResult.CommandResult.Command;
var option = command.Options.OfType<Option<T>>()
.FirstOrDefault(o => o.Name == optionName || o.Aliases.Contains(optionName));

return option != null ? parseResult.GetValueOrDefault(option) : default;
}
=> parseResult.CommandResult.GetValueOrDefault<T>(optionName);

public static bool HasAnyRetryOptions(this ParseResult parseResult)
{
foreach (var child in parseResult.CommandResult.Children)
{
if (child is OptionResult optionResult)
{
var option = optionResult.Option;
if (option is null)
{
continue;
}

var name = NameNormalization.NormalizeOptionName(option.Name);
if (RetryOptionNames.Contains(name))
return true;

var aliases = option.Aliases ?? [];
foreach (var alias in aliases)
{
var normalized = NameNormalization.NormalizeOptionName(alias);
if (RetryOptionNames.Contains(normalized))
{
return true;
}
}
}
}

return false;
}

private static readonly HashSet<string> RetryOptionNames = new(StringComparer.OrdinalIgnoreCase)
{
NameNormalization.NormalizeOptionName(OptionDefinitions.RetryPolicy.DelayName),
NameNormalization.NormalizeOptionName(OptionDefinitions.RetryPolicy.MaxDelayName),
NameNormalization.NormalizeOptionName(OptionDefinitions.RetryPolicy.MaxRetriesName),
NameNormalization.NormalizeOptionName(OptionDefinitions.RetryPolicy.ModeName),
NameNormalization.NormalizeOptionName(OptionDefinitions.RetryPolicy.NetworkTimeoutName),
};
=> parseResult.CommandResult.HasOptionResult(OptionDefinitions.RetryPolicy.Delay) ||
parseResult.CommandResult.HasOptionResult(OptionDefinitions.RetryPolicy.MaxDelay) ||
parseResult.CommandResult.HasOptionResult(OptionDefinitions.RetryPolicy.MaxRetries) ||
parseResult.CommandResult.HasOptionResult(OptionDefinitions.RetryPolicy.Mode) ||
parseResult.CommandResult.HasOptionResult(OptionDefinitions.RetryPolicy.NetworkTimeout);
}
30 changes: 15 additions & 15 deletions tools/Azure.Mcp.Tools.Compute/src/Commands/Vmss/VmssUpdateCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ protected override void RegisterOptions(Command command)
command.Validators.Add(commandResult =>
{
// Custom validation: At least one update property must be specified
if (string.IsNullOrEmpty(commandResult.GetValueOrDefault<string>(ComputeOptionDefinitions.UpgradePolicy.Name)) &&
!commandResult.GetValueOrDefault<int?>(ComputeOptionDefinitions.Capacity.Name).HasValue &&
string.IsNullOrEmpty(commandResult.GetValueOrDefault<string>(ComputeOptionDefinitions.VmSize.Name)) &&
!commandResult.GetValueOrDefault<bool?>(ComputeOptionDefinitions.Overprovision.Name).HasValue &&
!commandResult.GetValueOrDefault<bool?>(ComputeOptionDefinitions.EnableAutoOsUpgrade.Name).HasValue &&
string.IsNullOrEmpty(commandResult.GetValueOrDefault<string>(ComputeOptionDefinitions.ScaleInPolicy.Name)) &&
string.IsNullOrEmpty(commandResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Tags.Name)))
if (!commandResult.HasOptionResult(ComputeOptionDefinitions.UpgradePolicy) &&
!commandResult.HasOptionResult(ComputeOptionDefinitions.Capacity) &&
Comment thread
alzimmermsft marked this conversation as resolved.
!commandResult.HasOptionResult(ComputeOptionDefinitions.VmSize) &&
!commandResult.HasOptionResult(ComputeOptionDefinitions.Overprovision) &&
!commandResult.HasOptionResult(ComputeOptionDefinitions.EnableAutoOsUpgrade) &&
!commandResult.HasOptionResult(ComputeOptionDefinitions.ScaleInPolicy) &&
!commandResult.HasOptionResult(ComputeOptionDefinitions.Tags))
{
commandResult.AddError(
"At least one update property must be specified: --upgrade-policy, --capacity, --vm-size, --overprovision, --enable-auto-os-upgrade, --scale-in-policy, or --tags.");
Expand All @@ -80,14 +80,14 @@ protected override void RegisterOptions(Command command)
protected override VmssUpdateOptions BindOptions(ParseResult parseResult)
{
var options = base.BindOptions(parseResult);
options.VmssName = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.VmssName.Name);
options.UpgradePolicy = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.UpgradePolicy.Name);
options.Capacity = parseResult.GetValueOrDefault<int?>(ComputeOptionDefinitions.Capacity.Name);
options.VmSize = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.VmSize.Name);
options.Overprovision = parseResult.GetValueOrDefault<bool?>(ComputeOptionDefinitions.Overprovision.Name);
options.EnableAutoOsUpgrade = parseResult.GetValueOrDefault<bool?>(ComputeOptionDefinitions.EnableAutoOsUpgrade.Name);
options.ScaleInPolicy = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.ScaleInPolicy.Name);
options.Tags = parseResult.GetValueOrDefault<string>(ComputeOptionDefinitions.Tags.Name);
options.VmssName = parseResult.GetValueOrDefault(ComputeOptionDefinitions.VmssName);
options.UpgradePolicy = parseResult.GetValueOrDefault(ComputeOptionDefinitions.UpgradePolicy);
options.Capacity = parseResult.GetValueOrDefault(ComputeOptionDefinitions.Capacity);
options.VmSize = parseResult.GetValueOrDefault(ComputeOptionDefinitions.VmSize);
options.Overprovision = parseResult.GetValueOrDefault(ComputeOptionDefinitions.Overprovision);
options.EnableAutoOsUpgrade = parseResult.GetValueOrDefault(ComputeOptionDefinitions.EnableAutoOsUpgrade);
options.ScaleInPolicy = parseResult.GetValueOrDefault(ComputeOptionDefinitions.ScaleInPolicy);
options.Tags = parseResult.GetValueOrDefault(ComputeOptionDefinitions.Tags);
return options;
}

Expand Down
6 changes: 3 additions & 3 deletions tools/Azure.Mcp.Tools.Compute/src/Options/ComputeOptionDefinitions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public static class ComputeOptionDefinitions

public const string CapacityName = "capacity";

public static readonly Option<int> Capacity = new($"--{CapacityName}")
public static readonly Option<int?> Capacity = new($"--{CapacityName}")
Comment thread
alzimmermsft marked this conversation as resolved.
{
Description = "Number of VM instances (capacity) in the scale set",
Required = false
Expand All @@ -317,13 +317,13 @@ public static class ComputeOptionDefinitions
public const string OverprovisionName = "overprovision";
public const string EnableAutoOsUpgradeName = "enable-auto-os-upgrade";
public const string ScaleInPolicyName = "scale-in-policy";
public static readonly Option<bool> Overprovision = new($"--{OverprovisionName}")
public static readonly Option<bool?> Overprovision = new($"--{OverprovisionName}")
{
Description = "Enable or disable overprovisioning. When enabled, Azure provisions more VMs than requested and deletes extra VMs after deployment",
Required = false
};

public static readonly Option<bool> EnableAutoOsUpgrade = new($"--{EnableAutoOsUpgradeName}")
public static readonly Option<bool?> EnableAutoOsUpgrade = new($"--{EnableAutoOsUpgradeName}")
{
Description = "Enable automatic OS image upgrades. Requires health probes or Application Health extension",
Required = false
Expand Down
2 changes: 1 addition & 1 deletion tools/Azure.Mcp.Tools.Functions/src/Commands/Project/ProjectGetCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ protected override void RegisterOptions(Command command)

command.Validators.Add(commandResult =>
{
var language = commandResult.GetValueWithoutDefault<string>(FunctionsOptionDefinitions.Language.Name);
var language = commandResult.GetValueWithoutDefault(FunctionsOptionDefinitions.Language);
if (string.IsNullOrWhiteSpace(language))
{
commandResult.AddError("The --language parameter is required.");
Expand Down
2 changes: 1 addition & 1 deletion tools/Azure.Mcp.Tools.Functions/src/Commands/Template/TemplateGetCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected override void RegisterOptions(Command command)

command.Validators.Add(commandResult =>
{
var language = commandResult.GetValueWithoutDefault<string>(FunctionsOptionDefinitions.Language.Name);
var language = commandResult.GetValueWithoutDefault(FunctionsOptionDefinitions.Language);
if (string.IsNullOrWhiteSpace(language))
{
commandResult.AddError("The --language parameter is required.");
Expand Down
4 changes: 2 additions & 2 deletions tools/Azure.Mcp.Tools.LoadTesting/src/Commands/LoadTestRun/TestRunGetCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ protected override void RegisterOptions(Command command)

command.Validators.Add(commandResult =>
{
var testRunId = commandResult.GetValueWithoutDefault<string>(LoadTestingOptionDefinitions.TestRun.Name);
var testId = commandResult.GetValueWithoutDefault<string>(LoadTestingOptionDefinitions.Test.Name);
var testRunId = commandResult.GetValueWithoutDefault(LoadTestingOptionDefinitions.TestRun);
var testId = commandResult.GetValueWithoutDefault(LoadTestingOptionDefinitions.Test);

if (string.IsNullOrEmpty(testRunId) && string.IsNullOrEmpty(testId))
{
Expand Down
Loading