Skip to content
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

Fixed formatting on expanded example #1861

Closed
wants to merge 1 commit into from
Closed

Fixed formatting on expanded example #1861

wants to merge 1 commit into from

Conversation

Halkcyon
Copy link

The formatting on the MS page looks really convoluted with the expanded example so I changed the example into one large codeblock for consistent formatting

Version(s) of document impacted

  • Impacts 6 document
  • Impacts 5.1 document
  • Impacts 5.0 document
  • Impacts 4.0 document
  • Impacts 3.0 document

Reason(s) for not updating all version of documents

  • The documented feature was introduced in version (list version here) of PowerShell
  • This issue only shows up in version (list version(s) here) of the document
  • This PR partially fixes the issue, and issue # tracks the remaining work

The formatting on the MS page looks really convoluted with the expanded example so I changed the example into one large codeblock for consistent formatting
@msftclas
Copy link

msftclas commented Nov 16, 2017

CLA assistant check
All CLA requirements met.

@saldana
Copy link
Contributor

saldana commented Nov 16, 2017

⚠️ Validation status: warnings

File Status Preview URL Details
reference/5.1/Microsoft.PowerShell.Core/About/about_pipelines.md ⚠️Warning Details

reference/5.1/Microsoft.PowerShell.Core/About/about_pipelines.md

  • [Warning] Ignore metadata: locale. They are generated by open publish.

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@sdwheeler
Copy link
Contributor

@joeyaiello @zjalexander - What do you think about this formatting change?

Copy link
Contributor

@zjalexander zjalexander left a comment

Choose a reason for hiding this comment

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

I see the problem. I love the effort to think about it. I would like to either have the codeblocks be all plain or have just the ascii art blocked off and plain. I think all-plain is slightly better and easier, but would like some additional discussion.

| ( FileInfo objects )
| ( .txt )
|
V

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, the above should probably be in a plain codeblock (no powershell language indicator) so the ASCII art doesn't get syntax highlighted? but then you would need to alternate between PS code blocks and plain code blocks? or just have the entire thing in plain codeblocks...

Copy link
Author

Choose a reason for hiding this comment

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

Another solution I thought of in hindsight, having a code block in one example, and another below that code block with the object types of the pipeline (instead of the commands)

@sdwheeler
Copy link
Contributor

@TheIncorrigible1 - Thank you for your contribution. After some deliberation I am closing this PR unmerged. We talked about this internally and liked some of the changes you made and we had other ideas. I had to reformat this topic as part of the issue #1806 efforts. So I incorporated this into my changes.

cc: @zjalexander

@sdwheeler sdwheeler closed this Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants