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-detect command lines if 'data-filter' attribute present #1358

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Auto-detect command lines if 'data-filter' attribute present #1358

wants to merge 3 commits into from

Conversation

bradhowes
Copy link

@bradhowes bradhowes commented Mar 15, 2018

This change essentially performs the same operation as the existing code which processes the 'data-output' atrribute values, but in an automatic fashion.

When the 'data-filter' attribute is found in the <pre> element with a non-empty value, visit each line of the <code> content. If the line starts with the value found in the 'data-prompt' attribute, treat it as a command and wrap the text in <span class="command-line-command">. Otherwise, treat the line as command output and strip off any 'data-user', 'data-host', or 'data-prompt' attributes, just like the code which performs filter based on line numbers and ranges.

…empty.

This change essentially performs the same operation as the existing code which processes the 'data-output'
atrribute values, but in an automatic fashion.

When the 'data-filter' attribute is found in the <pre> element with a non-empty value, visit each line of the
<code> content. If the line starts with the value found in the 'data-prompt' attribute, treat it as a command
and wrap the text in <span class="command-line-command">. Otherwise, treat the line as command output and strip
off any data-user, data-host, or data-prompt attributes, just like the code which performs filter based on line
numbers and ranges.
@bradhowes
Copy link
Author

Pinging @chriswells0 as he is the author of the command-line plugin.

plugins/command-line/prism-command-line.js Outdated Show resolved Hide resolved
if (line.slice(0, promptText.length) == promptText) {
// We have a command -- strip off the prompt from the source text and wrap in <span>
content[i] = '<span class="command-line-command">' + line.slice(promptText.length + 1) +
'</span>';
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand this correctly, it means users who don't use the data-filter attribute won't benefit from the styled <span>. Am I right? Couldn't this behaviour be added in every cases?

Copy link
Author

Choose a reason for hiding this comment

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

I did not want to cause any possibly breaking change, hence the clear separation. I am more than willing to rework it if desired.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that would be better to have a predictible behaviour when the span is always present, indeed. But yes we need to make sure nothing breaks.

…empty.

This change essentially performs the same operation as the existing code which processes the 'data-output'
atrribute values, but in an automatic fashion.

When the 'data-filter' attribute is found in the <pre> element with a non-empty value, visit each line of the
<code> content. If the line starts with the value found in the 'data-prompt' attribute, treat it as a command
and wrap the text in <span class="command-line-command">. Otherwise, treat the line as command output and strip
off any data-user, data-host, or data-prompt attributes, just like the code which performs filter based on line
numbers and ranges.
@chriswells0
Copy link
Contributor

chriswells0 commented Mar 17, 2018

I like the concept for this functionality. I wonder if a more descriptive name could be used for the data-filter attribute to hint at what would be filtered (e.g., data-filter-prompt).

I have an open PR #856 from 2 years ago that automatically treats lines beginning with > as output, but we never really settled on the char to use as the indicator. I see that I also have local changes that were never merged into that PR. Perhaps if this PR used data-filter-prompt, we could then use data-filter-output=">" to do the reverse: provide a way for users to specify the leading character or character sequence for automatic output treatment. Obviously, you would only use 1 data-filter attribute at a time. Thoughts?

@Golmote
Copy link
Contributor

Golmote commented Mar 19, 2018

Hi @chriswells0! This sounds like a good idea. If you start working again on #856, feel free to @ me so I can review it.

@chriswells0
Copy link
Contributor

My apologies for the delay--been swamped at work during the week. I just updated #856 with my old changes modified to use data-filter-output to specify the prefix for output lines and rebased it.

@bradhowes, I realize this will require changes for your PR, so I was hoping to hear your thoughts before implementing the changes in mine. I just didn't expect to have more time soon, so I changed it while I could. I do believe the new approach will make it easier to accomplish what you're doing, though. Search for outputFilter to see that I've split prompt lines / code into one array with blank placeholders where output lines belong and stored the actual output lines in another array. After highlighting is done, the arrays get merged back together.

@bradhowes
Copy link
Author

@chriswells0 That sounds good to me. Thanks! I'm fine with closing this as a dupe of your work then.

@chriswells0
Copy link
Contributor

@bradhowes, I didn't mean it was a dupe. It's the reverse, which I think is a useful feature. My approach specifies the prefix for output lines, so you could use data-filter-output="out" (or any value) and it would treat lines beginning with "out" as output. I see both as useful depending upon whether you have more commands or output in a given code block.

@Golmote
Copy link
Contributor

Golmote commented Mar 26, 2018

@bradhowes, I merged #856, so you'll have to merge or rebase the gh-pages branch in order to address my remaining comment, and eventually get this PR merged.

@mAAdhaTTah
Copy link
Member

I'm still into this if we can get the conflicts merged.

@chriswells0
Copy link
Contributor

Sorry, I had a busy work week and just now had a minute to return to this. @bradhowes, like I said, I can see the benefit of having both options, so it's up to you if you want to merge it. I appreciate the contribution.

@mAAdhaTTah, in the meantime, you can also look at the data-filter-output attribute, which does the reverse of this PR.

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.

5 participants