-
Notifications
You must be signed in to change notification settings - Fork 157
Update to maml rendered to pad example titles issue #119 #322
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
Update to maml rendered to pad example titles issue #119 #322
Conversation
@vors Full loop tests on Add-Member fail, which is expected based on help for Add-Member doesn't include dashed example titles. Didn't want to change the test, because it is correct. What do you think? |
|
||
private bool ShouldBreak(SectionFormatOption formatOption) | ||
{ | ||
// If the line break flag is set return true. |
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.
Nit fix :)
_cmdletUpdated = true; | ||
} | ||
|
||
// Update the parameter with the merged in FormatOption |
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.
Nit fix :)
I think we need green build before we can merge it :) Let's just change the test to stripe out the padding on example header comparison. |
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.
Awesome, just 1 comment. Since we are auto-adding padding in maml now, we should stripe it of in the Update-MarkdownHelp
and New-MarkdownHelp
. Beware, this can be a little tricky. I think the update scenario may not use the maml model reflection at all, but we should include stripping it out, because there is a lot of existing platyPS markdown help that inconsistently uses it. The new-markdownhelp
definitely uses the reflection model and we should stipe it out there too.
@vors Yes, actually I see why we would need to update the tests anyway. Otherwise every commit after until Add-Member help is updated would fail. Good idea. I'll add tests for that and we'll see how it goes. So just confirming that I've understood correctly. In the |
Correct. The update-markdown scenario is fine, because we simply take the markdown part as is: platyPS/src/Markdown.MAML/Transformer/MamlModelMerger.cs Lines 65 to 66 in 16a5f68
|
👏 |
Fix #119