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

Created on 23 Mar 2020  ·  20Comments  ·  Source: pnp/cli-microsoft365

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

docs enhancement work in progress

Most helpful comment

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.

All 20 comments

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 :bowing_man:

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

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

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.

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

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

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.

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?

Nope. Whatever you prefer.

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 🤦🏻‍♂️

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.

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

All yours @jhagstrom thanks for the help 👍🏻

@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`

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.

Now it works #1445

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.

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

@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.

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.

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.

Was this page helpful?
0 / 5 - 0 ratings