-
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
[system-command-line] error handling for ambiguous results on template instantiation #4227
[system-command-line] error handling for ambiguous results on template instantiation #4227
Conversation
803fb52
to
e72ce4c
Compare
693b819
to
b4219f8
Compare
reporter.WriteLine(LocalizableStrings.AmbiguousTemplatesMultiplePackagesHint.Bold().Red()); | ||
if (templates.AllAreTheSame(t => t.MountPointUri)) | ||
{ | ||
string templatePackage = Task.Run(() => GetTemplatePackage(templates.First())).GetAwaiter().GetResult(); |
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.
We seem to be doing this somewhat often (.GetAwaiter().GetResult()
). Maybe we can refactor some of this later to be async
all the way.
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.
Main limitation for this is that help output is not async. Might be a good improvement to parser itself.
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.
I've made a note about this in related item in #2191
Co-authored-by: Bekir Ozturk <60651445+bekir-ozturk@users.noreply.github.com>
Problem
#3809:
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:
#nullable enable
to all the modified files ?