-
Notifications
You must be signed in to change notification settings - Fork 370
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.
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
feature/system-command-line --> main #4222
Conversation
803fb52
to
e72ce4c
Compare
} | ||
} | ||
} | ||
//#nullable enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string commandName) | ||
: base(host, logger, callbacks, commandName, SymbolStrings.Command_AliasAdd_Description) { } | ||
|
||
protected override Task<NewCommandStatus> ExecuteAsync(AliasAddCommandArgs args, IEngineEnvironmentSettings environmentSettings, InvocationContext context) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.Add(new AliasShowCommand(host, telemetryLogger, callbacks)); | ||
} | ||
|
||
protected override Task<NewCommandStatus> ExecuteAsync(AliasCommandArgs args, IEngineEnvironmentSettings environmentSettings, InvocationContext context) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string commandName) | ||
: base(host, logger, callbacks, commandName, SymbolStrings.Command_AliasShow_Description) { } | ||
|
||
protected override Task<NewCommandStatus> ExecuteAsync(AliasShowCommandArgs args, IEngineEnvironmentSettings environmentSettings, InvocationContext context) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | ||
//TODO: consider refactoring based on command definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string? defaultLanguage) | ||
{ | ||
IReadOnlyDictionary<string, string?>? appliedParameterMatches = TemplateCommandInput.GetTemplateParametersFromCommand(commandInput); | ||
//TODO: implement it for template options matching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | ||
if (string.IsNullOrWhiteSpace(commandInput.SearchNameCriteria) && SupportedFilters.All(filter => !filter.IsFilterSet(commandInput)) && !commandInput.RemainingParameters.Any()) | ||
if (string.IsNullOrWhiteSpace(commandArgs.SearchNameCriteria) && !commandArgs.AppliedFilters.Any()) | ||
//TODO: implement it for template options matching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IEnumerable<string> appliedFilters = commandArgs.AppliedFilters | ||
.Select(filter => $"{commandArgs.GetFilterToken(filter)}='{commandArgs.GetFilterValue(filter)}'"); | ||
|
||
//TODO: implement it for template options matching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var builder = new CommandLineBuilder(command) | ||
//.UseExceptionHandler(ExceptionHandler) | ||
//.UseLocalizationResources(new CommandLineValidationMessages()) | ||
//TODO: decide if it's needed to implement it; and implement if needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2191: implement [parse] and [suggest] directives
//TODO: decide if it's needed to implement it; and implement if needed | ||
//.UseParseDirective() | ||
//.UseSuggestDirective() | ||
.UseParseErrorReporting()//TODO: discuss with SDK if it is possible to use it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
78b4c7d
to
1472467
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this changes were already reviewed individually in feature branch, hence accepting as is.
moved AsyncMutex to Utils, documented creating IEngineEnvironmentSettings in BaseCommand now
This allows ParseContext to be called while handing suggestions
Also fixed most --help problems.
exception handling is needed as System.CommandLine is getting long stack trace for the error
* fixed existing tests; added TestHost.GetVirtualHost helper method Added possibility to exclude test templates * install command parser tests; fixed install integration tests updated System.CommandLine to latest version tabular output refactoring (still needs parsing of --columns and --columns-all * fixed tab completion tests on unix/macos * help builder fix * implemented validation for incorrect order of options in non-legacy scenario
* fixes dotnet/templating #3807 update subcommand
Co-authored-by: Bekir Ozturk <60651445+bekir-ozturk@users.noreply.github.com>
…eplaced with creating InstantiateCommandArgs from NewCommandArgs
b49ff14
to
a8c5a46
Compare
PR for merging system-command-line feature branch to main