Bug report: Make sure pipes are escaped in .md files

[jhagstrom]

Description

If pipe '|' is used in markdown without being escaped like '|', the pipe and the following text will not render

Steps to reproduce

See https://github.com/pnp/office365-cli/blob/master/docs/manual/docs/cmd/aad/o365group/o365group-set.md

Expected result

"Output type. json|text

Actual result

"Output type. `json "

Environment

N/A

[VelinGeorgiev]

That is an issue @jhagstrom . I remember we had discussion in the past with @waldekmastykarz and @garrytrinder where we kind of agreed that all the pipes should be replaced by comma so | will become ,. I just forgot to document it so thank you so doing it 🙇‍♂️

By the way the pipes render just find in the mkdocs here: https://pnp.github.io/office365-cli/, but not in Github

[jhagstrom]

Is replacing pipes with commas the desired solution? I can take this issue on.

[waldekmastykarz]

Yes, it is. I believe we've done some work in this area but we might have missed a thing or two. If you'd like to go through the help and double check everything is as it's supposed to be, that would help us.

[garrytrinder]

The way I handle this in the New command issue template is to escape the pipe, so json\|text should work to keep the pipe in place.

Source: https://github.com/pnp/office365-cli/blob/master/.github/ISSUE_TEMPLATE/new-command.md

[waldekmastykarz]

I believe @VelinGeorgiev already replaced pipe with comma in many places, so for consistency, it would be better to use , everywhere

[garrytrinder]

Whilst this maybe the case, would a pipe not be better semantically?

A pipe, to me, suggests “or” whereas a comma could suggest “and”. You may think different?

As our issue template currently uses a pipe, if we do go down the comma route, then the template should be changed to reflect this.

[waldekmastykarz]

A while back we discussed it and thought that using a comma would be easier but I'm all for using a |. @VelinGeorgiev any particular preference?

[VelinGeorgiev]

Nope. Whatever you prefer.

[garrytrinder]

I think we did agree on commas initially but that was before I “fixed” it in the issue template and I totally forgot about us saying we would use commas 🤦🏻‍♂️

[jhagstrom]

I sample clicked some of the command documentation and my impression is that most are using pipe still, among those all root level commands, login,logout, consent etc.

[waldekmastykarz]

All right, then let's agree on using |.

[garrytrinder]

All yours @jhagstrom thanks for the help 👍🏻

[garrytrinder]

@jhagstrom apologies for this but I now understand the reason why we had the discussion about commas initially. It is down the difference in how GitHub and MKDocs parse markdown.

In Github, json\|text works fine as GitHub flavour markdown supports using a slash as an escape character. Unfortunately though, MKDocs uses a different flavour of markdown which doesn't, so we end up fixing GitHub and breaking MKDocs by using \|

@VelinGeorgiev already mentioned this in his original post, so apologies for not picking up on that earlier.

By the way the pipes render just find in the mkdocs here: pnp.github.io/office365-cli, but not in Github

However, after some investigation this evening, the only way I have been able to solve this on both sides is to change to use HTML Entity value for pipe, which is |.

I've tested the below line in GitHub & MKDocs and it renders the pipe fine on both. The only very minor difference is that the pipe is now located outside of the back-ticks so that it is rendered correctly.

`json`|`text`

[jhagstrom]

I'll check it out. I tried replacing the escaped pipe with the html encrypted thing, but it doesn't render in github md for me now so I think I didn't do it right.

[jhagstrom]

Now it works #1445

[waldekmastykarz]

Expecting contributors to use html escaping is pretty excessive imho. I'd suggest we reconsider the change and consider simplicity of maintenance and contributions and use , which just works everywhere without any quirks.

[jhagstrom]

Is mkdocs configurable in terms of markdown flavor? It would be nice to have gfm in both places

[garrytrinder]

@jhagstrom after my research last night, there is no alternative GFM parser for MKDocs, there is one that I found but it had way too many limitations listed to really consider it.

I’d love to see the docs retain the use of pipe but I see @waldekmastykarz point, happy to reconsider.

[jhagstrom]

I replaced the html encoded pipe with commas now.
Then, in the spirit of bikeshedding I started thinking about keeping the escaped pipe that works in github in the repo, and then remove the escaping as a step in the circleci yaml before the deploying the docs to gh pages.

[waldekmastykarz]

Thanks for bearing with us @jhagstrom. For now, let's use commas, like you've done in the PR. We can always reconsider, should there be a newer version of MkDocs with additional capabilities.