-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Remove indentation causing RST errors #1495
base: main
Are you sure you want to change the base?
Remove indentation causing RST errors #1495
Conversation
@@ -616,9 +616,9 @@ $ source <(%[1]s completion bash) | |||
|
|||
To load completions for every new session, execute once: | |||
Linux: | |||
$ %[1]s completion bash > /etc/bash_completion.d/%[1]s | |||
$ %[1]s completion bash > /etc/bash_completion.d/%[1]s |
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 keep the indentation and add .. code:: sh
before? See https://docutils.sourceforge.io/docs/ref/rst/directives.html#code
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.
Would this be interpreted correctly for the help message @umarcor ?
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.
To be honest, I am genuinely confused. To my understanding, the help message is printed to screen/terminal, so unrelated to RST. My previous reply was considering that line 619 is going to end being part of a restructuredtext file. However, I now see why you are asking, since this is indeed the "Long" field of a command. Is the same text source used for both printing the help in screen and for generating the rst 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.
It's definitely used for the help message and I'm learning with this PR that it is also used for RST. So the original idea of removing the indentation is probably the best way forward. What do you think?
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 it is technically not correct to remove the indentation, because the resulting RST is not semantically valid. However, given the constraints, I believe it's fair enough. It allows removing the error found by @melissamahoney-mongodb and we don't need to learn the details of doc generation right now. I'll mark this dialogue as resolved, and we'll deal with docgen some other day 😉.
EDIT
It seems I cannot mark it as resolved myself. @melissamahoney-mongodb, do you mind marking, please?
This PR is being marked as stale due to a long period of inactivity |
I think this PR is giving problems:
@melissamahoney-mongodb can you specify how you get an error when generating RST doc? I haven't been able to reproduce it. @umarcor what do you think? I personally would pull this out of the release candidate until we can clarify the situation. |
@marckhouzam I removed this from #1496 and I pulled it out of #1525. It seems that supporting multiple output formats might require explicitly handling multiple text sources... |
We generate our docs in RST from Cobra. I removed the indentations causing an error in the generated RST. This also makes the long descriptions consistent with the other completion commands.