Skip to content

Support multi-line code blocks in examples#10776

Merged
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
Greg-Smulko:improve-code-block-recognition-in-examples
Nov 16, 2019
Merged

Support multi-line code blocks in examples#10776
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
Greg-Smulko:improve-code-block-recognition-in-examples

Conversation

@Greg-Smulko
Copy link
Copy Markdown
Contributor

@Greg-Smulko Greg-Smulko commented Oct 11, 2019 •
edited
Loading

PR Summary

Support multi-line code blocks in examples.

PR Context

Fix as asked for by @vors in PowerShell/platyPS#180 (comment).
Fixes the issue PowerShell/platyPS#180 Multiline examples don't display as markdown code.

PR Checklist

@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch from 2e12760 to 80fb675 Compare October 12, 2019 10:16
@Greg-Smulko Greg-Smulko changed the title WIP: Support multi-line code blocks in examples Support multi-line code blocks in examples Oct 12, 2019
@Greg-Smulko Greg-Smulko marked this pull request as ready for review October 12, 2019 10:17
@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch from 80fb675 to 1af3d7e Compare October 12, 2019 10:21
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 12, 2019

@Greg-Smulko Please rebase to pass Static CI.

iSazonov
iSazonov reviewed Oct 12, 2019
View reviewed changes
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 12, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.5 milestone Oct 12, 2019
@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch from 1af3d7e to 9699d50 Compare October 12, 2019 17:24
@iSazonov iSazonov changed the title Support multi-line code blocks in examples WIP: Support multi-line code blocks in examples Oct 12, 2019
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 12, 2019

Add WIP until we get Pester tests.

@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch from 9699d50 to c2bac4d Compare October 12, 2019 23:04
@Greg-Smulko Greg-Smulko changed the title WIP: Support multi-line code blocks in examples Support multi-line code blocks in examples Oct 12, 2019
@Greg-Smulko Greg-Smulko requested a review from iSazonov October 12, 2019 23:07
@Greg-Smulko
Copy link
Copy Markdown
Contributor Author

Greg-Smulko commented Oct 14, 2019

I replaced unit tests with Pester tests, could you take a look again @iSazonov?

@iSazonov iSazonov requested a review from vors October 14, 2019 14:15
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 14, 2019

@Greg-Smulko Could you please check the fix with PlatyPS?

iSazonov
iSazonov reviewed Oct 14, 2019
View reviewed changes
@adityapatwardhan adityapatwardhan added the WG-Interactive-HelpSystem help infrastructure and formatting of help label Oct 15, 2019
@adityapatwardhan adityapatwardhan self-assigned this Oct 15, 2019
@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch 2 times, most recently from 552809f to 3c2866e Compare October 18, 2019 16:10
@Greg-Smulko
Copy link
Copy Markdown
Contributor Author

Greg-Smulko commented Oct 18, 2019

Could you please check the fix with PlatyPS?

@iSazonov , I tested the fix with PlatyPS, everything looks fine. :)
See the files attached.
I generated them using the New-MarkdownHelp and New-ExternalHelp cmdlets.

iSazonov
iSazonov approved these changes Oct 18, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

@Greg-Smulko Thanks for confirmation!

I think it would be useful to measure and share a performance degradation since we use Regex now. Could you please do this?

@Greg-Smulko
Copy link
Copy Markdown
Contributor Author

Greg-Smulko commented Oct 18, 2019

@iSazonov , performance degradation? What degradation?
On master the Measure-Command -Expression { Get-Help Get-Help -examples } took 304 ms, on the new version only 287 ms.

But seriously, it seems that the very first run of Get-Help takes around 300 ms on both versions, and then the subsequent ones (for a different command) about 10-15 ms.

I tried running Measure-Command -Expression { Get-Help Get-Help -examples } (took ~300 ms) and then Measure-Command -Expression { Get-Help Get-Command -examples } (took ~15 ms).

To summarize, I think that there is no visible performance degradation at all, at least for the cmdlets that I tested.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 19, 2019

@Greg-Smulko Thanks!

@adityapatwardhan Please review the PR.

@Greg-Smulko
Copy link
Copy Markdown
Contributor Author

Greg-Smulko commented Oct 31, 2019

Hi @adityapatwardhan, any hope that you could take a look at this?

@Greg-Smulko Greg-Smulko force-pushed the improve-code-block-recognition-in-examples branch from 3c2866e to eda675e Compare November 2, 2019 16:02
@adityapatwardhan adityapatwardhan merged commit f32ae64 into PowerShell:master Nov 16, 2019
@adityapatwardhan
Copy link
Copy Markdown
Member

adityapatwardhan commented Nov 16, 2019

@Greg-Smulko Thank you for your contribution!

@Greg-Smulko Greg-Smulko deleted the improve-code-block-recognition-in-examples branch November 16, 2019 19:12
@ghost
Copy link
Copy Markdown

ghost commented Nov 21, 2019

🎉v7.0.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

@bravo-kernel bravo-kernel mentioned this pull request Dec 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

@vors vors Awaiting requested review from vors

Assignees

@adityapatwardhan adityapatwardhan

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log WG-Interactive-HelpSystem help infrastructure and formatting of help

Projects

None yet

Milestone

7.0.0-preview.6

Development

Successfully merging this pull request may close these issues.

3 participants

@Greg-Smulko @iSazonov @adityapatwardhan