Skip to content

Minor syntax fix #5737

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.

Sign up for GitHub

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

Merged
daxian-dbw merged 2 commits into from
Mar 13, 2018
Merged

Minor syntax fix #5737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f9d922
minor syntax fix
kwkam Dec 22, 2017
a813fa9
undo PSReadLine changes
kwkam Dec 23, 2017
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
10 changes: 3 additions & 7 deletions src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4262,7 +4262,7 @@ internal static IEnumerable<CompletionResult> CompleteFilename(CompletionContext
{
var sessionStateInternal = executionContext.EngineSessionState;
completionText = sessionStateInternal.NormalizeRelativePath(path, sessionStateInternal.CurrentLocation.ProviderPath);
string parentDirectory = ".." + Path.DirectorySeparatorChar;
string parentDirectory = ".." + StringLiterals.DefaultPathSeparator;
Copy link
Collaborator

@iSazonov iSazonov Mar 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@daxian-dbw Why we have and keep DefaultPathSeparator duplication in our code?

Copy link
Member

@daxian-dbw daxian-dbw Mar 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know :) Maybe just legacy reasons.

Copy link
Collaborator

@iSazonov iSazonov Mar 13, 2018

Choose a reason for hiding this comment

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

I opened #6373 to track this.

if (!completionText.StartsWith(parentDirectory, StringComparison.Ordinal))
completionText = Path.Combine(".", completionText);
}
Expand Down Expand Up @@ -4477,13 +4477,9 @@ internal static List<CompletionResult> CompleteVariable(CompletionContext contex
var wordToComplete = context.WordToComplete;
var colon = wordToComplete.IndexOf(':');

var prefix = "$";
var lastAst = context.RelatedAsts.Last();
var variableAst = lastAst as VariableExpressionAst;
if (variableAst != null && variableAst.Splatted)
{
prefix = "@";
}
var prefix = variableAst != null && variableAst.Splatted ? "@" : "$";

// Look for variables in the input (e.g. parameters, etc.) before checking session state - these
// variables might not exist in session state yet.
Expand Down Expand Up @@ -5929,7 +5925,7 @@ private static string GetNamespaceToRemove(CompletionContext context, TypeComple
internal static List<CompletionResult> CompleteHelpTopics(CompletionContext context)
{
var results = new List<CompletionResult>();
var dirPath = Utils.GetApplicationBase(Utils.DefaultPowerShellShellID) + Path.DirectorySeparatorChar + CultureInfo.CurrentCulture.Name;
var dirPath = Utils.GetApplicationBase(Utils.DefaultPowerShellShellID) + StringLiterals.DefaultPathSeparator + CultureInfo.CurrentCulture.Name;
var wordToComplete = context.WordToComplete + "*";
var topicPattern = WildcardPattern.Get("about_*.help.txt", WildcardOptions.IgnoreCase);
List<string> files = new List<string>();
Expand Down
2 changes: 1 addition & 1 deletion src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ internal class TypeInferenceVisitor : ICustomAstVisitor2
{
private readonly TypeInferenceContext _context;

private static readonly PSTypeName StringPSTypeName = new PSTypeName(typeof(string));
private static readonly PSTypeName StringPSTypeName = new PSTypeName(typeof(string));

public TypeInferenceVisitor(TypeInferenceContext context)
{
Expand Down
2 changes: 1 addition & 1 deletion src/System.Management.Automation/help/HelpCommentsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ internal XmlDocument BuildXmlFromComments()
// The title is automatically generated
XmlElement title = _doc.CreateElement("maml:title", mamlURI);
string titleStr = string.Format(CultureInfo.InvariantCulture,
" -------------------------- {0} {1} --------------------------",
"\t\t\t\t-------------------------- {0} {1} --------------------------",
Copy link
Collaborator

@iSazonov iSazonov Mar 12, 2018

Choose a reason for hiding this comment

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

Is the use of tab a problem #6313?

Copy link
Member

@daxian-dbw daxian-dbw Mar 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Don't know. For this specific change, it keeps the original behavior because the removed whitespaces are essentially 4 tabs.

Copy link
Collaborator

@iSazonov iSazonov Mar 13, 2018

Choose a reason for hiding this comment

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

@SteveL-MSFT Could you please comment - is this related to #6313?

Copy link
Member

@daxian-dbw daxian-dbw Mar 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@iSazonov This is not a blocking comment, right? This change itself doesn't alter the original behavior. Even if the use of tab here is proven to be the cause of #6313, I think it should be dealt with in a separate PR.

Copy link
Member

@SteveL-MSFT SteveL-MSFT Mar 13, 2018

Choose a reason for hiding this comment

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

Tabs in general are problematic as their size depends on the console. Try this on macOS or Linux:

tabs 6,10,12
"a`tb`tc`td"

If the intent of tabs in strings is to indent, I think it's better to just use four spaces. This PR is unrelated to #6313 expect for both related to behavior of tabs at the console.

Copy link
Member

@daxian-dbw daxian-dbw Mar 13, 2018

Choose a reason for hiding this comment

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

I opened #6381 to track the review of the use of '\t' here.

HelpDisplayStrings.ExampleUpperCase, count++);
XmlText title_text = _doc.CreateTextNode(titleStr);
example_node.AppendChild(title).AppendChild(title_text);
Expand Down
8 changes: 4 additions & 4 deletions src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4996,13 +4996,13 @@ protected override string NormalizeRelativePath(
try
{
string originalPathComparison = path;
if (!originalPathComparison.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase))
if (!originalPathComparison.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase))
{
originalPathComparison += StringLiterals.DefaultPathSeparator;
}

string basePathComparison = basePath;
if (!basePathComparison.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase))
if (!basePathComparison.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase))
{
basePathComparison += StringLiterals.DefaultPathSeparator;
}
Expand Down Expand Up @@ -5205,7 +5205,7 @@ private string NormalizeRelativePathHelper(string path, string basePath)
// See if the base and the path are already the same. We resolve this to
// ..\Leaf, since resolving "." to "." doesn't offer much information.
if (String.Equals(path, basePath, StringComparison.OrdinalIgnoreCase) &&
(!originalPath.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase)))
(!originalPath.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase)))
{
string childName = GetChildName(path);
result = MakePath("..", childName);
Expand Down Expand Up @@ -5252,7 +5252,7 @@ private string NormalizeRelativePathHelper(string path, string basePath)
if (!String.IsNullOrEmpty(commonBase))
{
if (String.Equals(path, commonBase, StringComparison.OrdinalIgnoreCase) &&
(!path.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase)))
(!path.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase)))
{
string childName = GetChildName(path);
result = MakePath("..", result);
Expand Down
4 changes: 2 additions & 2 deletions src/System.Management.Automation/namespaces/NavigationProviderBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ internal string ContractRelativePath(
// See if the base and the path are already the same. We resolve this to
// ..\Leaf, since resolving "." to "." doesn't offer much information.
if (String.Equals(normalizedPath, normalizedBasePath, StringComparison.OrdinalIgnoreCase) &&
(!originalPath.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase)))
(!originalPath.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase)))
{
string childName = GetChildName(path);
result = MakePath("..", childName);
Expand Down Expand Up @@ -705,7 +705,7 @@ internal string ContractRelativePath(
if (!String.IsNullOrEmpty(commonBase))
{
if (String.Equals(normalizedPath, commonBase, StringComparison.OrdinalIgnoreCase) &&
(!normalizedPath.EndsWith("" + StringLiterals.DefaultPathSeparator, StringComparison.OrdinalIgnoreCase)))
(!normalizedPath.EndsWith(StringLiterals.DefaultPathSeparatorString, StringComparison.OrdinalIgnoreCase)))
{
string childName = GetChildName(path);
result = MakePath("..", result);
Expand Down