Repository navigation
PowerShell completion code has pipe | as | on website #10678
Description
Activity
Hey @asasine, good find! I'm surprised this is happening, given we recently added pipe escaping for long docs like this:
Lines 144 to 147 in b787340
I wonder if it has something to do with being in the markdown code block. Anyway, marking this one as
help-wanted. Thanks!Expected outcome
The
|on the https://cli.github.com/manual/gh_completion page describing PowerShell instructions renders as a|instead of its escape code|, as shown belowInvoke-Expression -Command $(gh completion -s powershell | Out-String)Reacted by AzeemI put some time to investigate this, and it seems the last fix has had these side effects:
gh help reference: In lots of headings.gh alias import: In the example YAML file.gh completion: In PowerShell example (reported in current the issue).gh run rerun: In example usage in the long doc line (not in Examples section).
Among these, I believe the
gh help referencecase is the most annoying one, because it's meant to be a precise reference document, and also SEO-friendly.Context
CORRECTED VERSION: As a bit of context, our
--helpdocs (i.e.,command.Longvalues), are written in standard Markdown. However, at the website, we render these docs by using the kramdown (a Ruby community effort to extend Markdown) renderer (e.g. here). Since, we're using the same content on both terminal and our website, we tend to use less noisy syntax alternatives of Markdown. For example, we can just prefix a line with a tab (or 4 whitespaces) to make it appear in a code-block, so both of the experiences below are created from the same Markdown text:- In terminal; note the indented code-block:
- In HTML; note the indented code-block with a fixed-width font face:
Problem
Thanks to @iamazeem who reported and investigated into this in #10348, we now know that kramdown interprets single pipes (
|) as table column separators, even if there's no table header, or other Markdown syntax elements for tables. This is not the case with standard Markdown, though.
So, we cannot use pipes in our plain text, except for when they appear in a code-block or within a code-span.Our first attempt to fix this (#10371) produced side effects I mentioned at the top.
Suggested solutions
The most simple
As I checked, so far
gh config --helpis the only place where we use pipes in plain text. The most simple fix is to replace pipes in thegh configdocs with something else (e.g. commas), or rewrite it so that pipes appear in a code-block. Replacing pipes can be a bit unappealing since we use pipes to separate available values for--options everywhere, but I don't think it's a big deal.The ideal solution
We can look for pipes in plain text and escape them when we're generating website docs (i.e. in
genMarkdownCustomfunction). For this we need to parse Markdown input into an AST, walk though the AST, and fix plain text occurrences of pipes.Preventive measures
Regardless of the fix, we can prevent such cases by having a test that looks for pipes in our
--helpdocs.@williammartin @andyfeller @BagToad I'd love to hear your thoughts here.
Discussed this with @babakks and we are mostly decided on a simple approach here that should fix all these cases.
The original cause of this was trying to fix pipe characters in gh_config in #10371, but that PR caused the problem reported in this issue with other command sets.
To resolve this, we're going to revert 10371 which will cause
gh configdocs to break again, but it will fix everything else agian. Then, we'll fixgh configdocs by surrounding the offending pipe characters in backticks.This will look like:
- `git_protocol`: the protocol to use for git clone and push operations `{https | ssh}` (default `https`) - `editor`: the text editor program to use for authoring text - `prompt`: toggle interactive prompting in the terminal `{enabled | disabled}` (default `enabled`) - `prefer_editor_prompt`: toggle preference for editor-based interactive prompting in the terminal `{enabled | disabled}` (default `disabled`) - `pager`: the terminal pager program to send standard output to - `http_unix_socket`: the path to a Unix socket through which to make an HTTP connection - `browser`: the web browser to use for opening URLs - `color_labels`: whether to display labels using their RGB hex color codes in terminals that support truecolor `{enabled | disabled}` (default `disabled`) - `accessible_colors`: whether customizable, 4-bit accessible colors should be used `{enabled | disabled}` (default `disabled`) - `accessible_prompter`: whether an accessible prompter should be used `{enabled | disabled}` (default `disabled`) - `spinner`: whether to use a animated spinner as a progress indicator `{enabled | disabled}` (default `enabled`)Placing the values in backticks aligns more closely with the formatting of positional arguments in our
Usehelp docs, as it seems like that's what we are sort of referring to here anyways as these are config value options for commands likegh config set <value>.Example of what I'm referring to:
USAGE gh issue view {<number> | <url>} [flags]Edit: @babakks is going to see about adding some tests to CI to detect places where backticks are not escaped in codeblocks.
Reacted by Babak K. Shandiz@BagToad when I was writing the tests, I tried to check out the standard Markdown syntax and noticed we're actually using the standard syntax, not kramdown. I mean, indented lines as code-blocks is a standard Markdown syntax. So, I was wrong about the whole kramdown usage.
Anyway, since we're using standard Markdown, I could write a reliable test which deep dives into the AST and looks for pipes where it shouldn't be.
I'll now update my last comment to avoid confusion/misinformation in the future.
Connected PRs have been merged, so this should be fixed with our next release 👍
Describe the bug
The PowerShell completion code has what should be a pipe
|as its escaped characters|on the website https://cli.github.com/manual/gh_completionAffected version
N/A
Steps to reproduce the behavior
Expected vs actual behavior
Expected:
Actual:
Logs
N/A