-
Notifications
You must be signed in to change notification settings - Fork 289
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
Change code example type #3619
Change code example type #3619
Conversation
The new docs site uses `console` instead of `terminal` to be compatible with Pygments.
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
@@ -104,7 +104,7 @@ func generateMarkdownPage( | |||
p("\n\n") | |||
if command.Runnable() { | |||
p("### Usage\n") | |||
p("```terminal\n$ %s\n```\n\n", command.UseLine()) | |||
p("```console\n$ %s\n```\n\n", command.UseLine()) |
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.
We'll also want to update this in the processDescription
function below, and make sure we update the function documentation.
For that function though, I did notice that while we're typically showing usage of buf
in some console-esque context, we'll occasionally mis-classify some non-console text — e.g. on https://buf.build/docs/reference/cli/buf/generate/, the "template file" YAML is classified as terminal
and would be changed to console
(when it should really be yaml
). I think it's ok to come back and fix that separately, if at all.
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.
Yeah, I think it's fine to unify all to terminal
. I'm not sure there is a better way to deal with different types of code blocks in descriptions unless the description itself passes the code block type. But yeah, definitely something to consider later.
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.
s/terminal/console
and I agree with you :)
} | ||
if len(command.Long) > 0 { | ||
p("### Description\n\n") | ||
p("%s \n\n", processDescription(command.Long)) | ||
} | ||
if len(command.Example) > 0 { | ||
p("### Examples\n\n") | ||
p("```\n%s\n```\n\n", processDescription(command.Example)) | ||
p("```console\n%s\n```\n\n", processDescription(command.Example)) |
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 is fine to do, but I don't think we use command.Example
— I was more referring in my comment to the function down on line 195; we'll want to update that from terminal
-> console
in the code and probably the comments above it. Happy to push a commit if you'd like me to!
Yep, realized that after committing this—working on it. |
The new docs site uses
console
instead ofterminal
to be compatible with Pygments.