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

Code blocks that contain shell output have misleading syntax highlighting #34621

Closed
1 task done
TheOne04 opened this issue Sep 17, 2024 · 11 comments · Fixed by #34650
Closed
1 task done

Code blocks that contain shell output have misleading syntax highlighting #34621

TheOne04 opened this issue Sep 17, 2024 · 11 comments · Fixed by #34650
Labels
authentication Content relating to authentication content This issue or pull request belongs to the Docs Content team help wanted Anyone is welcome to open a pull request to fix this issue SME reviewed An SME has reviewed this issue/PR

Comments

@TheOne04
Copy link
Contributor

TheOne04 commented Sep 17, 2024

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/authentication/troubleshooting-ssh/using-ssh-over-the-https-port

What part(s) of the article would you like to see updated?

The code blocks containing shell output unintentionally highlight the command's output due to the presence of characters like quotes.
This creates a visually misleading representation of the actual command output.
Ideally, only the command line should be highlighted, though leaving the entire block unhighlighted is also a viable solution.

Additional information

image
image

@TheOne04 TheOne04 added the content This issue or pull request belongs to the Docs Content team label Sep 17, 2024
Copy link

welcome bot commented Sep 17, 2024

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Sep 17, 2024
@nguyenalex836 nguyenalex836 added waiting for review Issue/PR is waiting for a writer's review authentication Content relating to authentication and removed triage Do not begin working on this issue until triaged by the team labels Sep 17, 2024
@nguyenalex836
Copy link
Contributor

@TheOne04 Thank you for raising this issue! I'll get this triaged for review ✨ Our team will provide feedback regarding the best next steps for this issue - thanks for your patience! 💛

@subatoi subatoi added the needs SME This proposal needs review from a subject matter expert label Sep 18, 2024
Copy link
Contributor

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert 👀

@subatoi
Copy link
Contributor

subatoi commented Sep 18, 2024

@TheOne04 Thanks for raising this as an issue first before raising a PR, so we can discuss it: it's much appreciated.

I agree with you and I think the simplest solution is simply to remove the angle bracket characters, as they don't serve any real purpose and the $ should mark what's a command line command as opposed to what's a prompt.

We could apply that solution to a large number of articles in our docs, and not just the page you've highlighted, so I'll defer to @nguyenalex836 on the best course of action, but we will open this up to contributors in some form.

Thank you!

@nguyenalex836 nguyenalex836 added SME reviewed An SME has reviewed this issue/PR and removed waiting for review Issue/PR is waiting for a writer's review needs SME This proposal needs review from a subject matter expert labels Sep 18, 2024
@nguyenalex836
Copy link
Contributor

@subatoi Thank you for the review and guidance here! ✨

@TheOne04 Thanks again for raising an issue! Per @subatoi's guidance, would you be open to:

  1. Raising a PR (and linking to this issue) to remove the angle bracket characters for the code blocks you referenced in this issue
  2. Opening a new issue for any other code blocks where the angle bracket characters should be removed

Once that is complete, we can comb through the docs on our end to find other code blocks where the angle bracket characters should be removed, and open up issues for contributors to work on 💛

@TheOne04
Copy link
Contributor Author

@TheOne04 Thanks for raising this as an issue first before raising a PR, so we can discuss it: it's much appreciated.

I agree with you and I think the simplest solution is simply to remove the angle bracket characters, as they don't serve any real purpose and the $ should mark what's a command line command as opposed to what's a prompt.

We could apply that solution to a large number of articles in our docs, and not just the page you've highlighted, so I'll defer to @nguyenalex836 on the best course of action, but we will open this up to contributors in some form.

Thank you!

Thank you for the detailed response. While I agree with removing the > characters, this doesn't address the issue of command output being colored differently (blue) after a single quote '. This coloring is misleading, as the text is plain output, not shell code.

@nguyenalex836
Copy link
Contributor

@TheOne04 Thank you for the elaboration! I'll let @subatoi responding regarding your last comment 💛

@subatoi
Copy link
Contributor

subatoi commented Sep 19, 2024

You're quite right @TheOne04: let's change the environment to bash and use # comments instead of the angle bracket, so:

\```shell
$ ssh -T -p 443 [email protected]
> Hi USERNAME! You've successfully authenticated, but GitHub does not
> provide shell access.
\```

Will become:

\```bash
$ ssh -T -p 443 [email protected]
# Hi USERNAME! You've successfully authenticated, but GitHub does not
# provide shell access.
\```

Please note I've escaped backticks for illustration purposes only.

If you haven't already, please refrain from making the issue that we suggested you create (cc @nguyenalex836, my apologies for creating confusion) — we'll go ahead and create an issue internally. We'd need to evaluate whether bash is always appropriate before we'd open that up to contributors, but in this case I'm satisfied that it's appropriate–if only because it's better than the strange syntax highlighting you point out.

For now, I'll go ahead and open it up to contributors that anyone can raise a PR with the fix I've suggested above, for the article https://docs.github.com/en/authentication/troubleshooting-ssh/using-ssh-over-the-https-port

@subatoi subatoi added the help wanted Anyone is welcome to open a pull request to fix this issue label Sep 19, 2024
@TheOne04
Copy link
Contributor Author

Thanks! If it's alright can I go ahead and submit a PR with the correction?

Also I would be open to help with correcting any other pages that face a similar issue but bash may not necessarily be the best solution for, so please do let me know if there's anything. :)

@gonmmarques
Copy link
Contributor

Oops sorry @TheOne04, I will close my PR.

@TheOne04
Copy link
Contributor Author

My bad I guess I should've just created a PR to avoid this confusion sorry @gonmmarques

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication Content relating to authentication content This issue or pull request belongs to the Docs Content team help wanted Anyone is welcome to open a pull request to fix this issue SME reviewed An SME has reviewed this issue/PR
Projects
None yet
4 participants