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

Auto-remove leading $ symbols when copying from docs terminal blocks #9204

Closed
wants to merge 5 commits into from

Conversation

rpeden
Copy link
Contributor

@rpeden rpeden commented Apr 12, 2023

PR #9026 added copy buttons to all code blocks in the docs, so users can now copy the entire block with a single click.

However, many code blocks intended for use in the terminal contain a leading $ character. Consequently, if the user clicks the copy button and then pastes into a terminal, they'll see an error message when trying to run it.

Per a suggestion from @cicdw in #9026, this PR adds JavaScript that removes leading $ symbols from terminal blocks before the text is copied. It does this invisibly in the background, by creating a hidden element containing the cleaned text, which then gets copied to the clipboard. The $ signs remain in the visible code element on the docs page.

The JavaScript only acts on code blocks where we intentionally apply terminal styling, like so:

<div class="terminal">
```bash
$ sqlite3 --version
```
</div>

It leaves regular code blocks alone.

Example and QA

Before

  1. Navigate to a docs code block with leading $ signs, like this one: https://docs.prefect.io/2.10.3/getting-started/installation/#installing-for-development

  2. Click the copy button in the top right of the terminal block.

  3. Paste the commands into a terminal and notice the leading $ characters:

    $ git clone https://github.com/PrefectHQ/prefect.git
    $ cd prefect
    $ pip install -e ".[dev]"
    $ pre-commit install
  4. Try to run the commands and see an error message (the exact message will vary depending on which OS and terminal you use):

     zsh: command not found: $
     zsh: command not found: $
     zsh: command not found: $
     zsh: command not found: $

After

  1. Follow steps 1 to 3 above using this PR's deploy preview.
  2. Notice the leading $ characters are gone:
    git clone https://github.com/PrefectHQ/prefect.git
    cd prefect
    pip install -e ".[dev]"
    pre-commit install

The commands will now run successfully if you press return. Don't actually run them unless you want to clone the Prefect repo and pip install it in dev mode in your current environment. Just to be thorough, I ran the pasted commands in a fresh Conda environment, and everything worked as expected.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement, docs.

@rpeden rpeden added the docs label Apr 12, 2023
@netlify
Copy link

netlify bot commented Apr 12, 2023

Deploy Preview for prefect-docs-preview ready!

Name Link
🔨 Latest commit 5087752
🔍 Latest deploy log https://app.netlify.com/sites/prefect-docs-preview/deploys/64371dc56d62a1000885e879
😎 Deploy Preview https://deploy-preview-9204--prefect-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb
Copy link
Contributor

zanieb commented Apr 12, 2023

Sweet!

@rpeden rpeden marked this pull request as ready for review April 12, 2023 21:11
@rpeden rpeden requested a review from a team as a code owner April 12, 2023 21:11
@rpeden rpeden marked this pull request as draft April 12, 2023 21:18
Copy link
Contributor

@billpalombi billpalombi left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@rpeden
Copy link
Contributor Author

rpeden commented Apr 12, 2023

Moved back to draft pending implementation of a simpler and superior solution based on an excellent suggestion by @znicholasbrown 😀

@jawnsy jawnsy mentioned this pull request Apr 19, 2023
3 tasks
@flapili
Copy link

flapili commented Apr 20, 2023

the best solution in my opinion is to use CSS (but from what I see that will need a little rework about the generated dom).
The common tricks is to use before pseudoselector

image

(from https://caddyserver.com/docs/install)

@rpeden
Copy link
Contributor Author

rpeden commented Jun 9, 2023

Another option is just editing the docs to remove leading $ symbols. It's not used consistently throughout the docs, anyway - some code blocks containing terminal commands have a leading $, and some don't. I haven't counted, but eyeballing it, it looks the docs already exclude $ more often than they include it.

In most cases, removing the leading $ should not cause confusion. I haven't noticed any users confused about the lack of a $ in GitHub issues or community Slack, anyway. It also makes the docs more platform-agnostic, because no Windows terminals use a $ in their prompt.

This approach also requires no custom JavaScript, so there's one less thing that could break when updating to new Mkdocs versions.

There are a few code blocks that contain a terminal command and its output, and I think a $ before the command adds clarity here. However, I doubt anyone is copying these, because the copy button grabs both the command and its output.

Do you have any thoughts on this, @billpalombi and/or @discdiver? I know you have higher-priority items to tackle, so I'd be happy to update the docs if you think this is workable.

@znicholasbrown demonstrated another approach that only requires a little custom JS, and would give docs authors the option to specify alternate code to copy. I think that's still useful, but it seems like a separate feature you'd want even if commands in the docs have no leading $s.

@billpalombi
Copy link
Contributor

Hey @rpeden!

Thanks for spelling out that case for us. I agree that removing the $ is the simplest solution. I'll open a PR for that.

@discdiver
Copy link
Contributor

Thank you, Ryan. I support removing the $ for easier copying, standardized formatting, and simplified doc creation.

@rpeden
Copy link
Contributor Author

rpeden commented Jun 9, 2023

Thanks got the quick feedback! Since you plan a separate PR, @billpalombi, I'll close this one.

@rpeden rpeden closed this Jun 9, 2023
@billpalombi
Copy link
Contributor

See #9875

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

Successfully merging this pull request may close these issues.

6 participants