Add support for Command Line highlighting

[chriswells0]

Adds the ability to style shell examples as shown in this article:

https://chriswells.io/blog/view/style-shell-commands-with-css

Please let me know if I missed any files that I should have created or edited.

[zeitgeist87]

I really like your plugin. I think it would be a very useful addition to Prism. I have just a few remarks:

• First of all the file ./prism.js is auto-generated by gulp. If you want to change the core of Prism you have to edit the file ./components/prism-core.js.
• You seem to have an editor that removes trailing whitespace, which is very good, but you should put all unrelated whitespace changes of prism-core.js into a separate commit.
• You added your new language shell twice to the language list in ./components.js.
• Your language shell seems to be just a copy of bash. Why exactly do you need a new language anyway?

[chriswells0]

Thanks for the feedback. I wanted to respond sooner, but I had a busy week leading up to getting my wisdom teeth removed on Thursday.

• I'll revert ./prism.js and edit ./components/prism-core.js instead.
• Do you mean a separate commit or a separate PR for the whitespace?
• I added shell to ./components.js twice because I was trying to add a language and a plugin. If that's not the right approach, let me know.
• The shell language is currently just a copy of bash. The reason for creating a standalone language was three-fold:
  1. It felt more semantically correct.
  2. When the show-language plugin is used, shell and bash blocks are easily distinguished.
  3. If someone wanted to extend or modify the shell language to suit their tastes, they can easily do so. For example, as a FreeBSD user, I might want to override shell to use the commands/builtins that are part of csh instead of bash.

[zeitgeist87]

Do you mean a separate commit or a separate PR for the whitespace?

I meant as a separate commit, but a separate PR would also be great. If you create a separate whitespace PR it probably would be accepted immediately, because such a PR doesn't require any review. But this is just my opinion and not a requirement at all. I am also not a project owner so feel free to ignore my comments...

I added shell to ./components.js twice because I was trying to add a language and a plugin. If that's not the right approach, let me know.

Sorry that was my mistake. It is correct to add it as a plugin and a language.

The shell language is currently just a copy of bash.

We could also rename bash into shell or provide an alias. I think the bash language is actually meant as a general purpose bourne shell. But I think @Golmote or @LeaVerou should make this decision.

[chriswells0]

I'd suggest an alias rather than renaming bash to shell if a separate, stubbed language is not an option. I can imagine renaming would cause issues for people with existing code using bash for the language. Also, renaming wouldn't address the scenario of a bash script and interactive shell appearing on the same web page: they would show the same label while presenting different types of data. There could be cases where a single web page presents an interactive shell as well as a bash script to be created on the remote server.

I've reverted the changes to ./prism.js and made the same change (minus white space removal) to ./components/prism-core.js.

Please let me know if I should make further changes.

[zeitgeist87]

First of all I'd like to stress, that I am not a project owner of Prism, so you don't have to follow any of my suggestions 😄 . I just like your plugin and I wrote the Bash language plugin.

If you add a language to the list in ./components.js, it will show up on the website in the Download section as a separate language. I think two Unix-shell languages would confuse people.
Also the term shell is a bit ambiguous. I know it is commonly understood to mean Unix-shell or Bourne Shell, but it could also mean Powershell or Windows Shell etc. That is the reason why I chose bash for my language, but that is not ideal either. Maybe we could rename bash to unix-shell and provide an alias for bash and shell. I don't feel comfortable making this decision though, so this point definitely needs more discussion.

The name of the plugin is also a bit confusing. How about something like terminal-simulator, command-line, cli-simulator, command-line-prompt or something else more descriptive. What do you think?

It would be nice if the plugin could also simulate the appearance of a Windows Powershell and maybe other command line interfaces. You could simply add an additional attribute data-prompt. So if it is a Unix-shell one would use data-user and data-host. If it is anything else like Powershell, PHP or Python we could use data-prompt to provide the prompt directly.

[zeitgeist87]

One other thing: A PR should be about only one issue or feature. I think the plugin could be accepted right away.But if you bundle it up with lots of other more controversial changes that affect ./components/prism-core.js and other plugins, your PR is less likely to be accepted. I would suggest that you make this PR only about the plugin and create separate PRs for the other stuff you want added to Prism.

[chriswells0]

I'd like to stress, that I am not a project owner of Prism, so you don't have to follow any of my suggestions

That's OK. You're clearly more familiar with it than I am, and I appreciate the input. You also sound like someone who might actually use this plugin, so your feedback is even more relevant.

A PR should be about only one issue or feature.

This PR was meant to only add the language and plugin for styling shells. I'm not sure if you're suggesting the language should be removed until that discussion is complete or if you're referring specifically to the change that allows highlighting of kbd elements. That highlighting is actually part of the plugin, because it seemed more semantically correct to mark up interactive shells using kbd rather than code.

Maybe we could rename bash to unix-shell and provide an alias for bash and shell.

I think bash makes perfect sense for what it is. While bash isn't my preferred shell, it's a good base for other UNIX shells since many of the commands are the same. My original shell language actually used keywords/builtins from csh instead, but I thought I was being too specific to my own usage. The thing I'd like to ensure is that bash scripts and interactive shell examples can be presented on the same page without showing the same label from the show-language plugin. I'm open as to how to make that happen, though.

It would be nice if the plugin could also simulate the appearance of a Windows Powershell and maybe other command line interfaces.

I'm admittedly out of touch with Windows, so I didn't think about it at all. I thought creating the prompt via CSS would make it flexible enough, but it's a great idea to embed the entire prompt in the markup for the examples you gave. I'd even be on board with removing data-user and data-host in favor of specifying data-prompt for every case. What do you think?

The name of the plugin is also a bit confusing. How about something like terminal-simulator, command-line, cli-simulator, command-line-prompt or something else more descriptive.

The shell name makes sense to me, but I can see where you're coming from. Perhaps a name like terminal, cli, or command-line would be more descriptive and/or universal. Both shell and terminal are interchangeable in my mind, but obviously you're running a specific shell any time you're inside a terminal. I'd lean away from including "simulator" in the name since it's not really simulating in this case (more representational than interactive). Similarly, I'd leave "prompt" out of the plugin name and let that refer specifically to the prompt displayed for command input (i.e., the prompt is one part of the shell/cli).

[zeitgeist87]

That highlighting is actually part of the plugin, because it seemed more semantically correct to mark up interactive shells using kbd rather than code.

Ok.

The thing I'd like to ensure is that bash scripts and interactive shell examples can be presented on the same page without showing the same label from the show-language plugin. I'm open as to how to make that happen, though.

The show-language plugin is quite smart. The only thing you have to do is add the following line to the bottom of ./components/prism-bash.js:

// shell is an alias for bash
Prism.languages.shell = Prism.languages.bash;

Everything else should work automatically. You don't even have to change the file prism-show-languages.js, because the show-language plugin will generate an upper case Shell string by default.

<pre><code class="language-shell">ls -al</code></pre>

The show-language plugin works with a simple reference just as well as with your new language.

I'd even be on board with removing data-user and data-host in favor of specifying data-prompt for every case. What do you think?

I really like the data-user and data-host feature. I especially like that it automatically adds the correct # and $ signs. I also like that it can be freely styled with CSS. I would add data-prompt only as a fallback or for other languages.

The shell name makes sense to me, but I can see where you're coming from. Perhaps a name like terminal, cli, or command-line would be more descriptive and/or universal.

I like command-line but I am quite bad at naming stuff.

I'd lean away from including "simulator" in the name since it's not really simulating in this case (more representational than interactive).

I agree.

Similarly, I'd leave "prompt" out of the plugin name and let that refer specifically to the prompt displayed for command input (i.e., the prompt is one part of the shell/cli).

I would argue that prompt is the most important part. It is the main thing your plugin does. It gives the user the ability to easily add different command line prompts to any highlighted code in any language. But I don't want to influence you too much. You can name it however you like.

[zeitgeist87]

You also have to include your language here:

<script src="components/prism-shell.js"></script>

[chriswells0]

Are you saying the language or alias should be defined in components/prism-shell.js?

[zeitgeist87]

Are you saying the language or alias should be defined in components/prism-shell.js?

No I just meant that however we eventually decide to solve the language question, you have to include that language here for it to work.

[chriswells0]

I'm still pondering what to do about the language and the plugin name. Renaming the plugin will require a lot of changes, so I want to be sure before doing that.

[chriswells0]

Give me your thoughts on this, please:

As I stated before, I wanted to be able to distinguish shell scripts and sessions when shown on the same web page. Specifically, I didn't want the Show Language plugin to display Bash for both.

Adding an alias for bash (e.g., shell, terminal, or whatever) seems to be a "good enough" solution for that. However, now that the plugin supports other types of prompts, it doesn't seem to make sense anymore. The plugin could be displaying any type of code in the interactive session, so the highlighting may need to be Bash, PHP, PowerShell, or anything else.

I'm not sure how I'd prefer my code snippets to be labeled in a case where there are shell scripts and shell sessions on the same page, but maybe it should stick with the name of the language being highlighted and remove the new language completely. Of course, I (or others) could always add an alias in my own JS if I want Show Language to display a different value on my site.

[zeitgeist87]

I would love to use your plugin on my website, but I don't use the show-language plugin and I am not a semantic tag purist, so I don't really care about the kbd tag. My point is, that your plugin works perfectly fine without these things. If you remove the show-language support and the kbd tag this PR would boil down to a neat, self-contained and still very useful plugin.

This doesn't mean that adding the kbd tag is a bad idea, but it definitely needs a separate PR, where it can be discussed on its own. Adding a new tag is kind of a big deal. It will take a lot of discussion, testing and convincing.

The best approach for your issue with show-language would probably be to patch the show-language plugin directly (with another PR) . You could add an attribute data-language-alias or data-rename-language, which one could use to specify a custom name:

<pre class="shell" data-user="chris" data-host="remotehost" data-output="2">
<code class="language-bash" data-language-alias="My Favorite Shell">pwd
/usr/home/chris/bin</code></pre>

<pre><code class="language-python" data-language-alias="Python 2.7.1.1">....</code></pre>

<pre><code class="language-python" data-language-alias="Python 3.1.4">....</code></pre>

<pre><code class="language-python" data-language-alias="PyPy">....</code></pre>

Try to see it from the reviewers perspective. Every PR that gets accepted is immediately published on the website. Thousands of people use Prism. So the reviewers have to make absolutely sure that your changes don't adversely affect other stuff. If your PR changes things all over the place, it takes more work to review. If your PR is a self-contained plugin that touches nothing else, the review takes 5 minutes.

Also once people start using your plugin, you have a better argument for the kbd tag.

[chriswells0]

I've made the suggested changes and updated the documentation page to include an example using the data-prompt attribute. Is there a way for me to notify the project owner that I'm finished making changes related to this PR?

I do believe an alias attribute would be a great addition to the show-language plugin, so I'll make that change and submit a separate PR.

This is my first GitHub PR and I'm new to Prism, so thanks for the guidance as well as the feedback.

[zeitgeist87]

data-output="2-19" is missing here.

[chriswells0]

I can't believe I did that. I noticed it when I pasted the code from that file to another, but forgot to put the fix back into my git branch. It's there now.

[zeitgeist87]

I've made the suggested changes and updated the documentation page to include an example using the data-prompt attribute.

It looks good to me 👍 . There are currently 7 commits in this PR and they include all the changes you made so far. Since the PR will eventually end up in the Prism git-log it is considered nice to squash everything down into a single commit and give it a nice descriptive commit message.

Is there a way for me to notify the project owner that I'm finished making changes related to this PR?

The project owner usually gets notified automatically by email. They seem to be a bit busy at the moment so you have to be a bit patient. You can address them directly by using tags in your message. As far as I know @LeaVerou is the original author of Prism and @Golmote is very active and has admin rights for the project.

This is my first GitHub PR and I'm new to Prism, so thanks for the guidance as well as the feedback.

You're welcome. Thanks for the great plugin and your contribution to Prism.

[chriswells0]

It's rebased and committed. Should look a lot cleaner now.

[chriswells0]

If I may, I have one more question: when making an update to someone else's plugin, would it be a better practice to submit the PR to that plugin maintainer's fork and let them upstream the changes or submit the PR to the main project?

For example, I submitted a PR to add support for a data-language-label attribute in the show-language plugin, but I thought it might have been better to submit that to nauzilus so he could provide feedback/changes first.

[uranusjr]

Based on my observations, I think the current practice is to just submit the PR here, and just ping the main author/maintainer of the plugin for suggestions and review in it.

[chriswells0]

Thanks!

[LeaVerou]

Hi there, sorry I’m late to this.
Thanks @chriswells0 for the contribution and @zeitgeist87 for assisting with the review!
Quesiton: Why a plugin and not just a language definition?
Sorry if this has been answered, it's a long thread and I’m in a rush…

[zeitgeist87]

Hi @LeaVerou,

Quesiton: Why a plugin and not just a language definition?

That is a good point. A language definition would definitely be possible. The following points are some of the advantages of the plugin in my opinion:

• I guess the main advantage of the plugin is, that it works for all languages. Most scripting languages like Python, Lua, PHP and even JavaScript have an interactive shell. With the plugin the user can define an arbitrary command prompt.
• The plugin is also very convenient to use. Especially if you use the default prompt for Unix/Linux shells.
• The command prompts created by the plugin cannot be copy and pasted. Usually the user only wants to copy the command and not the prompt. The plugin makes this process easier.

But a command-line language definition would definitely be a viable alternative. I would implement it something like this (not tested):

Prism.languages['command-line'] = {
    'bash-command': {
        pattern: /(^\[\w+@\w+\s+\]\s+[$#]).*/m,
        lookbehind: true,
        inside: Prism.languages.bash
    },
    'unix-prompt': /^\[\w+@\w+\s+\]\s+[$#]/
}

[chriswells0]

In addition to the benefits listed by @zeitgeist87, the plugin also allows you to have some lines display the prompt while other lines are output (no prompt) without manually adding the prompt to every line where it should appear.

[zeitgeist87]

Instead of using an output class you could also simply remove the relevant attributes on the output lines:

var node = prompt.children[j - 1];
node.removeAttribute('data-user');
node.removeAttribute('data-host');
node.removeAttribute('data-prompt');

With some small rearrangements of the CSS code we can avoid the output class altogether. What do you think?

[chriswells0]

The output class makes it very simple to exclude the prompt from some lines, so I'd be hesitant to remove that. Where are you suggesting the sample code you provided would go? I may be misunderstanding, but do you mean the user would need to add that script for the lines that should be rendered as output?

[zeitgeist87]

Where are you suggesting the sample code you provided would go?

Sorry my comment was unclear. My sample code should replace the line number 66 in plugins/command-line/prism-command-line.js

I may be misunderstanding, but do you mean the user would need to add that script for the lines that should be rendered as output?

No not at all.

Here is how I would modify the CSS:

.command-line-prompt > span:before {
    color: #999;
    content: ' ';
    display: block;
    padding-right: 0.8em;
}

.command-line-prompt > span[data-user]:before, .command-line-prompt > span[data-host]:before {
    content: "[" attr(data-user) "@" attr(data-host) "] $";
}

.command-line-prompt > span[data-user="root"]:before {
    content: "[" attr(data-user) "@" attr(data-host) "] #";
}

.command-line-prompt > span[data-prompt]:before {
    content: attr(data-prompt);
}

On the other hand the output class would allow the user potentially greater freedom to create her own CSS style for the output lines.

[zeitgeist87]

I think this PR is ready to be merged. You don't have to implement my suggestion about the output class. It was just an idea I had while reviewing your code.

[chriswells0]

Sorry, it's been a while since I looked at this code. I thought you were suggesting removal of the data-output attribute rather than the output class. I could go either way on that: I don't mind removing it if you think there's a benefit in simplifying it further.

[chriswells0]

Thanks for merging this! I was in the process of removing the output class, but I can submit that change as a separate PR.

I was updating the Prism code on my site today, so I went back to grab the latest after you merged my 2 PRs. I notice that it fails to fetch the minified version of the JS for this plugin. I didn't include a minified version because I expected it to be created as part of the build process. Should I minify it myself and submit a PR to add it?

[zeitgeist87]

I notice that it fails to fetch the minified version of the JS for this plugin.

Thanks for pointing that out.

I didn't include a minified version because I expected it to be created as part of the build process.

We use gulp to auto-generate the minified versions.

Should I minify it myself and submit a PR to add it?

I've already commited a minified version, so there is no need for a PR. It would be nice if you could include minified versions in future PRs though.

[LeaVerou]

Hi all! Thanks @chriswells0 for contributing and @zeitgeist87 for reviewing!

A few remarks:

You seem to have an editor that removes trailing whitespace, which is very good, but you should put all unrelated whitespace changes of prism-core.js into a separate commit.

You can use ?w=1 to strip whitespace-only changes when reviewing a diff. Historically we have been allowing stripping trailing whitespace. Other whitespace changes do indeed need a separate commit or even PR sometimes.

It is correct to add it as a plugin and a language.

I’m a bit uneasy with that. I don’t see such a need. There are many language definitions with code, let’s not clutter the plugin listing with things that, semantically, are not plugins. @zeitgeist87 had a good suggestion about how to turn this into just a language definition.

[zeitgeist87]

You can use ?w=1 to strip whitespace-only changes when reviewing a diff. Historically we have been allowing stripping trailing whitespace. Other whitespace changes do indeed need a separate commit or even PR sometimes.

Thanks I'll keep that in mind. I didn't know Github had a strip-whitespace-changes feature.

It is correct to add it as a plugin and a language.

I was referring to a now obsolete version of the PR with that statement. The original PR included a new language shell that was just a copy of bash. The reason for that was to get the show-language plugin to show the string "Shell" instead of "Bash". But #837 solves this much more elegantly.

I’m a bit uneasy with that. I don’t see such a need. There are many language definitions with code, let’s not clutter the plugin listing with things that, semantically, are not plugins. @zeitgeist87 had a good suggestion about how to turn this into just a language definition.

Sorry if I merged this too early. In my mind this is more like a line marker than a full blown language. The commands are marked with a prompt. But it would be quite easy to convert it into a language-definition. @LeaVerou shall I remove the plugin for now?

[chriswells0]

Historically, how do you distinguish between a language and plugin? I'd expect the language to refer to the syntax/highlighting and plugins to add meta-functionality.

If this is converted to a language, how would that impact the ability to use any language you want with it?

[zeitgeist87]

If this is converted to a language, how would that impact the ability to use any language you want with it?

You would have to explicitly add support for any language you want to use. That being said, a language-definition would have other advantages:

• It would look a bit more natural, because the prompt is part of the source code
• You would only highlight the lines with prompts and not the output lines, which can prevent false matches of keywords and operators in the output of a command

[chriswells0]

You would have to explicitly add support for any language you want to use.

I don't want to waste your time here, but would you mind giving an example? Rough pseudo-code is fine. I'm trying to imagine how that would work from the user's perspective with one or more code blocks on a given page/site and a variety of potential languages.

the prompt is part of the source code

This seems unnatural to me. The prompt is merely the setting you're in. Imagine if anyone who wanted line numbers had to number them manually when they're not actually part of the code.

You would only highlight the lines with prompts and not the output lines, which can prevent false matches of keywords and operators in the output of a command

I like the thought of that. I considered the possibility of false positives but hadn't addressed it yet. As a plugin, we could do this with a command-line-output CSS class applied to the output lines. Is there an existing way to tell languages not to highlight certain lines? It would need to apply to the prompt portions as well since they would become part of the code in this case.

[zeitgeist87]

I don't want to waste your time here, but would you mind giving an example? Rough pseudo-code is fine. I'm trying to imagine how that would work from the user's perspective with one or more code blocks on a given page/site and a variety of potential languages.

It would look something like this (not tested):

Prism.languages['command-line'] = {
    /*
     * Bash support
     */
    'bash-command': {
        pattern: /(^\[\w+@\w+\s+\]\s+[$#]).*/m,
        lookbehind: true,
        inside: Prism.languages.bash
    },
    'unix-prompt': /^\[\w+@\w+\s+\]\s+[$#]/,

    /*
     * Powershell support
     */
    'powershell-command': {
        pattern: /(^PS .*?>).*/m,
        lookbehind: true,
        inside: Prism.languages.powershell
    },
    'powershell-prompt': /^PS .*?>/,

    /*
     * Support for other languages
     */
}

You just explicitly match the prompts in your source code and use the matched language for the inside attribute.

This seems unnatural to me. The prompt is merely the setting you're in. Imagine if anyone who wanted line numbers had to number them manually when they're not actually part of the code.

I agree that its merely the setting the command is executed in. I was trying to say, that it would look a bit more like a real terminal window, since in a terminal everything is monospace text of the same size, including the prompt.

Is there an existing way to tell languages not to highlight certain lines?

Not that I am aware of.

[zeitgeist87]

I don't want to waste your time here, but would you mind giving an example? Rough pseudo-code is fine. I'm trying to imagine how that would work from the user's perspective with one or more code blocks on a given page/site and a variety of potential languages.

The end user would use it like this:

<pre class="language-command-line"><code>[andreas@terok]$ date
Mit Dez 30 21:11:42 CET 2015</code></pre>

[chriswells0]

So the language code would loop through the *-command properties looking for a matching pattern and then use the associated inside property to decide which language to use for highlighting? Forcing the user to hard-code prompts in order to match the language for highlighting seems limiting. How about an attribute such as data-language to simply specify it?

Additionally, if we want to allow special highlighting/styling of the prompt, there could be a data-prompt attribute that accepts either the exact prompt being used or a regex of it.

We'd be limiting the supported languages to those that are explicitly defined in this meta-language, but we could start with a pretty good set (Bash, PHP, Python, JavaScript, PowerShell, Batch) and expand it on request. That could be a barrier to use, though.

As someone who expected to use this plugin, which approach would you prefer as a user? I'd try the meta-language approach with an open mind if you guys believe it's an improvement, but I'm still leaning toward the plugin for my own use because I don't want to hard-code shell prompts on every line. I'd likely add the command-line-output CSS class to prevent false positives and possibly set the font to monospace.