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

Conversation

vlada-shubina
Copy link
Member

Problem

#3809:

  • error handling for ambiguous template group
  • error handling for ambiguous template

Solution

Ported implementation from old parser, no major differences from previous UX.
Only improvement: now in case of ambiguous template group hint for uninstalling the package is shown.

Checks:

  • Added unit tests
  • Added #nullable enable to all the modified files ?

@vlada-shubina vlada-shubina requested a review from a team as a code owner December 23, 2021 08:14
@vlada-shubina vlada-shubina changed the title Ambiguous results [system-command-line] error handling for ambiguous results on template instantiation Dec 23, 2021
@vlada-shubina vlada-shubina force-pushed the feature/system-command-line branch from 803fb52 to e72ce4c Compare December 23, 2021 12:24
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

Co-authored-by: Bekir Ozturk <60651445+bekir-ozturk@users.noreply.github.com>
@vlada-shubina vlada-shubina merged commit 65befb5 into dotnet:feature/system-command-line Dec 23, 2021
@vlada-shubina vlada-shubina deleted the ambiguous-results branch December 23, 2021 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants