mdBook icon indicating copy to clipboard operation
mdBook copied to clipboard

Remove leading $, > and # chars from clipboard of console snippets

Open diegocn opened this issue 5 years ago • 15 comments

This PR aims to add a function to remove leading $, >, and # characters from the clipboard of console snippets.

Related PR: https://github.com/rust-lang/mdBook/pull/1110 Related issues: https://github.com/rust-lang/mdBook/issues/864, https://github.com/rust-lang/book/issues/1756, https://github.com/rust-lang/book/issues/2420, https://github.com/rust-lang/book/issues/2179,

diegocn avatar Sep 30 '20 02:09 diegocn

Hello folks, just a friendly reminder about this PR :D @Michael-F-Bryan @budziq, @steveklabnik, @frewsxcv

diegocn avatar Oct 08 '20 15:10 diegocn

I'm a little concerned this would be surprising behavior more than it'd be helpful behavior, so I don't feel comfortable merging it. Especially since it's trivial for a user to remove special leading characters from console output. Not blocking though in case someone else approves.

frewsxcv avatar Oct 08 '20 19:10 frewsxcv

Hi @frewsxcv. I did not get your point. Would you mind to elaborate a little bit more on the reason to not accept this PR?

From my point of view, as a user/consumer of the Rust book that uses mdBook, I expect that the action of copying/pasting console snippets just works without the need to remove a unnecessary leading char. The related PR/Issues shows that other users think the same.

diegocn avatar Oct 12 '20 03:10 diegocn

concerned this would be surprising behavior more than it'd be helpful behavior

I've encountered this behavior (i.e. copy button excludes shell prompts) on a few other sites in the past, and personally have always found it pleasantly surprising.

I can't think of many circumstances where this would be the "wrong thing" to do (and on the few occasions when it would be undesirable, it can be worked around trivially, by manually selecting the entire block, including the prompts - whereas removing the prompt characters is a more drawn out manual process).

Here's an online demo for another implementation of the same feature, which can be used to try out the behaviour (and workaround): https://sphinx-copybutton.readthedocs.io/en/latest/

Also here's another example of irritation with the current behaviour. The example cited is a (broken link) to the Embedded Rust "Discovery" mdBook, which has since moved here.

tim-seoss avatar Oct 29 '20 08:10 tim-seoss

Hi @tim-seoss, thanks for your comment.

I really like the Sphinx-copybutton example that you bring here.

diegocn avatar Nov 09 '20 18:11 diegocn

Here is another example of a copy button that removes the leading $ https://www.envoyproxy.io/docs/envoy/latest/start/install#install-envoy-on-debian-gnu-linux

diegocn avatar Nov 19 '20 13:11 diegocn

Here is another example of a copy button that removes the leading $ https://www.envoyproxy.io/docs/envoy/latest/start/install#install-envoy-on-debian-gnu-linux

Here's the GitHub Issue and PR for this feature: https://github.com/readthedocs/readthedocs.org/issues/4698 https://github.com/readthedocs/readthedocs.org/pull/4990

And here's the library they're using: https://sbrunner.github.io/sphinx-prompt/

This seems to be a different approach than the one proposed in this PR. Their approach requires the user to opt-in to making the prompt prefix symbol unselectable, whereas the approach in this PR is to remove all prompt prefix symbols whether the user wants it or not

frewsxcv avatar Nov 19 '20 17:11 frewsxcv

Looking at: https://sphinx-copybutton.readthedocs.io/en/latest/

By default, if sphinx-copybutton detects lines that begin with code prompts, it will only copy the text in those lines (after stripping the prompts). This assumes that the rest of the code block contains outputs that shouldn’t be copied.

When this default hasn't been opted out of, the user is still able to easily select the text, including the prompts i.e.

  1. Mouse or finger to drag-select (click-select, etc.) the text in question, then
  2. Copy via context menu etc.

The prompt stripping only occurs when the copy icon at the edge of the text block element.

If you consider the case where the user is presented with a list of 5 shell commands, and wants to execute them all, having visually inspected them...

i.e. With this functionality in place the user has: "I want the commands without the promtps" → Very easy "I want the commands with the prompts" → Easy

... you can verify this by experimenting with the example in the above link.

Without the functionality (or with it disabled), the user has: "I want the commands without the promtps" → Hard "I want the commands with the prompts" → Very Easy

I can't think of an occasion when I've wanted to copy shell commands with the prompts, but if I did, it would still be reasonably easy to do-so (and by an IMO intuitive method).

tim-seoss avatar Nov 23 '20 19:11 tim-seoss

I began reading the Book today, and I came here to find if there already was an issue or a PR about this. It would be a nice feature to have in my (very humble) opinion!

alsyia avatar Mar 03 '21 22:03 alsyia

I wanted this feature, so I implemented it in my project's custom mdBook back-end. The result is available online.

NB: As a comment above notes, it's very fragile (I won't even try disputing that, I'm fine with relying on impl details & having to update those bodges periodically for my own stuff), but it still produces something that works for the end user and can be judged, and the implementation will be cleaned up if this is deemed good enough to upstream.

The logic is as follows:

  • Only apply this filtering to the console language (which is just an alias for shell), since that's where you'll want this behavior.
  • Only apply it to the lines starting with $ ; this is not exactly what highlight.js does, though.
  • Remove the console language, since we don't want the lines themselves to be highlighted, and instead create inline spans tagged with the bash language, again imitating what hljs does. Lines that don't start with a prompt are not highlighted, anyway.

It's not the most general logic, it's what works for my case; the custom renderer is a bunch of bodges, so I didn't bother making it generic & clean, but I would bother if this was to be upstreamed.

Rationale:

  • This works JS-less, allowing plain copy-paste without the hassle, including if you only want some of those lines still without the prompt.
  • This also does not require tampering with the clipboard.
  • The creation of a separate console-line element allows giving it the hljs-meta class as well so that styling is not lost but not duplicated either.
  • This would be cleaner as a preprocessor, but I think I encountered a couple of snags? I'd have to check again. Shouldn't be a big deal in theory, since markup gets processed neither in code blocks nor in HTML tags.
  • This does duplicate some logic, but the alternative to that (while keeping the behavior) would be to scan all elements after hljs generated the lines, and performing the transformation then, which I like less because of the heavier CPU tax (DOM manipulation!) & still complex logic.

ISSOtm avatar Aug 31 '21 12:08 ISSOtm

Given how often people create issue on the rust book I think most users find this surprising that the $ is copied. The most recent issue. Counting off the top of my head this is at least the 2nd or 3rd this month.

c-git avatar Aug 14 '22 13:08 c-git

Independent of this PR as it currently stands, is there any consensus on whether this would a useful enhancement? I suppose the options are:

  • Never useful (bad UX)
  • Useful but should be an opt-in feature, perhaps just using a special "language" for this, e.g. with something like: ```shell-commands $ curl https://example.org
  • Useful if unconditionally applied

Personally, I think as an opt-in feature it would allow each individual book author to decide on whether this was appropriate, so wouldn't risk changing behaviour in a surprising way for any existing books. Any thoughts?

tim-seoss avatar Aug 16 '22 16:08 tim-seoss

I think it's useful to apply it to the console language specifically, being exactly the "shell commands preceded by prompts" language.

ISSOtm avatar Aug 16 '22 21:08 ISSOtm

Hi folks! I had forgotten this PR (as it's been here for almost 2 years). @ISSOtm that conditional does exactly what you have said.

I think it would be nice to have an opt-in if this is something that might cause unexpected behaviors when forced.

diegocn avatar Aug 23 '22 03:08 diegocn

I'd consider the console language to be a suitable opt-in, since it's one of at least three aliases to the same language.

ISSOtm avatar Aug 28 '22 20:08 ISSOtm

I ran into this issue again today when copying

$ curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf | sh

from https://doc.rust-lang.org/book/ch01-01-installation.html#installing-rustup-on-linux-or-macos.

hamirmahal avatar Oct 05 '23 23:10 hamirmahal

Closing this. There’s absolutely no point in keeping such a simple change waiting for 3+ years for a resolution.

diegocn avatar Oct 28 '23 02:10 diegocn