From bfe0c465c62a07646b0cb6f12ecb790005d147ab Mon Sep 17 00:00:00 2001 From: seancpeters Date: Tue, 11 Apr 2017 16:48:53 -0700 Subject: [PATCH] Changes per Mike's informal review. Fixed output when trying to remove an alias that doesn't exist. --- Microsoft.TemplateEngine.sln | 2 +- .../AliasSupport.cs | 93 ++++++++++++++++--- .../LocalizableStrings.Designer.cs | 93 +++++++++++++++++-- .../LocalizableStrings.resx | 31 ++++++- .../New3Command.cs | 16 +--- .../Settings/AliasManipulationStatus.cs | 1 + .../Settings/AliasRegistry.cs | 37 ++------ 7 files changed, 204 insertions(+), 69 deletions(-) diff --git a/Microsoft.TemplateEngine.sln b/Microsoft.TemplateEngine.sln index 2b8a6343dbc..e2c16a5435a 100644 --- a/Microsoft.TemplateEngine.sln +++ b/Microsoft.TemplateEngine.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 -VisualStudioVersion = 15.0.26214.0 +VisualStudioVersion = 15.0.26312.0 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{7DAC892E-ADAE-4CEB-8A0C-EDC452A5826A}" EndProject diff --git a/src/Microsoft.TemplateEngine.Cli/AliasSupport.cs b/src/Microsoft.TemplateEngine.Cli/AliasSupport.cs index 0cee8bdfa5e..eb503e3aa2e 100644 --- a/src/Microsoft.TemplateEngine.Cli/AliasSupport.cs +++ b/src/Microsoft.TemplateEngine.Cli/AliasSupport.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; using Microsoft.TemplateEngine.Abstractions; using Microsoft.TemplateEngine.Cli.CommandParsing; using Microsoft.TemplateEngine.Edge.Settings; @@ -28,33 +30,68 @@ public static AliasExpansionStatus TryExpandAliases(INewCommandInput commandInpu return AliasExpansionStatus.ExpansionError; } - public static CreationResultStatus ManipulateAliasValue(INewCommandInput commandInput, AliasRegistry aliasRegistry) + // Matches on any non-word character (letter, number, or underscore) + // Almost the same as \W, except \W has some quirks with unicode characters, and we allow '.' + private static readonly Regex InvalidAliasRegex = new Regex("[^a-z0-9_.]", RegexOptions.IgnoreCase); + // The first token must be a valid template short name. This naively tests for it by checking the first character. + // TODO: make this test more robust. + private static readonly Regex ValidFirstTokenRegex = new Regex("^[a-z0-9]", RegexOptions.IgnoreCase); + + public static CreationResultStatus ManipulateAliasIfValid(AliasRegistry aliasRegistry, string aliasName, List inputTokens, HashSet reservedAliasNames) { - List inputTokens = commandInput.Tokens.ToList(); + if (reservedAliasNames.Contains(aliasName)) + { + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasCannotBeShortName, aliasName)); + return CreationResultStatus.CreateFailed; + } + else if (InvalidAliasRegex.IsMatch(aliasName)) + { + Reporter.Output.WriteLine(LocalizableStrings.AliasNameContainsInvalidCharacters); // TODO - change this string + return CreationResultStatus.InvalidParamValues; + } + inputTokens.RemoveAt(0); // remove the command name - AliasManipulationResult result = aliasRegistry.TryCreateOrRemoveAlias(inputTokens); + IReadOnlyList aliasTokens = FilterForAliasTokens(inputTokens); // remove '-a' or '--alias', and the alias name + + // The first token refers to a template name, or another alias. + if (aliasTokens.Count > 0 && !ValidFirstTokenRegex.IsMatch(aliasTokens[0])) + { + Reporter.Output.WriteLine(LocalizableStrings.AliasValueFirstArgError); + return CreationResultStatus.InvalidParamValues; + } + + // create, update, or delete an alias. + return ManipulateAliasValue(aliasName, aliasTokens, aliasRegistry); + } + + private static CreationResultStatus ManipulateAliasValue(string aliasName, IReadOnlyList aliasTokens, AliasRegistry aliasRegistry) + { + AliasManipulationResult result = aliasRegistry.TryCreateOrRemoveAlias(aliasName, aliasTokens); CreationResultStatus returnStatus = CreationResultStatus.OperationNotSpecified; switch (result.Status) { case AliasManipulationStatus.Created: - Reporter.Output.WriteLine(string.Format("Successfully created alias named '{0}' with value '{1}'", result.AliasName, result.AliasValue)); + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasCreated, result.AliasName, result.AliasValue)); returnStatus = CreationResultStatus.Success; break; case AliasManipulationStatus.Removed: - Reporter.Output.WriteLine(string.Format("Successfully removed alias named '{0}' whose value was '{1}'", result.AliasName, result.AliasValue)); + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasRemoved, result.AliasName, result.AliasValue)); returnStatus = CreationResultStatus.Success; break; + case AliasManipulationStatus.RemoveNonExistentFailed: + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasRemoveNonExistentFailed, result.AliasName)); + break; case AliasManipulationStatus.Updated: - Reporter.Output.WriteLine(string.Format("Successfully updated alias named '{0}' to value '{1}'", result.AliasName, result.AliasValue)); + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasUpdated, result.AliasName, result.AliasValue)); returnStatus = CreationResultStatus.Success; break; case AliasManipulationStatus.WouldCreateCycle: - Reporter.Output.WriteLine(string.Format("Alias not created. It would have created an alias cycle, resulting in infinite expansion.")); + Reporter.Output.WriteLine(LocalizableStrings.AliasCycleError); returnStatus = CreationResultStatus.CreateFailed; break; case AliasManipulationStatus.InvalidInput: - Reporter.Output.WriteLine(string.Format("Alias not created. The input was invalid")); + Reporter.Output.WriteLine(LocalizableStrings.AliasNotCreatedInvalidInput); returnStatus = CreationResultStatus.InvalidParamValues; break; } @@ -62,6 +99,40 @@ public static CreationResultStatus ManipulateAliasValue(INewCommandInput command return returnStatus; } + private static IReadOnlyList FilterForAliasTokens(IReadOnlyList inputTokens) + { + List aliasTokens = new List(); + bool nextIsAliasName = false; + string aliasName = null; + + foreach (string token in inputTokens) + { + if (nextIsAliasName) + { + aliasName = token; + nextIsAliasName = false; + } + else if (string.Equals(token, "-a", StringComparison.Ordinal) || string.Equals(token, "--alias", StringComparison.Ordinal)) + { + if (!string.IsNullOrEmpty(aliasName)) + { + // found multiple alias names, which is invalid. + aliasTokens.Clear(); + aliasName = null; + return aliasTokens; + } + + nextIsAliasName = true; + } + else if (!token.StartsWith("--debug:", StringComparison.Ordinal)) + { + aliasTokens.Add(token); + } + } + + return aliasTokens; + } + public static CreationResultStatus DisplayAliasValues(IEngineEnvironmentSettings environment, INewCommandInput commandInput, AliasRegistry aliasRegistry) { IReadOnlyDictionary aliasesToShow; @@ -77,14 +148,14 @@ public static CreationResultStatus DisplayAliasValues(IEngineEnvironmentSettings } else { - Reporter.Output.WriteLine(string.Format("Unknown alias name '{0}'\nRun 'dotnet --show-aliases' with no args to show all aliases.", commandInput.ShowAliasesAliasName)); + Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasShowErrorUnknownAlias, commandInput.ShowAliasesAliasName)); return CreationResultStatus.InvalidParamValues; } } else { aliasesToShow = aliasRegistry.AllAliases; - Reporter.Output.WriteLine("All Aliases:"); + Reporter.Output.WriteLine(LocalizableStrings.AliasShowAllAliasesHeader); } HelpFormatter> formatter = new HelpFormatter>(environment, aliasesToShow, 2, '-', false) diff --git a/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.Designer.cs b/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.Designer.cs index fc24232fa8e..ec6c8956532 100644 --- a/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.Designer.cs +++ b/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.Designer.cs @@ -70,15 +70,6 @@ public static string Alias { } } - /// - /// Looks up a localized string similar to Alias names cannot begin with dashes ('-').. - /// - public static string AliasCannotBeginWithDashes { - get { - return ResourceManager.GetString("AliasCannotBeginWithDashes", resourceCulture); - } - } - /// /// Looks up a localized string similar to Alias '{0}' is a template short name, and therefore cannot be aliased.. /// @@ -99,7 +90,7 @@ public static string AliasCommandAfterExpansion { } /// - /// Looks up a localized string similar to Alias creation successful.. + /// Looks up a localized string similar to Successfully created alias named '{0}' with value '{1}'. /// public static string AliasCreated { get { @@ -107,6 +98,15 @@ public static string AliasCreated { } } + /// + /// Looks up a localized string similar to Alias not created. It would have created an alias cycle, resulting in infinite expansion.. + /// + public static string AliasCycleError { + get { + return ResourceManager.GetString("AliasCycleError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Command is invalid after expanding aliases.. /// @@ -143,6 +143,70 @@ public static string AliasName { } } + /// + /// Looks up a localized string similar to Alias names can only contain letters, numbers, underscores, and periods.. + /// + public static string AliasNameContainsInvalidCharacters { + get { + return ResourceManager.GetString("AliasNameContainsInvalidCharacters", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Alias not created. The input was invalid.. + /// + public static string AliasNotCreatedInvalidInput { + get { + return ResourceManager.GetString("AliasNotCreatedInvalidInput", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Successfully removed alias named '{0}' whose value was '{1}'.. + /// + public static string AliasRemoved { + get { + return ResourceManager.GetString("AliasRemoved", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unable to remove alias '{0}'. It did not exist.. + /// + public static string AliasRemoveNonExistentFailed { + get { + return ResourceManager.GetString("AliasRemoveNonExistentFailed", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to All Aliases:. + /// + public static string AliasShowAllAliasesHeader { + get { + return ResourceManager.GetString("AliasShowAllAliasesHeader", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unknown alias name '{0}'. + ///Run 'dotnet --show-aliases' with no args to show all aliases.. + /// + public static string AliasShowErrorUnknownAlias { + get { + return ResourceManager.GetString("AliasShowErrorUnknownAlias", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Successfully updated alias named '{0}' to value '{1}'.. + /// + public static string AliasUpdated { + get { + return ResourceManager.GetString("AliasUpdated", resourceCulture); + } + } + /// /// Looks up a localized string similar to Alias Value. /// @@ -152,6 +216,15 @@ public static string AliasValue { } } + /// + /// Looks up a localized string similar to First argument of an alias value must be a letter or digit.. + /// + public static string AliasValueFirstArgError { + get { + return ResourceManager.GetString("AliasValueFirstArgError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Do not allow scripts to run. /// diff --git a/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.resx b/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.resx index dcb2fa0794f..f7c597af40d 100644 --- a/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.resx +++ b/src/Microsoft.TemplateEngine.Cli/LocalizableStrings.resx @@ -121,7 +121,7 @@ Alias - Alias creation successful. + Successfully created alias named '{0}' with value '{1}' Unable to determine the desired template from the input template name: {0}. @@ -443,8 +443,8 @@ Displays the value of the named alias, or all aliases if no name is specified. - - Alias names cannot begin with dashes ('-'). + + Alias names can only contain letters, numbers, underscores, and periods. Alias '{0}' is a template short name, and therefore cannot be aliased. @@ -463,4 +463,29 @@ After expanding the extra args files, the command is: dotnet {0} + + Alias not created. It would have created an alias cycle, resulting in infinite expansion. + + + Alias not created. The input was invalid. + + + Successfully removed alias named '{0}' whose value was '{1}'. + + + All Aliases: + + + Unknown alias name '{0}'. +Run 'dotnet --show-aliases' with no args to show all aliases. + + + Successfully updated alias named '{0}' to value '{1}'. + + + First argument of an alias value must be a letter or digit. + + + Unable to remove alias '{0}'. It did not exist. + \ No newline at end of file diff --git a/src/Microsoft.TemplateEngine.Cli/New3Command.cs b/src/Microsoft.TemplateEngine.Cli/New3Command.cs index 3cf860b431c..5fc0a403f81 100644 --- a/src/Microsoft.TemplateEngine.Cli/New3Command.cs +++ b/src/Microsoft.TemplateEngine.Cli/New3Command.cs @@ -968,22 +968,8 @@ private async Task ExecuteAsync() { if (!string.IsNullOrEmpty(_commandInput.Alias) && !_commandInput.IsHelpFlagSpecified) { - // Check that the alias name is not the short name of a template. - if (AllTemplateShortNames.Contains(_commandInput.Alias)) - { - Reporter.Output.WriteLine(string.Format(LocalizableStrings.AliasCannotBeShortName, _commandInput.Alias)); - return CreationResultStatus.CreateFailed; - } - else if (_commandInput.Alias.StartsWith("-")) - { - Reporter.Output.WriteLine(LocalizableStrings.AliasCannotBeginWithDashes); - return CreationResultStatus.InvalidParamValues; - } - - // create, update, or delete an alias. - return AliasSupport.ManipulateAliasValue(_commandInput, _aliasRegistry); + return AliasSupport.ManipulateAliasIfValid(_aliasRegistry, _commandInput.Alias, _commandInput.Tokens.ToList(), AllTemplateShortNames); } - // TODO try expanding aliases... then redo everything except checking for alias expansion. if (string.IsNullOrWhiteSpace(TemplateName)) { diff --git a/src/Microsoft.TemplateEngine.Edge/Settings/AliasManipulationStatus.cs b/src/Microsoft.TemplateEngine.Edge/Settings/AliasManipulationStatus.cs index fc665999115..1f20bb7a6ed 100644 --- a/src/Microsoft.TemplateEngine.Edge/Settings/AliasManipulationStatus.cs +++ b/src/Microsoft.TemplateEngine.Edge/Settings/AliasManipulationStatus.cs @@ -5,6 +5,7 @@ public enum AliasManipulationStatus { Created, Removed, + RemoveNonExistentFailed, // for trying to remove an alias that didn't exist. Updated, WouldCreateCycle, InvalidInput diff --git a/src/Microsoft.TemplateEngine.Edge/Settings/AliasRegistry.cs b/src/Microsoft.TemplateEngine.Edge/Settings/AliasRegistry.cs index f6dbddf6683..da4c4569c43 100644 --- a/src/Microsoft.TemplateEngine.Edge/Settings/AliasRegistry.cs +++ b/src/Microsoft.TemplateEngine.Edge/Settings/AliasRegistry.cs @@ -7,7 +7,7 @@ namespace Microsoft.TemplateEngine.Edge.Settings { public class AliasRegistry { - private AliasModel _aliases { get; set; } + private AliasModel _aliases; private readonly IEngineEnvironmentSettings _environmentSettings; private readonly Paths _paths; @@ -58,35 +58,10 @@ private void Save() _environmentSettings.Host.FileSystem.WriteAllText(_paths.User.AliasesFile, serialized.ToString()); } - public AliasManipulationResult TryCreateOrRemoveAlias(IReadOnlyList inputTokens) + public AliasManipulationResult TryCreateOrRemoveAlias(string aliasName, IReadOnlyList aliasTokens) { EnsureLoaded(); - IList aliasTokens = new List(); - bool nextIsAliasName = false; - string aliasName = null; - - foreach (string token in inputTokens) - { - if (nextIsAliasName) - { - aliasName = token; - nextIsAliasName = false; - } - else if (string.Equals(token, "-a", StringComparison.Ordinal) || string.Equals(token, "--alias", StringComparison.Ordinal)) - { - if (!string.IsNullOrEmpty(aliasName)) - { - return new AliasManipulationResult(AliasManipulationStatus.InvalidInput); - } - nextIsAliasName = true; - } - else if (!string.Equals(token, "--debug:attach", StringComparison.Ordinal)) - { - aliasTokens.Add(token); - } - } - if (aliasName == null) { // the input was malformed. Alias flag without alias name @@ -98,8 +73,12 @@ public AliasManipulationResult TryCreateOrRemoveAlias(IReadOnlyList inpu if (_aliases.TryRemoveCommandAlias(aliasName, out string removedAliasValue)) { Save(); + return new AliasManipulationResult(AliasManipulationStatus.Removed, aliasName, removedAliasValue); + } + else + { + return new AliasManipulationResult(AliasManipulationStatus.RemoveNonExistentFailed, aliasName, string.Empty); } - return new AliasManipulationResult(AliasManipulationStatus.Removed, aliasName, removedAliasValue); } string aliasValue = string.Join(" ", aliasTokens); @@ -107,7 +86,6 @@ public AliasManipulationResult TryCreateOrRemoveAlias(IReadOnlyList inpu aliasesWithCandidate[aliasName] = aliasValue; if (!TryExpandCommandAliases(aliasesWithCandidate, aliasValue, out string expandedInput)) { - // TODO: more meaningful return info... alias would create a cycle return new AliasManipulationResult(AliasManipulationStatus.WouldCreateCycle, aliasName, aliasValue); } @@ -130,6 +108,7 @@ public bool TryExpandCommandAliases(IReadOnlyList inputTokens, out IRead string input = string.Join(" ", inputTokens); if (TryExpandCommandAliases(_aliases.CommandAliases, input, out string expandedInput)) { + // splits on any whitespace (odd usage, but documented on msdn) expandedInputTokens = expandedInput.Split((char[])null, StringSplitOptions.RemoveEmptyEntries); return true; }