[Feature] Address code review comments

This commit is contained in:
Aditya Patwardhan 2018-07-10 08:58:19 -07:00 committed by Aditya Patwardhan
parent 32876c89e7
commit 275e109202
8 changed files with 173 additions and 72 deletions

View file

@ -19,28 +19,28 @@ namespace Microsoft.PowerShell.Commands
/// </summary>
[Cmdlet(
VerbsData.ConvertFrom, "Markdown",
DefaultParameterSetName = PathParamSet,
DefaultParameterSetName = PathParameterSet,
HelpUri = "TBD"
)]
[OutputType(typeof(Microsoft.PowerShell.MarkdownRender.MarkdownInfo))]
public class ConvertFromMarkdownCommand : PSCmdlet
{
/// <summary>
/// Path to the file to convert from Markdown to MarkdownInfo
/// Path to the file to convert from Markdown to MarkdownInfo.
/// </summary>
[ValidateNotNullOrEmpty]
[Parameter(ParameterSetName = PathParamSet, Mandatory = true)]
[Parameter(ParameterSetName = PathParameterSet, Mandatory = true)]
public string[] Path { get; set; }
/// <summary>
/// Path to the file to convert from Markdown to MarkdownInfo
/// Path to the file to convert from Markdown to MarkdownInfo.
/// </summary>
[ValidateNotNullOrEmpty]
[Parameter(ParameterSetName = LitPathParamSet, Mandatory = true)]
[Parameter(ParameterSetName = LiteralPathParameterSet, Mandatory = true)]
public string[] LiteralPath { get; set; }
/// <summary>
/// InputObject of type System.IO.FileInfo or string with content to convert from Markdown to MarkdownInfo
/// InputObject of type System.IO.FileInfo or string with content to convert from Markdown to MarkdownInfo.
/// </summary>
[ValidateNotNullOrEmpty]
[Parameter(ParameterSetName = InputObjParamSet, Mandatory = true, ValueFromPipeline = true)]
@ -52,19 +52,18 @@ namespace Microsoft.PowerShell.Commands
[Parameter()]
public SwitchParameter AsVT100EncodedString { get; set; }
private const string PathParamSet = "PathParamSet";
private const string LitPathParamSet = "LiteralParamSet";
private const string PathParameterSet = "PathParamSet";
private const string LiteralPathParameterSet = "LiteralParamSet";
private const string InputObjParamSet = "InputObjParamSet";
private MarkdownConversionType conversionType = MarkdownConversionType.HTML;
private MarkdownOptionInfo mdOption = null;
/// <summary>
/// Override ProcessRecord
/// Override BeginProcess.
/// </summary>
protected override void ProcessRecord()
protected override void BeginProcessing()
{
var conversionType = MarkdownConversionType.HTML;
var mdOption = (SessionState.PSVariable.GetValue("MarkdownOptionInfo", new MarkdownOptionInfo())) as MarkdownOptionInfo;
mdOption = (SessionState.PSVariable.GetValue("MarkdownOptionInfo", new MarkdownOptionInfo())) as MarkdownOptionInfo;
if(mdOption == null)
{
@ -75,14 +74,20 @@ namespace Microsoft.PowerShell.Commands
{
conversionType = MarkdownConversionType.VT100;
}
}
/// <summary>
/// Override ProcessRecord.
/// </summary>
protected override void ProcessRecord()
{
switch (ParameterSetName)
{
case InputObjParamSet:
Object baseObj = InputObject.BaseObject;
var fileInfo = baseObj as FileInfo;
if (fileInfo != null)
//var fileInfo = baseObj as FileInfo;
if (baseObj is FileInfo fileInfo)
{
WriteObject(
MarkdownConverter.Convert(
@ -92,33 +97,28 @@ namespace Microsoft.PowerShell.Commands
)
);
}
else if (baseObj is string inpObj)
{
WriteObject(MarkdownConverter.Convert(inpObj, conversionType, mdOption));
}
else
{
var inpObj = baseObj as string;
if (inpObj != null)
{
WriteObject(MarkdownConverter.Convert(inpObj, conversionType, mdOption));
}
else
{
string errorMessage = StringUtil.Format(ConvertMarkdownStrings.InvalidInputObjectType, baseObj.GetType());
ErrorRecord errorRecord = new ErrorRecord(
new InvalidDataException(errorMessage),
"InvalidInputObject",
ErrorCategory.InvalidData,
InputObject);
string errorMessage = StringUtil.Format(ConvertMarkdownStrings.InvalidInputObjectType, baseObj.GetType());
ErrorRecord errorRecord = new ErrorRecord(
new InvalidDataException(errorMessage),
"InvalidInputObject",
ErrorCategory.InvalidData,
InputObject);
WriteError(errorRecord);
}
WriteError(errorRecord);
}
break;
case PathParamSet:
case PathParameterSet:
ConvertEachFile(Path, conversionType, isLiteral: false, optionInfo: mdOption);
break;
case LitPathParamSet:
case LiteralPathParameterSet:
ConvertEachFile(LiteralPath, conversionType, isLiteral: true, optionInfo: mdOption);
break;
}
@ -128,7 +128,6 @@ namespace Microsoft.PowerShell.Commands
{
foreach (var path in paths)
{
// ResolvePath checks for file existence.
var resolvedPaths = ResolvePath(path, isLiteral);
foreach (var resolvedPath in resolvedPaths)

View file

@ -22,101 +22,112 @@ namespace Microsoft.PowerShell.Commands
public class SetMarkdownOptionCommand : PSCmdlet
{
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 1.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header1Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 2.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header2Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 3.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header3Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 4.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header4Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 5.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header5Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for Header Level 6.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string Header6Color { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for code block background.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string CodeBlockForegroundColor { get; set;}
/// <summary>
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string CodeBlockBackgroundColor { get; set;}
public string Code { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for image alt text foreground.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string ImageAltTextForegroundColor { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for link foreground.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string LinkForegroundColor { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for italics text foreground.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string ItalicsForegroundColor { get; set;}
/// <summary>
/// Gets or sets the VT100 escape sequence for bold text foreground.
/// </summary>
[ValidatePattern(@"^\[*[0-9;]*?m{1}")]
[Parameter(ParameterSetName = IndividualSetting)]
public string BoldForegroundColor { get; set;}
/// <summary>
/// Gets or sets the switch to PassThru the values set.
/// </summary>
[Parameter()]
public SwitchParameter PassThru { get; set;}
/// <summary>
/// Gets or sets the Theme.
/// </summary>
[ValidateNotNullOrEmpty]
[Parameter(ParameterSetName = ThemeParamSet, Mandatory = true)]
[ValidateSet(DarkThemeName, LightThemeName)]
public string Theme { get; set;}
/// <summary>
/// Gets or sets InputObject.
/// </summary>
[ValidateNotNullOrEmpty]
[Parameter(ParameterSetName = InputObjectParamSet, Mandatory = true, ValueFromPipeline = true)]
public PSObject InputObject { get; set;}
private const string IndividualSetting = "IndividualSetting";
private const string InputObjectParamSet = "InputObject";
private const string ThemeParamSet = "Theme";
private const string MarkdownOptionInfoVariableName = "MarkdownOptionInfo";
private const string LightThemeName = "Light";
private const string DarkThemeName = "Dark";
/// <summary>
/// Override EndProcessing.
/// </summary>
protected override void EndProcessing()
{
@ -126,11 +137,11 @@ namespace Microsoft.PowerShell.Commands
{
case ThemeParamSet:
mdOptionInfo = new MarkdownOptionInfo();
if(string.Equals(Theme, "Light", StringComparison.OrdinalIgnoreCase))
if(string.Equals(Theme, LightThemeName, StringComparison.OrdinalIgnoreCase))
{
mdOptionInfo.SetLightTheme();
}
else if(string.Equals(Theme, "Dark", StringComparison.OrdinalIgnoreCase))
else if(string.Equals(Theme, DarkThemeName, StringComparison.OrdinalIgnoreCase))
{
mdOptionInfo.SetDarkTheme();
}
@ -144,7 +155,6 @@ namespace Microsoft.PowerShell.Commands
{
throw new ArgumentException();
}
break;
case IndividualSetting:
@ -154,7 +164,7 @@ namespace Microsoft.PowerShell.Commands
}
var sessionVar = SessionState.PSVariable;
sessionVar.Set("MarkdownOptionInfo", mdOptionInfo);
sessionVar.Set(MarkdownOptionInfoVariableName, mdOptionInfo);
if(PassThru.IsPresent)
{
@ -194,14 +204,9 @@ namespace Microsoft.PowerShell.Commands
mdOptionInfo.Header6 = Header6Color;
}
if (!String.IsNullOrEmpty(CodeBlockBackgroundColor))
if (!String.IsNullOrEmpty(Code))
{
mdOptionInfo.Code = CodeBlockBackgroundColor;
}
if (!String.IsNullOrEmpty(CodeBlockForegroundColor))
{
mdOptionInfo.Code = CodeBlockForegroundColor;
mdOptionInfo.Code = Code;
}
if (!String.IsNullOrEmpty(ImageAltTextForegroundColor))
@ -227,6 +232,7 @@ namespace Microsoft.PowerShell.Commands
}
/// <summary>
/// Implements the cmdlet for getting the markdown options that are set.
/// </summary>
[Cmdlet(
VerbsCommon.Get, "MarkdownOption",
@ -235,11 +241,14 @@ namespace Microsoft.PowerShell.Commands
[OutputType(typeof(Microsoft.PowerShell.MarkdownRender.MarkdownOptionInfo))]
public class GetMarkdownOptionCommand : PSCmdlet
{
private const string MarkdownOptionInfoVariableName = "MarkdownOptionInfo";
/// <summary>
/// Override endproessing.
/// </summary>
protected override void EndProcessing()
{
WriteObject(SessionState.PSVariable.GetValue("MarkdownOptionInfo", new MarkdownOptionInfo()));
WriteObject(SessionState.PSVariable.GetValue(MarkdownOptionInfoVariableName, new MarkdownOptionInfo()));
}
}
}

View file

@ -58,7 +58,8 @@ namespace Microsoft.PowerShell.Commands
{
Object inpObj = InputObject.BaseObject;
var markdownInfo = inpObj as MarkdownInfo;
if (markdownInfo == null)
if (markdownInfo != null)
{
var errorRecord = new ErrorRecord(
new ArgumentException(),

View file

@ -124,6 +124,6 @@
<value>The given file path '{0}' is not found.</value>
</data>
<data name="FileSystemPathsOnly" xml:space="preserve">
<value>The given path '{0}' is not supported. This command only supports the FileSystem Provider paths.</value>
<value>Only FileSystem Provider paths are supported. The given path '{0}' is not supported.</value>
</data>
</root>

View file

@ -21,9 +21,7 @@ namespace Microsoft.PowerShell.MarkdownRender
foreach (var item in obj)
{
var listItem = item as ListItemBlock;
if (listItem != null)
if (item is ListItemBlock listItem)
{
if (obj.IsOrdered)
{
@ -44,9 +42,7 @@ namespace Microsoft.PowerShell.MarkdownRender
// For a numbered list, we need to make sure the index is incremented.
foreach (var line in block)
{
var paragraphBlock = line as ParagraphBlock;
if(paragraphBlock != null)
if(line is ParagraphBlock paragraphBlock)
{
renderer.Write(index.ToString()).Write(". ").Write(paragraphBlock.Inline);
}

View file

@ -3,6 +3,7 @@
using System;
using System.IO;
using System.Threading;
using Markdig;
using Markdig.Syntax;
using Markdig.Renderers;
@ -33,7 +34,7 @@ namespace Microsoft.PowerShell.MarkdownRender
private void RenderWithIndent(VT100Renderer renderer, MarkdownObject block, char listBullet, int indentLevel)
{
// Indent left by 2 for each level on list.
string indent = "".PadLeft(indentLevel * 2);
string indent = Padding(indentLevel * 2);
var paragraphBlock = block as ParagraphBlock;
@ -62,5 +63,24 @@ namespace Microsoft.PowerShell.MarkdownRender
}
}
}
// Typical padding is at most a screen's width, any more than that and we won't bother caching.
private const int IndentCacheMax = 120;
private static readonly string[] IndentCache = new string[IndentCacheMax];
internal static string Padding(int countOfSpaces)
{
if (countOfSpaces >= IndentCacheMax)
return new string(' ', countOfSpaces);
var result = IndentCache[countOfSpaces];
if (result == null)
{
Interlocked.CompareExchange(ref IndentCache[countOfSpaces], new string(' ', countOfSpaces), comparand:null);
result = IndentCache[countOfSpaces];
}
return result;
}
}
}

View file

@ -6,7 +6,7 @@
<AssemblyName>Microsoft.PowerShell.MarkdownRender</AssemblyName>
</PropertyGroup>
<PropertyGroup>
<!--PropertyGroup>
<DefineConstants>$(DefineConstants);CORECLR</DefineConstants>
</PropertyGroup>
@ -20,7 +20,7 @@
<PropertyGroup Condition=" '$(Configuration)' == 'CodeCoverage' ">
<DebugType>full</DebugType>
</PropertyGroup>
</PropertyGroup-->
<ItemGroup>
<!-- Source: https://github.com/lunet-io/markdig/ -->

View file

@ -150,7 +150,7 @@ bool function()`n{`n}
$expectedString = GetExpectedString -ElementType $element -CodeText $codeText
}
$output.VT100EncodedString | Should BeExactly $expectedString
$output.VT100EncodedString | Should -BeExactly $expectedString
}
It 'Can convert element : <element> to HTML using pipeline input' -TestCases $TestCases {
@ -181,22 +181,22 @@ bool function()`n{`n}
$expectedString = GetExpectedHTML -ElementType $element -Text $text -Url $url
}
$output.Html | Should BeExactly $expectedString
$output.Html | Should -BeExactly $expectedString
}
It 'Can convert input from a file path to vt100 encoded string' {
$output = ConvertFrom-Markdown -Path $mdFile.FullName -AsVT100EncodedString
$output.VT100EncodedString | Should BeExactly $expectedStringFromFile
$output.VT100EncodedString | Should -BeExactly $expectedStringFromFile
}
It 'Can convert input from a fileinfo object to vt100 encoded string' {
$ouputFromFileInfo = $mdFile | ConvertFrom-Markdown -AsVT100EncodedString
$ouputFromFileInfo.VT100EncodedString | Should BeExactly $expectedStringFromFile
$ouputFromFileInfo.VT100EncodedString | Should -BeExactly $expectedStringFromFile
}
It 'Can convert input from a literal path to vt100 encoded string' {
$output = ConvertFrom-Markdown -Path $mdLiteralPath -AsVT100EncodedString
$output.VT100EncodedString | Should BeExactly $expectedStringFromFile
$output.VT100EncodedString | Should -BeExactly $expectedStringFromFile
}
}
@ -213,4 +213,80 @@ bool function()`n{`n}
{ ConvertFrom-Markdown -InputObject 1 -ErrorAction Stop } | Should -Throw -ErrorId 'InvalidInputObject,Microsoft.PowerShell.Commands.ConvertFromMarkdownCommand'
}
}
Context "Get/Set-MarkdownOption tests" {
BeforeAll {
$esc = [char]0x1b
}
BeforeEach {
$originalOptions = Get-MarkdownOption
}
AfterEach {
Set-MarkdownOption -InputObject $originalOptions
}
It "Verify default values for MarkdownOptions" {
$options = Get-MarkdownOption
$options.Header1 | Should -BeExactly "$esc[7m[7m$esc[0m"
$options.Header2 | Should -BeExactly "$esc[4;93m[4;93m$esc[0m"
$options.Header3 | Should -BeExactly "$esc[4;94m[4;94m$esc[0m"
$options.Header4 | Should -BeExactly "$esc[4;95m[4;95m$esc[0m"
$options.Header5 | Should -BeExactly "$esc[4;96m[4;96m$esc[0m"
$options.Header6 | Should -BeExactly "$esc[4;97m[4;97m$esc[0m"
$options.Code | Should -BeExactly "$esc[48;2;155;155;155;38;2;30;30;30m[48;2;155;155;155;38;2;30;30;30m$esc[0m"
$options.Link | Should -BeExactly "$esc[4;38;5;117m[4;38;5;117m$esc[0m"
$options.Image | Should -BeExactly "$esc[33m[33m$esc[0m"
$options.EmphasisBold | Should -BeExactly "$esc[1m[1m$esc[0m"
$options.EmphasisItalics | Should -BeExactly "$esc[36m[36m$esc[0m"
}
It "Verify Set-MarkdownOption can get options" {
Set-MarkdownOption `
-Header1Color "[4;1m" `
-Header2Color "[93m" `
-Header3Color "[94m" `
-Header4Color "[95m" `
-Header5Color "[96m" `
-Header6Color "[97m" `
-ImageAltTextForegroundColor "[34m" `
-LinkForegroundColor "[4;38;5;88m" `
-ItalicsForegroundColor "[35m" `
-BoldForegroundColor "[32m"
$newOptions = Get-MarkdownOption
$options.Header1 | Should -BeExactly "$esc[4;1m[4;1m$esc[0m"
$options.Header2 | Should -BeExactly "$esc[93m[93m$esc[0m"
$options.Header3 | Should -BeExactly "$esc[94m[94m$esc[0m"
$options.Header4 | Should -BeExactly "$esc[95m[95m$esc[0m"
$options.Header5 | Should -BeExactly "$esc[96m[96m$esc[0m"
$options.Header6 | Should -BeExactly "$esc[97m[97m$esc[0m"
#$options.Code | Should -BeExactly "$esc[48;2;155;155;155;38;2;30;30;30m[48;2;155;155;155;38;2;30;30;30m$esc[0m"
$options.Link | Should -BeExactly "$esc[4;38;5;88m[4;38;5;88m$esc[0m"
$options.Image | Should -BeExactly "$esc[34m[34m$esc[0m"
$options.EmphasisBold | Should -BeExactly "$esc[32m[32m$esc[0m"
$options.EmphasisItalics | Should -BeExactly "$esc[35m[35m$esc[0m"
}
It "Verify defaults for light theme" {
$options = Get-MarkdownOption
$options.Header1 | Should -BeExactly "$esc[7m[7m$esc[0m"
$options.Header2 | Should -BeExactly "$esc[4;33m[4;33m$esc[0m"
$options.Header3 | Should -BeExactly "$esc[4;34m[4;34m$esc[0m"
$options.Header4 | Should -BeExactly "$esc[4;35m[4;35m$esc[0m"
$options.Header5 | Should -BeExactly "$esc[4;36m[4;36m$esc[0m"
$options.Header6 | Should -BeExactly "$esc[4;30m[4;30m$esc[0m"
$options.Code | Should -BeExactly "$esc[48;2;155;155;155;38;2;30;30;30m[48;2;155;155;155;38;2;30;30;30m$esc[0m"
$options.Link | Should -BeExactly "$esc[4;38;5;117m[4;38;5;117m$esc[0m"
$options.Image | Should -BeExactly "$esc[33m[33m$esc[0m"
$options.EmphasisBold | Should -BeExactly "$esc[1m[1m$esc[0m"
$options.EmphasisItalics | Should -BeExactly "$esc[36m[36m$esc[0m"
}
}
}