Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Issue#390 - bold commands in codeblocks #413
Issue#390 - bold commands in codeblocks #413
Changes from 3 commits
8ce04c0
5e81685
f0ddde1
f7d7cbd
358e3d7
8095c71
9c97313
31ead15
ef1efcd
9346e54
a05bcc0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only feedback is whether this example should actually be shown as a bullet or numbered step. But I don't think that's important enough to hold up the PR over since it has 2 other approvals. Just something to consider if we should adjust in a separate PR or not!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is good guidance. Writers should still be able to use bolded strings in outputs if necessary, especially if in the given code block, the output is more important than the command itself. Also, it would be rather strange to express the same kind of emphasis differently in the very same code block.
Your other suggestions are decidedly improvements, though, thank you :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jherrman Apologies for the delay in replying to your comment. It seems odd to me to say bold the command in one place and not in another, but perhaps I misunderstood the intent of this update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the idea is basically "bold the important part, so that it's visually distinct" - and since the command itself is usually the important part of a CLI example (and the given step in general), it would be the default part do bold. However, when a part of the output is as important as the command in the given context, or more, we should not bar the writer from bolding something else than the command, and potentially even leaving the command un-bolded.
It's entirely possible I did not formulate that clearly enough in the actual source update, though...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jafiala Can you give us an example of what you're thinking of with bolding pieces of the output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the examples, @jherrman.
I think it's better to highlight such lines in the command output by using a callout instead of bold text? The following example shows callouts (in a code example, but the same method could be used for command output):
https://docs.openshift.com/pipelines/1.13/about/understanding-openshift-pipelines.html
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, callouts do not seem like an optimal solution to me.
They are pretty wonky to work with in adoc (for instance can only be at the end of a line, cannot be used out of sequence, one callout cannot be used repeatedly, and they must always have an explanation under the codeblock, AFAIK). Also, Customer Portal stylesheets do not render them very well, currently.
When they work as expected, callouts are visually very distinct, which may not always be what you want when highlighting a small part of a command output.
If the general intention behind this update is "bold out the operative/important parts of the code block for greater visual clarity", I would say the output should follow that consistently with the commands. Also, it's easier for writers to internalize and follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jherrman and @dfitzmau Thank you both! I understand that callouts might not be optimal, but I wonder if we could highlight the command output in some other way rather than by reusing the bold format.
Do you know whether the highlight syntax described here would render correctly in Customer Portal and OpenShift docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that syntax a few years ago on the CP and it did render, but I was informed to drop it as the yellow highlighting was really distracting. It definetly grabs attention, but it distracts from the surrounding content. I can understand why yellow is a well-adopted colour for safety 💛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested out the
#highlight#
markup earlier today. Renders reasonably well in asciidoctor (though, as Darragh points out, the colour is a bit much), but both bccutil and the Customer Portal stylesheets completely ignore it, unfortunately:https://dxp-docp-prod.apps.ext-waf.spoke.prod.us-west-2.aws.paas.redhat.com/documentation/en-us/zen_and_zany_asciidoc_training_titles/42/html-single/davozenis_advanced_asciidoc_training/index?lb_target=stage#proc_test-two_assembly_nested-test
Source: https://privatebin.corp.redhat.com/?d7d35e248ea634d9#6icGw5GgEyf71smCmNSsPem7bvZ6CFMBjiLVkA7zi8Uj