Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Fixing incorrect rendering of pipe character #115

Closed
wants to merge 3 commits into from

Conversation

SoniaLopezBravo
Copy link
Contributor

This closes #352

@SoniaLopezBravo
Copy link
Contributor Author

@bettinaheim @tcNickolas @cgranade Could you review this? Thanks!

@tcNickolas
Copy link
Member

This approach doesn't seem to work when viewing the files on GitHub: looking at https://github.com/microsoft/qsharp-language/blob/740b3427d7be2f8bcba1e9c14675d82b16590143/Specifications/Language/3_Expressions/PrecedenceAndAssociativity.md, it renders as
image
If we want it to be viewable both in the docs and in the GitHub version of the files, we might have to do some kind of processing in the documentation generation pipeline to account for this difference.

@SoniaLopezBravo
Copy link
Contributor Author

@tcNickolas I have changed it for another of the possible solutions. This one renders ok on GitHub files, hope it does on the docs page too.

@tcNickolas
Copy link
Member

Can you open a pull request with a matching change on the docs repo so that we get a preview to check whether this solution works indeed?

@SoniaLopezBravo
Copy link
Contributor Author

Can you open a pull request with a matching change on the docs repo so that we get a preview to check whether this solution works indeed?

Hi @tcNickolas. I have created this matching PR, in the preview we see that the pipe character is rendering ok :)

tcNickolas
tcNickolas previously approved these changes Nov 3, 2021
Copy link
Member

@tcNickolas tcNickolas left a comment

Choose a reason for hiding this comment

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

Great, looks like this solution works both on GitHub and in the docs!
Just one small comment: let's keep the whitespace between ? and |, since they are not a single operator like |||, but rather two parts of an operator that have an expression between them.

…ty.md

Co-authored-by: Mariia Mykhailova <michaylova@gmail.com>
@SoniaLopezBravo
Copy link
Contributor Author

@bettinaheim could you review this?

Copy link
Contributor

@alan-geller alan-geller left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@bettinaheim
Copy link
Contributor

I believe this is addressed by the now merged PR #124. I'll hence close this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect rendering of operators that include |
4 participants