admin/PowerShell-PowerShell
1
0
Fork 0
mirror of https://github.com/PowerShell/PowerShell synced 2026-06-08 12:12:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Feature] Address code review comments

Browse Source
This commit is contained in:
Aditya Patwardhan
2018-07-12 17:17:50 -07:00
parent 731afbf14d
commit e711c5dcc3
11 changed files with 212 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFromMarkdownCommand.cs
+38 -8
View File
@@ -8,6 +8,7 @@ using System.IO;
using System.Management.Automation;
using System.Management.Automation.Internal;
using System.Threading.Tasks;
using System.Security;
using Microsoft.PowerShell.MarkdownRender;
using Dbg = System.Management.Automation;
@@ -58,7 +59,7 @@ namespace Microsoft.PowerShell.Commands
private MarkdownOptionInfo mdOption = null;
/// <summary>
/// Override BeginProcessing.
/// Read the MarkdownOptionInfo set in SessionState.
/// </summary>
protected override void BeginProcessing()
{
@@ -89,7 +90,7 @@ namespace Microsoft.PowerShell.Commands
{
WriteObject(
MarkdownConverter.Convert(
ReadContentFromFile(fileInfo.FullName).Result,
ReadContentFromFile(fileInfo.FullName)?.Result,
conversionType,
mdOption));
}
@@ -131,7 +132,7 @@ namespace Microsoft.PowerShell.Commands
{
WriteObject(
MarkdownConverter.Convert(
ReadContentFromFile(resolvedPath).Result,
ReadContentFromFile(resolvedPath)?.Result,
conversionType,
optionInfo));
}
@@ -140,13 +141,43 @@ namespace Microsoft.PowerShell.Commands
private async Task<string> ReadContentFromFile(string filePath)
{
Dbg.Diagnostics.Assert(File.Exists(filePath), "Caller should make sure the file exists.");
ErrorRecord errorRecord = null;
using (StreamReader reader = new StreamReader(new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read)))
try
{
string mdContent = await reader.ReadToEndAsync();
return mdContent;
using (StreamReader reader = new StreamReader(new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read)))
{
string mdContent = await reader.ReadToEndAsync();
return mdContent;
}
}
catch(FileNotFoundException fnfe)
{
errorRecord = new ErrorRecord(
fnfe,
"FileNotFound",
ErrorCategory.ResourceUnavailable,
filePath);
}
catch(SecurityException se)
{
errorRecord = new ErrorRecord(
se,
"FileSecurityError",
ErrorCategory.SecurityError,
filePath);
}
catch(UnauthorizedAccessException uae)
{
errorRecord = new ErrorRecord(
uae,
"FileUnauthorizedAccess",
ErrorCategory.SecurityError,
filePath);
}
WriteError(errorRecord);
return null;
}
private List<string> ResolvePath(string path, bool isLiteral)
@@ -168,7 +199,6 @@ namespace Microsoft.PowerShell.Commands
}
catch (ItemNotFoundException infe)
{
string errorMessage = StringUtil.Format(ConvertMarkdownStrings.InputFileNotFound, path);
var errorRecord = new ErrorRecord(
infe,
"FileNotFound",
src/Microsoft.PowerShell.Commands.Utility/commands/utility/MarkdownOptionCommands.cs
+8 -1
View File
@@ -6,6 +6,7 @@ using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Management.Automation;
using System.Management.Automation.Internal;
using System.Threading.Tasks;
using Microsoft.PowerShell.MarkdownRender;
@@ -154,7 +155,13 @@ namespace Microsoft.PowerShell.Commands
if (mdOptionInfo == null)
{
throw new ArgumentException();
var errorMessage = StringUtil.Format(ConvertMarkdownStrings.InvalidInputObjectType, baseObj.GetType());
ErrorRecord errorRecord = new ErrorRecord(
new ArgumentException(errorMessage),
"InvalidObject",
ErrorCategory.InvalidArgument,
InputObject);
}
break;
src/Microsoft.PowerShell.Commands.Utility/commands/utility/ShowMarkdownCommand.cs
+51 -34
View File
@@ -44,7 +44,8 @@ namespace Microsoft.PowerShell.Commands
/// </summary>
protected override void BeginProcessing()
{
if (!this.MyInvocation.BoundParameters.ContainsKey("UseBrowser"))
//if (!this.MyInvocation.BoundParameters.ContainsKey("UseBrowser"))
if(!UseBrowser.IsPresent)
{
// Since UseBrowser is not bound, we use proxy to Out-Default
stepPipe = ScriptBlock.Create(@"Microsoft.PowerShell.Core\Out-Default @PSBoundParameters").GetSteppablePipeline(this.MyInvocation.CommandOrigin);
@@ -58,20 +59,8 @@ namespace Microsoft.PowerShell.Commands
protected override void ProcessRecord()
{
object inpObj = InputObject.BaseObject;
var markdownInfo = inpObj as MarkdownInfo;
if (markdownInfo == null)
{
string errorMessage = StringUtil.Format(ConvertMarkdownStrings.InvalidInputObjectType, inpObj.GetType());
var errorRecord = new ErrorRecord(
new ArgumentException(errorMessage),
"InvalidInputObject",
ErrorCategory.InvalidArgument,
InputObject);
WriteError(errorRecord);
}
else
if(inpObj is MarkdownInfo markdownInfo)
{
if (UseBrowser)
{
@@ -80,21 +69,50 @@ namespace Microsoft.PowerShell.Commands
if (!string.IsNullOrEmpty(html))
{
string tmpFilePath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString() + ".html");
using (var writer = new StreamWriter(new FileStream(tmpFilePath, FileMode.Create, FileAccess.Write, FileShare.Write)))
try
{
writer.Write(html);
using (var writer = new StreamWriter(new FileStream(tmpFilePath, FileMode.Create, FileAccess.Write, FileShare.Write)))
{
writer.Write(html);
}
}
catch (Exception e)
{
var errorRecord = new ErrorRecord(
e,
"ErrorWritingTempFile",
ErrorCategory.WriteError,
tmpFilePath);
WriteError(errorRecord);
return;
}
if (outputBypassTestHook)
if (InternalTestHooks.ShowMarkdownOutputBypass)
{
WriteObject(html);
return;
}
ProcessStartInfo startInfo = new ProcessStartInfo();
startInfo.FileName = tmpFilePath;
startInfo.UseShellExecute = true;
Process.Start(startInfo);
try
{
ProcessStartInfo startInfo = new ProcessStartInfo();
startInfo.FileName = tmpFilePath;
startInfo.UseShellExecute = true;
Process.Start(startInfo);
}
catch (Exception e)
{
var errorRecord = new ErrorRecord(
e,
"ErrorLaunchingDefaultApplication",
ErrorCategory.InvalidOperation,
targetObject : null);
WriteError(errorRecord);
return;
}
}
else
{
@@ -114,7 +132,7 @@ namespace Microsoft.PowerShell.Commands
if (!string.IsNullOrEmpty(vt100String))
{
if (outputBypassTestHook)
if (InternalTestHooks.ShowMarkdownOutputBypass)
{
WriteObject(vt100String);
return;
@@ -138,6 +156,17 @@ namespace Microsoft.PowerShell.Commands
}
}
}
else
{
string errorMessage = StringUtil.Format(ConvertMarkdownStrings.InvalidInputObjectType, inpObj.GetType());
var errorRecord = new ErrorRecord(
new ArgumentException(errorMessage),
"InvalidInputObject",
ErrorCategory.InvalidArgument,
InputObject);
WriteError(errorRecord);
}
}
/// <summary>
@@ -150,17 +179,5 @@ namespace Microsoft.PowerShell.Commands
stepPipe.End();
}
}
private static bool outputBypassTestHook = false;
/// <summary>
/// Test hook to enable or disable launching of browser.
/// When set, the converted output is returned.
/// </summary>
/// <param name="value">True to enable test hook, false to disable.</param>
public static void SetOutputBypassTestHook(bool value)
{
outputBypassTestHook = value;
}
}
}
src/Microsoft.PowerShell.Commands.Utility/resources/ConvertMarkdownStrings.resx
+2 -2
View File
@@ -121,10 +121,10 @@
<value>The type of the input object '{0}' is invalid.</value>
</data>
<data name="InputFileNotFound" xml:space="preserve">
<value>The given file path '{0}' is not found.</value>
<value>The file is not found: '{0}'.</value>
</data>
<data name="FileSystemPathsOnly" xml:space="preserve">
<value>Only FileSystem Provider paths are supported. The given path '{0}' is not supported.</value>
<value>Only FileSystem Provider paths are supported. The file path is not supported: '{0}'.</value>
</data>
<data name="MarkdownInfoInvalid" xml:space="preserve">
<value>The property {0} of the given object is null or empty.</value>
src/Microsoft.PowerShell.MarkdownRender/FencedCodeBlockRenderer.cs
+1 -1
View File
@@ -24,7 +24,7 @@ namespace Microsoft.PowerShell.MarkdownRender
// This specifically helps for parameters help content.
if (string.Equals(obj.Info, "yaml", StringComparison.OrdinalIgnoreCase))
{
renderer.WriteLine("\t" + codeLine.ToString());
renderer.Write("\t").WriteLine(codeLine.ToString());
}
else
{
src/Microsoft.PowerShell.MarkdownRender/ListItemBlockRenderer.cs
+3 -8
View File
@@ -17,9 +17,7 @@ namespace Microsoft.PowerShell.MarkdownRender
{
protected override void Write(VT100Renderer renderer, ListItemBlock obj)
{
var parent = obj.Parent as ListBlock;
if (parent != null)
if (obj.Parent is ListBlock parent)
{
if (!parent.IsOrdered)
{
@@ -36,17 +34,14 @@ namespace Microsoft.PowerShell.MarkdownRender
// Indent left by 2 for each level on list.
string indent = Padding(indentLevel * 2);
var paragraphBlock = block as ParagraphBlock;
if (paragraphBlock != null)
if (block is ParagraphBlock paragraphBlock)
{
renderer.Write(indent).Write(listBullet).Write(" ").Write(paragraphBlock.Inline);
}
else
{
// If there is a sublist, the block is a ListBlock instead of ParagraphBlock.
var subList = block as ListBlock;
if (subList != null)
if (block is ListBlock subList)
{
foreach (var subListItem in subList)
{
src/Microsoft.PowerShell.MarkdownRender/Microsoft.PowerShell.MarkdownRender.csproj
-16
View File
@@ -6,22 +6,6 @@
<AssemblyName>Microsoft.PowerShell.MarkdownRender</AssemblyName>
</PropertyGroup>
<!--PropertyGroup>
<DefineConstants>$(DefineConstants);CORECLR</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Debug' ">
<DebugType>portable</DebugType>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Linux' ">
<DefineConstants>$(DefineConstants);UNIX</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'CodeCoverage' ">
<DebugType>full</DebugType>
</PropertyGroup-->
<ItemGroup>
<!-- Source: https://github.com/lunet-io/markdig/ -->
<PackageReference Include="Markdig.Signed" Version="0.14.9" />
src/Microsoft.PowerShell.MarkdownRender/VT100EscapeSequences.cs
+97 -33
View File
@@ -15,6 +15,7 @@ namespace Microsoft.PowerShell.MarkdownRender
public sealed class MarkdownOptionInfo
{
private const char Esc = (char)0x1b;
private const string EndSequence = "[0m";
/// <summary>
/// Gets or sets current VT100 escape sequence for header 1.
@@ -79,16 +80,48 @@ namespace Microsoft.PowerShell.MarkdownRender
/// <returns>Specified property name as escape sequence.</returns>
public string AsEscapeSequence(string propertyName)
{
var propertyValue = this.GetType().GetProperty(propertyName)?.GetValue(this) as string;
var propName = propertyName?.ToLower();
if (!string.IsNullOrEmpty(propertyValue))
switch(propName)
{
return string.Concat(Esc, propertyValue, propertyValue, Esc, "[0m");
}
else
{
throw new InvalidOperationException();
case "header1":
return string.Concat(Esc, Header1, Header1, Esc, EndSequence);
case "header2":
return string.Concat(Esc, Header2, Header2, Esc, EndSequence);
case "header3":
return string.Concat(Esc, Header3, Header3, Esc, EndSequence);
case "header4":
return string.Concat(Esc, Header4, Header4, Esc, EndSequence);
case "header5":
return string.Concat(Esc, Header5, Header5, Esc, EndSequence);
case "header6":
return string.Concat(Esc, Header6, Header6, Esc, EndSequence);
case "code":
return string.Concat(Esc, Code, Code, Esc, EndSequence);
case "link":
return string.Concat(Esc, Link, Link, Esc, EndSequence);
case "image":
return string.Concat(Esc, Image, Image, Esc, EndSequence);
case "emphasisbold":
return string.Concat(Esc, EmphasisBold, EmphasisBold, Esc, EndSequence);
case "emphasisitalics":
return string.Concat(Esc, EmphasisItalics, EmphasisItalics, Esc, EndSequence);
default:
break;
}
return null;
}
/// <summary>
@@ -99,22 +132,46 @@ namespace Microsoft.PowerShell.MarkdownRender
SetDarkTheme();
}
private const string Header1Dark = "[7m";
private const string Header2Dark = "[4;93m";
private const string Header3Dark = "[4;94m";
private const string Header4Dark = "[4;95m";
private const string Header5Dark = "[4;96m";
private const string Header6Dark = "[4;97m";
private const string CodeDark = "[48;2;155;155;155;38;2;30;30;30m";
private const string LinkDark = "[4;38;5;117m";
private const string ImageDark = "[33m";
private const string EmphasisBoldDark = "[1m";
private const string EmphasisItalicsDark = "[36m";
private const string Header1Light = "[7m";
private const string Header2Light = "[4;33m";
private const string Header3Light = "[4;34m";
private const string Header4Light = "[4;35m";
private const string Header5Light = "[4;36m";
private const string Header6Light = "[4;30m";
private const string CodeLight = "[48;2;155;155;155;38;2;30;30;30m";
private const string LinkLight = "[4;38;5;117m";
private const string ImageLight = "[33m";
private const string EmphasisBoldLight = "[1m";
private const string EmphasisItalicsLight = "[36m";
/// <summary>
/// Set all preference for dark theme.
/// </summary>
public void SetDarkTheme()
{
Header1 = "[7m";
Header2 = "[4;93m";
Header3 = "[4;94m";
Header4 = "[4;95m";
Header5 = "[4;96m";
Header6 = "[4;97m";
Code = "[48;2;155;155;155;38;2;30;30;30m";
Link = "[4;38;5;117m";
Image = "[33m";
EmphasisBold = "[1m";
EmphasisItalics = "[36m";
Header1 = Header1Dark;
Header2 = Header2Dark;
Header3 = Header3Dark;
Header4 = Header4Dark;
Header5 = Header5Dark;
Header6 = Header6Dark;
Code = CodeDark;
Link = LinkDark;
Image = ImageDark;
EmphasisBold = EmphasisBoldDark;
EmphasisItalics = EmphasisItalicsDark;
}
/// <summary>
@@ -122,17 +179,17 @@ namespace Microsoft.PowerShell.MarkdownRender
/// </summary>
public void SetLightTheme()
{
Header1 = "[7m";
Header2 = "[4;33m";
Header3 = "[4;34m";
Header4 = "[4;35m";
Header5 = "[4;36m";
Header6 = "[4;30m";
Code = "[48;2;155;155;155;38;2;30;30;30m";
Link = "[4;38;5;117m";
Image = "[33m";
EmphasisBold = "[1m";
EmphasisItalics = "[36m";
Header1 = Header1Light;
Header2 = Header2Light;
Header3 = Header3Light;
Header4 = Header4Light;
Header5 = Header5Light;
Header6 = Header6Light;
Code = CodeLight;
Link = LinkLight;
Image = ImageLight;
EmphasisBold = EmphasisBoldLight;
EmphasisItalics = EmphasisItalicsLight;
}
}
@@ -142,9 +199,10 @@ namespace Microsoft.PowerShell.MarkdownRender
public class VT100EscapeSequences
{
private const char Esc = (char)0x1B;
private string endSequence = Esc + "[0m";
// For code blocks, [500@ make sure that the whole line has background color.
private const string LongBackgroundCodeBlock = "[500@";
private MarkdownOptionInfo options;
/// <summary>
@@ -235,8 +293,7 @@ namespace Microsoft.PowerShell.MarkdownRender
}
else
{
// For code blocks, [500@ make sure that the whole line has background color.
return string.Concat(Esc, options.Code, codeText, Esc, "[500@", endSequence);
return string.Concat(Esc, options.Code, codeText, Esc, LongBackgroundCodeBlock, endSequence);
}
}
@@ -278,7 +335,14 @@ namespace Microsoft.PowerShell.MarkdownRender
/// <returns>Formatted image string.</returns>
public string FormatImage(string altText)
{
return string.Concat(Esc, options.Image, "[", altText, "]", endSequence);
var text = altText;
if(string.IsNullOrEmpty(altText))
{
text = "Image";
}
return string.Concat(Esc, options.Image, "[", text, "]", endSequence);
}
}
}
src/System.Management.Automation/engine/Utils.cs
+2
View File
@@ -1449,6 +1449,8 @@ namespace System.Management.Automation.Internal
internal static bool StopwatchIsNotHighResolution;
internal static bool DisableGACLoading;
internal static bool ShowMarkdownOutputBypass;
/// <summary>This member is used for internal test purposes.</summary>
public static void SetTestHook(string property, object value)
{
test/powershell/Modules/Microsoft.PowerShell.Utility/MarkdownCmdlets.Tests.ps1
+6 -2
View File
@@ -212,6 +212,10 @@ bool function()`n{`n}
It "Gets an error if input object type is not correct" {
{ ConvertFrom-Markdown -InputObject 1 -ErrorAction Stop } | Should -Throw -ErrorId 'InvalidInputObject,Microsoft.PowerShell.Commands.ConvertFromMarkdownCommand'
}
It "Gets an error if input file does not exist" {
{ [System.IO.FileInfo]::new("IDoNoExist") | ConvertFrom-Markdown -ErrorAction Stop } | Should -Throw -ErrorId 'FileNotFound,Microsoft.PowerShell.Commands.ConvertFromMarkdownCommand'
}
}
Context "Get/Set-MarkdownOption tests" {
@@ -292,11 +296,11 @@ bool function()`n{`n}
Context "Show-Markdown tests" {
BeforeEach {
[Microsoft.PowerShell.Commands.ShowMarkdownCommand]::SetOutputBypassTestHook($true)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook("ShowMarkdownOutputBypass", $true)
}
AfterEach {
[Microsoft.PowerShell.Commands.ShowMarkdownCommand]::SetOutputBypassTestHook($false)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook("ShowMarkdownOutputBypass", $false)
}
It "can show VT100 converted from markdown" {
test/powershell/engine/Basic/DefaultCommands.Tests.ps1
+4 -4
View File
@@ -191,7 +191,7 @@ Describe "Verify approved aliases list" -Tags "CI" {
"Cmdlet", "Connect-WSMan", , $($FullCLR -or $CoreWindows )
"Cmdlet", "ConvertFrom-Csv", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "ConvertFrom-Json", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "ConvertFrom-Markdown", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "ConvertFrom-Markdown", , $($CoreWindows -or $CoreUnix)
"Cmdlet", "ConvertFrom-SecureString", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "ConvertFrom-String", , $($FullCLR )
"Cmdlet", "ConvertFrom-StringData", , $($FullCLR -or $CoreWindows -or $CoreUnix)
@@ -271,7 +271,7 @@ Describe "Verify approved aliases list" -Tags "CI" {
"Cmdlet", "Get-ItemPropertyValue", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-Job", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-Location", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-MarkdownOption", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-MarkdownOption", , $($CoreWindows -or $CoreUnix)
"Cmdlet", "Get-Member", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-Module", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Get-PfxCertificate", , $($FullCLR -or $CoreWindows -or $CoreUnix)
@@ -410,7 +410,7 @@ Describe "Verify approved aliases list" -Tags "CI" {
"Cmdlet", "Set-Item", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-ItemProperty", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-Location", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-MarkdownOption", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-MarkdownOption", , $($CoreWindows -or $CoreUnix)
"Cmdlet", "Set-PSBreakpoint", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-PSDebug", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Set-PSSessionConfiguration", , $($FullCLR -or $CoreWindows )
@@ -425,7 +425,7 @@ Describe "Verify approved aliases list" -Tags "CI" {
"Cmdlet", "Show-Command", , $($FullCLR )
"Cmdlet", "Show-ControlPanelItem", , $($FullCLR )
"Cmdlet", "Show-EventLog", , $($FullCLR )
"Cmdlet", "Show-Markdown", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Show-Markdown", , $($CoreWindows -or $CoreUnix)
"Cmdlet", "Sort-Object", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Split-Path", , $($FullCLR -or $CoreWindows -or $CoreUnix)
"Cmdlet", "Start-Job", , $($FullCLR -or $CoreWindows -or $CoreUnix)