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

Command Line: Add support for line continuation #3326

Merged
merged 13 commits into from
Feb 10, 2022

Conversation

at055612
Copy link
Contributor

@at055612 at055612 commented Feb 9, 2022

Comand line plugin

Adds the following data attributes to allow you to show multi-line commands that have line continuation characters/strings.

  • data-continuation-prompt
  • data-continuation-str

This effectively changes the prompt for the continued lines as show in the updated help page for the plugin. If data-continuation-str is not set then the plugin will behave as before, though maybe it should default to \ for posix shell.

image

This is how it looks without this PR. It is hard to see what is a command and what is a continued line.

image

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of +129 B (+12.9%).

file master pull size diff % diff
plugins/command-line/prism-command-line.min.js 998 B 1.13 KB +129 B +12.9%

Generated by 🚫 dangerJS against 88cd35d

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Thank you for the cool PR @at055612!

The general approach looks good to me. I just a few minor nits here and there.

plugins/command-line/index.html Outdated Show resolved Hide resolved
plugins/command-line/prism-command-line.js Outdated Show resolved Hide resolved
plugins/command-line/prism-command-line.js Outdated Show resolved Hide resolved
plugins/command-line/prism-command-line.js Outdated Show resolved Hide resolved
plugins/command-line/prism-command-line.js Outdated Show resolved Hide resolved
at055612 and others added 3 commits February 10, 2022 08:54
Co-authored-by: Michael Schmidt <[email protected]>
Opacity works better with the different themes.
Preventing user-select on shell output allows you to copy just the
commands for pasting into a shell.
@at055612
Copy link
Contributor Author

While I was in there I also tweaked the styling for the prompts and output so they use opacity instead of a colour which seems to work better with the different themes.

image
image

I also prevented user-select on the shell output so you can copy just the commands which I think is what most people would want to do. Should have done this as part of the previous PR I did, sorry.
image

@RunDevelopment
Copy link
Member

I agree with the color change but not with the user-select: none.

People not wanting to select the output is a pretty big assumption. If they really didn't want the output to be selectable, then they could still add that themselves. However, it's a no-go for a syntax highlighter to make the highlighted non-selectable by default.

I also want to point out that the color change should have been its own PR. The change is completely independent of the line continuation feature but I can only merge it once we have line continuations ready. You can keep the color change in this PR, but, in the future, please try to make many smaller PRs instead of one big one.

@at055612
Copy link
Contributor Author

I agree with the color change but not with the user-select: none.

People not wanting to select the output is a pretty big assumption. If they really didn't want the output to be selectable, then they could still add that themselves. However, it's a no-go for a syntax highlighter to make the highlighted non-selectable by default.

I thought it might be contentious. I will remove it and add some words to the html page saying how to do it you want to.

I also want to point out that the color change should have been its own PR. The change is completely independent of the line continuation feature but I can only merge it once we have line continuations ready. You can keep the color change in this PR, but, in the future, please try to make many smaller PRs instead of one big one.

Yes, sorry, was a bit naughty but I figured it was a tiny change that I could sneak in. Noted.

@RunDevelopment RunDevelopment merged commit 1784b17 into PrismJS:master Feb 10, 2022
@RunDevelopment
Copy link
Member

Thank you for contributing @at055612!

@at055612
Copy link
Contributor Author

@RunDevelopment Thanks for the merge. Sorry for all the back/forth.

@at055612 at055612 deleted the cli-line-continuation branch February 11, 2022 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants