Skip to content
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

[system-command-line] error handling for ambiguous results on template instantiation #4227

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/Microsoft.TemplateEngine.Cli/AnsiConsole.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

namespace Microsoft.TemplateEngine.Cli
{
internal class AnsiConsole
{
private int _boldRecursion;

private AnsiConsole(TextWriter writer)
internal AnsiConsole(TextWriter writer)
{
Writer = writer;

Expand Down
29 changes: 29 additions & 0 deletions src/Microsoft.TemplateEngine.Cli/CliTemplateInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#nullable enable

using Microsoft.TemplateEngine.Abstractions;
using Microsoft.TemplateEngine.Abstractions.TemplatePackage;
using Microsoft.TemplateEngine.Edge.Settings;
using Microsoft.TemplateEngine.Utils;

namespace Microsoft.TemplateEngine.Cli
Expand Down Expand Up @@ -120,5 +122,32 @@ internal static IEnumerable<CliTemplateInfo> FromTemplateInfo(IEnumerable<ITempl

return templateInfos.Select(templateInfo => new CliTemplateInfo(templateInfo, hostSpecificDataLoader.ReadHostSpecificTemplateData(templateInfo)));
}

/// <summary>
/// Gets the <b>managed</b> template package which contain the template, <see langword="null"/> otherwise.
/// </summary>
/// <remarks>
/// The method might throw exceptions if <see cref="TemplatePackageManager.GetTemplatePackageAsync(ITemplateInfo, CancellationToken)"/> call throws.
/// </remarks>
internal async Task<IManagedTemplatePackage?> GetManagedTemplatePackageAsync(
TemplatePackageManager templatePackageManager,
CancellationToken cancellationToken)
{
ITemplatePackage templatePackage = await GetTemplatePackageAsync(templatePackageManager, cancellationToken).ConfigureAwait(false);
return templatePackage as IManagedTemplatePackage;
}

/// <summary>
/// Gets the template package which contains the template.
/// </summary>
/// <remarks>
/// The method might throw exceptions if <see cref="TemplatePackageManager.GetTemplatePackageAsync(ITemplateInfo, CancellationToken)"/> call throws.
/// </remarks>
internal Task<ITemplatePackage> GetTemplatePackageAsync(
TemplatePackageManager templatePackageManager,
CancellationToken cancellationToken)
{
return templatePackageManager.GetTemplatePackageAsync(this, cancellationToken);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void WriteHelp(HelpContext context, ParseResult parseResult)
}
if (selectedTemplateGroups.Take(2).Count() > 1)
{
HandleAmbiguousTemplateGroup(instantiateCommandArgs);
HandleAmbiguousTemplateGroup(environmentSettings, templatePackageManager, selectedTemplateGroups, Reporter.Output);
return;
}

Expand All @@ -78,8 +78,8 @@ public void WriteHelp(HelpContext context, ParseResult parseResult)

if (!VerifyMatchingTemplates(
environmentSettings,
context.Output,
matchingTemplates,
Reporter.Output,
out IEnumerable<TemplateCommand>? templatesToShow))
{
//error
Expand Down Expand Up @@ -129,8 +129,8 @@ internal static IEnumerable<TemplateCommand> GetMatchingTemplates(

internal static bool VerifyMatchingTemplates(
IEngineEnvironmentSettings environmentSettings,
TextWriter writer,
IEnumerable<TemplateCommand> matchingTemplates,
Reporter reporter,
[NotNullWhen(true)]
out IEnumerable<TemplateCommand>? filteredTemplates)
{
Expand All @@ -153,7 +153,7 @@ internal static bool VerifyMatchingTemplates(
HandleAmbiguousLanguage(
environmentSettings,
matchingTemplates.Select(c => c.Template),
writer);
reporter);

filteredTemplates = null;
return false;
Expand All @@ -164,7 +164,7 @@ internal static bool VerifyMatchingTemplates(
HandleAmbiguousLanguage(
environmentSettings,
matchingTemplates.Select(c => c.Template),
writer);
reporter);

filteredTemplates = null;
return false;
Expand All @@ -178,7 +178,7 @@ internal static bool VerifyMatchingTemplates(
HandleAmbiguousType(
environmentSettings,
matchingTemplates.Select(c => c.Template),
writer);
reporter);
filteredTemplates = null;
return false;
}
Expand Down Expand Up @@ -371,38 +371,6 @@ internal void ShowUsage(IReadOnlyList<string> shortNames, HelpContext context)
context.Output.WriteLine();
}

private static NewCommandStatus HandleAmbiguousLanguage(
IEngineEnvironmentSettings environmentSettings,
IEnumerable<CliTemplateInfo> templates,
TextWriter writer)
{
writer.WriteLine(HelpStrings.TableHeader_AmbiguousTemplatesList);
TemplateGroupDisplay.DisplayTemplateList(
environmentSettings,
templates,
new TabularOutputSettings(environmentSettings.Environment),
writer: writer);
writer.WriteLine(HelpStrings.Hint_AmbiguousLanguage);
return NewCommandStatus.NotFound;
}

private static NewCommandStatus HandleAmbiguousType(
IEngineEnvironmentSettings environmentSettings,
IEnumerable<CliTemplateInfo> templates,
TextWriter writer)
{
writer.WriteLine(HelpStrings.TableHeader_AmbiguousTemplatesList);
TemplateGroupDisplay.DisplayTemplateList(
environmentSettings,
templates,
new TabularOutputSettings(
environmentSettings.Environment,
columnsToDisplay: new[] { TabularOutputSettings.ColumnNames.Type }),
writer: writer);
writer.WriteLine(HelpStrings.Hint_AmbiguousType);
return NewCommandStatus.NotFound;
}

/// <summary>
/// Ensure <paramref name="templates"/> are sorted in priority order
/// The highest priority should come first.
Expand Down
159 changes: 155 additions & 4 deletions src/Microsoft.TemplateEngine.Cli/Commands/InstantiateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
using System.CommandLine.Help;
using System.CommandLine.Invocation;
using System.CommandLine.Parsing;
using Microsoft.Extensions.Logging;
using Microsoft.TemplateEngine.Abstractions;
using Microsoft.TemplateEngine.Abstractions.TemplatePackage;
using Microsoft.TemplateEngine.Cli.Extensions;
using Microsoft.TemplateEngine.Cli.TabularOutput;
using Microsoft.TemplateEngine.Edge.Settings;
using Microsoft.TemplateEngine.Utils;

namespace Microsoft.TemplateEngine.Cli.Commands
{
Expand Down Expand Up @@ -136,7 +140,48 @@ internal void HandleNoMatchingTemplateGroup(InstantiateCommandArgs instantiateAr
reporter.WriteLine();
}

internal NewCommandStatus HandleAmbiguousTemplateGroup(InstantiateCommandArgs instantiateArgs) => throw new NotImplementedException();
internal NewCommandStatus HandleAmbiguousTemplateGroup(
IEngineEnvironmentSettings environmentSettings,
TemplatePackageManager templatePackageManager,
IEnumerable<TemplateGroup> templateGroups,
Reporter reporter,
CancellationToken cancellationToken = default)
{
IEnvironment environment = environmentSettings.Environment;
reporter.WriteLine(LocalizableStrings.AmbiguousTemplatesHeader.Bold().Red());
TabularOutput<TemplateGroup> formatter =
TabularOutput.TabularOutput
.For(
new TabularOutputSettings(environment),
templateGroups)
.DefineColumn(t => t.GroupIdentity ?? t.Templates[0].Identity, out object identityColumn, LocalizableStrings.ColumnNameIdentity, showAlways: true)
.DefineColumn(t => t.Name, LocalizableStrings.ColumnNameTemplateName, shrinkIfNeeded: true, minWidth: 15, showAlways: true)
.DefineColumn(t => string.Join(",", t.ShortNames), LocalizableStrings.ColumnNameShortName, showAlways: true)
.DefineColumn(t => string.Join(",", t.Languages), LocalizableStrings.ColumnNameLanguage, showAlways: true)
.DefineColumn(t => string.Join(",", t.Authors), LocalizableStrings.ColumnNameAuthor, showAlways: true, shrinkIfNeeded: true, minWidth: 10)
.DefineColumn(t => Task.Run(() => GetTemplatePackagesList(t)).GetAwaiter().GetResult(), LocalizableStrings.ColumnNamePackage, showAlways: true)
.OrderBy(identityColumn, StringComparer.CurrentCultureIgnoreCase);

reporter.WriteLine(formatter.Layout().Bold().Red());
reporter.WriteLine(LocalizableStrings.AmbiguousTemplatesMultiplePackagesHint.Bold().Red());
return NewCommandStatus.NotFound;

async Task<string> GetTemplatePackagesList(TemplateGroup templateGroup)
{
try
{
IReadOnlyList<IManagedTemplatePackage> templatePackages =
await templateGroup.GetManagedTemplatePackagesAsync(templatePackageManager, cancellationToken).ConfigureAwait(false);
return string.Join(environment.NewLine, templatePackages.Select(templatePackage => templatePackage.Identifier));
}
catch (Exception ex)
{
environmentSettings.Host.Logger.LogWarning($"Failed to get information about template packages for template group {templateGroup.GroupIdentity}.");
environmentSettings.Host.Logger.LogDebug($"Details: {ex}.");
return string.Empty;
}
}
}

protected async override Task<NewCommandStatus> ExecuteAsync(InstantiateCommandArgs instantiateArgs, IEngineEnvironmentSettings environmentSettings, InvocationContext context)
{
Expand Down Expand Up @@ -167,7 +212,7 @@ protected async override Task<NewCommandStatus> ExecuteAsync(InstantiateCommandA
}
if (selectedTemplateGroups.Count() > 1)
{
return HandleAmbiguousTemplateGroup(instantiateArgs);
return HandleAmbiguousTemplateGroup(environmentSettings, templatePackageManager, selectedTemplateGroups, Reporter.Error, cancellationToken);
}
return await HandleTemplateInstantationAsync(
instantiateArgs,
Expand All @@ -179,6 +224,38 @@ protected async override Task<NewCommandStatus> ExecuteAsync(InstantiateCommandA

protected override InstantiateCommandArgs ParseContext(ParseResult parseResult) => new(this, parseResult);

private static NewCommandStatus HandleAmbiguousLanguage(
IEngineEnvironmentSettings environmentSettings,
IEnumerable<CliTemplateInfo> templates,
Reporter reporter)
{
reporter.WriteLine(HelpStrings.TableHeader_AmbiguousTemplatesList);
TemplateGroupDisplay.DisplayTemplateList(
environmentSettings,
templates,
new TabularOutputSettings(environmentSettings.Environment),
reporter);
reporter.WriteLine(HelpStrings.Hint_AmbiguousLanguage);
return NewCommandStatus.NotFound;
}

private static NewCommandStatus HandleAmbiguousType(
IEngineEnvironmentSettings environmentSettings,
IEnumerable<CliTemplateInfo> templates,
Reporter reporter)
{
reporter.WriteLine(HelpStrings.TableHeader_AmbiguousTemplatesList);
TemplateGroupDisplay.DisplayTemplateList(
environmentSettings,
templates,
new TabularOutputSettings(
environmentSettings.Environment,
columnsToDisplay: new[] { TabularOutputSettings.ColumnNames.Type }),
reporter);
reporter.WriteLine(HelpStrings.Hint_AmbiguousType);
return NewCommandStatus.NotFound;
}

private async Task<NewCommandStatus> HandleTemplateInstantationAsync(
InstantiateCommandArgs args,
IEngineEnvironmentSettings environmentSettings,
Expand All @@ -195,13 +272,87 @@ private async Task<NewCommandStatus> HandleTemplateInstantationAsync(
}
else if (candidates.Any())
{
return HandleAmbuguousResult();
return HandleAmbiguousResult(
environmentSettings,
templatePackageManager,
candidates.Select(c => c.Template),
Reporter.Error,
cancellationToken);
}

return HandleNoTemplateFoundResult(args, environmentSettings, templatePackageManager, templateGroup, Reporter.Error);
}

private NewCommandStatus HandleAmbuguousResult() => throw new NotImplementedException();
private NewCommandStatus HandleAmbiguousResult(
IEngineEnvironmentSettings environmentSettings,
TemplatePackageManager templatePackageManager,
IEnumerable<CliTemplateInfo> templates,
Reporter reporter,
CancellationToken cancellationToken = default)
{
if (!templates.Any(t => string.IsNullOrWhiteSpace(t.GetLanguage()))
&& !templates.AllAreTheSame(t => t.GetLanguage()))
{
return HandleAmbiguousLanguage(
environmentSettings,
templates,
Reporter.Error);
}

if (!templates.Any(t => string.IsNullOrWhiteSpace(t.GetTemplateType()))
&& !templates.AllAreTheSame(t => t.GetTemplateType()))
{
return HandleAmbiguousType(
environmentSettings,
templates,
Reporter.Error);
}

reporter.WriteLine(LocalizableStrings.AmbiguousTemplatesHeader.Bold().Red());
IEnvironment environment = environmentSettings.Environment;
TabularOutput<CliTemplateInfo> formatter =
TabularOutput.TabularOutput
.For(
new TabularOutputSettings(environment),
templates)
.DefineColumn(t => t.Identity, out object identityColumn, LocalizableStrings.ColumnNameIdentity, showAlways: true)
.DefineColumn(t => t.Name, LocalizableStrings.ColumnNameTemplateName, shrinkIfNeeded: true, minWidth: 15, showAlways: true)
.DefineColumn(t => string.Join(",", t.ShortNameList), LocalizableStrings.ColumnNameShortName, showAlways: true)
.DefineColumn(t => t.GetLanguage(), LocalizableStrings.ColumnNameLanguage, showAlways: true)
.DefineColumn(t => t.Precedence.ToString(), out object prcedenceColumn, LocalizableStrings.ColumnNamePrecedence, showAlways: true)
.DefineColumn(t => t.Author, LocalizableStrings.ColumnNameAuthor, showAlways: true, shrinkIfNeeded: true, minWidth: 10)
.DefineColumn(t => Task.Run(() => GetTemplatePackage(t)).GetAwaiter().GetResult(), LocalizableStrings.ColumnNamePackage, showAlways: true)
.OrderBy(identityColumn, StringComparer.CurrentCultureIgnoreCase)
.OrderByDescending(prcedenceColumn, new NullOrEmptyIsLastStringComparer());
reporter.WriteLine(formatter.Layout().Bold().Red());

reporter.WriteLine(LocalizableStrings.AmbiguousTemplatesMultiplePackagesHint.Bold().Red());
if (templates.AllAreTheSame(t => t.MountPointUri))
{
string templatePackage = Task.Run(() => GetTemplatePackage(templates.First())).GetAwaiter().GetResult();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be doing this somewhat often (.GetAwaiter().GetResult()). Maybe we can refactor some of this later to be async all the way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main limitation for this is that help output is not async. Might be a good improvement to parser itself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a note about this in related item in #2191

if (!string.IsNullOrWhiteSpace(templatePackage))
{
reporter.WriteLine(string.Format(LocalizableStrings.AmbiguousTemplatesSamePackageHint, templatePackage).Bold().Red());
}
}
return NewCommandStatus.NotFound;

async Task<string> GetTemplatePackage(CliTemplateInfo template)
{
try
{
IManagedTemplatePackage? templatePackage =
await template.GetManagedTemplatePackageAsync(templatePackageManager, cancellationToken).ConfigureAwait(false);
return templatePackage?.Identifier ?? string.Empty;
}
catch (Exception ex)
{
environmentSettings.Host.Logger.LogWarning($"Failed to get information about template packages for template group {template.Identity}.");
environmentSettings.Host.Logger.LogDebug($"Details: {ex}.");
return string.Empty;
}
}
}

private HashSet<TemplateCommand> ReparseForTemplate(
InstantiateCommandArgs args,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Microsoft.TemplateEngine.Cli/LocalizableStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ Run 'dotnet {1} --show-aliases' with no args to show all aliases.</value>
<value>Unable to resolve the template, the following installed templates are conflicting:</value>
</data>
<data name="AmbiguousTemplatesMultiplePackagesHint" xml:space="preserve">
<value>Uninstall the templates or the packages to keep only one template from the list.</value>
<value>Uninstall the template packages containing the templates to keep only one template from the list or add the template options which differentiate the template to run.</value>
</data>
<data name="AmbiguousTemplatesSamePackageHint" xml:space="preserve">
<value>The package {0} is not correct, uninstall it and report the issue to the package author.</value>
Expand Down
Loading