-
Notifications
You must be signed in to change notification settings - Fork 73
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
Conversation
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 submitting this PR! Added some initial feedback.
@@ -104,6 +104,42 @@ Write all titles and headings, including the titles of product documentation gui | |||
* _Configuring the node port service range_ | |||
* _How to perform an unsupported conversion from a RHEL-derived Linux distribution to RHEL_ | |||
|
|||
[[commands-in-code-blocks]] | |||
== Commands in code blocks |
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.
Can you move this up so that it is alphabetical? (Should be first in the file, before "Date formats")
I've also gone back and forth on whether this makes the most sense under formatting, or whether it would be better suited under "Technical examples". It is formatting, but I'm leaning toward the technical examples sense, just so that all of the code block misc is together. 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.
+1 For Technical examples
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.
Hm, I'd say it fits rather well under the "Formatting" section, since "Technical examples" primarily seems to deal with guidelines for the content itself rather than its markup.
Also, the "User-replaced values" parts, which deal with a very much related topic, including code block content (which is why I initially bunched up the new section with them), are under Formatting as well.
[[commands-in-code-blocks]] | ||
== Commands in code blocks | ||
|
||
As a default, use bold formatting for commands in code blocks, to visually distinguish them from their lead-in sentences and example outputs. |
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 know this sentence implies it, but I would add another sentence that states that output should not use bold formatting.
---- | ||
==== | ||
|
||
Disregard this guideline for example when the command is not an important part of what you want to show in the given code block. |
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 get what we're trying to say with this line, but I think this leaves a lot of room for people to ignore this guideline. Maybe we could leave it out?
Or maybe word it differently to state that if you are using bold to emphasize a different item in the code block, you are not required to bold the command?
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, I do want to leave people some room to manoeuvre around this if they feel so inclined. Your suggestion for the wording does make sense, though.
@@ -104,6 +104,42 @@ Write all titles and headings, including the titles of product documentation gui | |||
* _Configuring the node port service range_ | |||
* _How to perform an unsupported conversion from a RHEL-derived Linux distribution to RHEL_ | |||
|
|||
[[commands-in-code-blocks]] |
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 know we also briefly talked about stating to show a single command in a code block. And that if you have multiple commands, they should be broken into separate code blocks.
Should we consider adding an entry for this as well? Not sure if it would make sense to add into this one, or to separate them into different entries?
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.
You're right, that's a topic that falls quite squarely under "Commands in code blocks]", so I'll add a mention of that, too.
== Commands in code blocks | ||
|
||
As a default, use bold formatting for commands in code blocks, to visually distinguish them from their lead-in sentences and example outputs. | ||
|
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.
Do you think we need to clarify that you should still use bold even if there is no example 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.
I think so, good point.
|
||
.Example AsciiDoc: Command and its output in a code block | ||
|
||
Verify that the `libvirt` default network is active and configured to start automatically: |
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.
Maybe show this as a step, since that's how it would normally appear in a procedure? (If you do, update in the below rendered output as well
Verify that the `libvirt` default network is active and configured to start automatically: | |
. Verify that the `libvirt` default network is active and configured to start automatically: |
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.
Actually, I just tested this and it seems to break the formatting of the example in the render for some reason. So, given that it's not really important for the example lead-in sentence to be a step, I'd leave it out.
@@ -104,6 +104,42 @@ Write all titles and headings, including the titles of product documentation gui | |||
* _Configuring the node port service range_ | |||
* _How to perform an unsupported conversion from a RHEL-derived Linux distribution to RHEL_ | |||
|
|||
[[commands-in-code-blocks]] | |||
== Commands in code blocks |
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.
+1 For Technical examples
---- | ||
|
||
|
||
This renders as: |
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.
Incomplete sentence. How about: The previous AsciiDoc example outputs as follows
?
I'm suggesting this in the event that a new writer scans the RH SSG (to validate term usability) to check for instances of "This renders as" and they then apply it to customer 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.
This exact wording was actually used in multiple other examples in the section, and I just copied it as a "template". Doesn't change the fact that it's an incomplete sentence, so I'll update the other occurrences, too.
|
||
Disregard this guideline for example when the command is not an important part of what you want to show in the given code block. | ||
|
||
In addition, both in commands and outputs in code blocks, observe the correct mark-up for user-replaced values, as described in xref:user-replaced-values[] and xref:user-replaced-values-xml[]. |
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.
The three "in" instances makes me stop and reread the sentence. How about:
For a command and its output in a code block, observe the correct mark-up for user-replaced values, as described in xref:user-replaced-values[] and xref:user-replaced-values-xml[].
Kudos to Andrea Hoffer and Darragh Fitzmaurice for the feedback
@bergerhoffer @dfitzmau thank you for the reviews, hopefully the next iteration is a bit ship-shape-er :-) Diff: f0ddde1 |
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.
Left some suggestions.
Kudos to Miriam Portman for the suggestion Co-authored-by: Miriam Portman <[email protected]>
Kudos to Miriam Portman for the suggestion Co-authored-by: Miriam Portman <[email protected]>
Kudos to Miriam Portman for the suggestion Co-authored-by: Miriam Portman <[email protected]>
Thank you for the reviews @bergerhoffer @dfitzmau @mportman12 ! Your suggested changes have now been applied, feel free to take another look at the diff and merge the PR if everything looks copacetic. |
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 Some minor comments for your consideration :)
---- | ||
==== | ||
|
||
Do not use bold formatting for the output, unless you need to highlight a specific part of it. When you do emphasize a non-command item in a code block, you are not required to bold the command. |
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.
Do not use bold formatting for the output, unless you need to highlight a specific part of it. When you do emphasize a non-command item in a code block, you are not required to bold the command. | |
Do not use bold formatting in the command output. If you need to highlight a specific part of the output, use a different highlighting method. |
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:
Source: https://privatebin.corp.redhat.com/?d7d35e248ea634d9#6icGw5GgEyf71smCmNSsPem7bvZ6CFMBjiLVkA7zi8Uj
Co-authored-by: Breda McColgan <[email protected]>
Co-authored-by: Breda McColgan <[email protected]>
Co-authored-by: Breda McColgan <[email protected]>
Hi @mportman12 , I think you need to ack the changes (based on your comments from Oct 11) before the PR can be merged. Cheers, |
@jherrman I think you need ACK from Breda, not me. All my comments have been resolved. |
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.
LGTM
@bergerhoffer @dfitzmau (or anyone else with merging privileges), unless you have any other comments or suggestions as to the changes, please consider merging this sooner rather than later - so that this PR and #390 don't bloat their respective queues unnecessarily. Thanks! |
Hi @jherrman . Looks like Andrea is on PTO. I can merge this PR when the 3rd approval tick is provided. |
@jherrman Would you mind adding it to the agenda for the SSG meeting today, just to make sure all agree with the proposed change? |
I won't be able to attend the style guide meeting today due to a QPS meeting, but if that's not an issue, I can definitely pop this on the agenda for discussion. Alternatively, I'd leave it until the next meet-up. |
I can attend the first 30 minutes of today's meeting if that helps, but I'm happy to wait until you can attend if you prefer? I know you'd like to get this PR merged sooner rather than later, but I am uncomfortable with the text in line 43, as indicated by my comment above, and would like to get the council's opinion. |
@bredamc , sounds like a plan :-) I added the issue as the first one to be discussed at today's style meet-up. |
@jherrman Could you please provide an example of where the proposed formatting would be preferred over a different highlighting method? |
Added some examples to the thread above |
On OpenShift, we have a rule of one command per codeblock and the output should also be in its own codeblock. In saying this, I do see some edge cases in OCP docs where bolding some aspects of a large output block would help a customer see what key output lines they need to see to verify a command ran succesfully. The non-essential output would still hold value in that it provides a customer comfort that this output is also expected. As in a previous comment, callouts could be used, but callouts used with ifdef or ifndef statements can create a held-with-sticky-tape file, as I've found out recently. |
If I remember correctly, the stylesheets on https://docs.openshift.com automatically bold out everything in a code block (though probably in different typefaces based on the context). That way, keeping the command and the output together would create a long(ish) block of bolded text, which is not great for visual clarity either - so keeping them separate is very sensible. However, it also seems to create some "textbox bloat", which is something we generally try to avoid when possible. Therefore, given that Customer Portal stylesheets do not use bold typeface automatically, keeping the comand and the output together makes more sense in CP-based docs. This, nevertheless, raises a good question: Given that the RH SSG does not aim to govern only documentation that resides on the Customer Portal, how much do we want to be prescriptive about matters that may be governed by the stylesheets of the respective documentation site, and thus outside the writer's control?
Yeah, callouts do not work super-great in asciidoc in general, in my experience, and unless they are eminently useful in the given context, I usually prefer to avoid them. |
Based on the discussion on Slack , I've updated the guidelines about bolding in command output - 9346e54 @dfitzmau @bergerhoffer @bredamc , please take a look and either approve the PR or let me know if any other changes are needed. Cheers, |
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.
LGTM! Thank you for your patience, @jherrman :)
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.
LGTM
|
||
.Example AsciiDoc: A command and its output in a code block | ||
|
||
Verify that the `libvirt` default network is active and configured to start automatically: |
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!
3 approvals, merging! |
Update draft for #390