docs: Update generated #1385
docs: Update generated #1385
Conversation
github-actions
bot
added
documentation
control/skip-ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates generated documentation files. There are a few issues:
- Typos in
bootc-container-lint.mdandbootc-rollback.md. --run-fetch-checkand--skip-fetch-checkhave identical descriptions across allbootc-install-*man pages.
| **\--run-fetch-check** | ||
|
|
||
| : This is now the default and has no effect. | ||
| : Verify the image can be fetched from the bootc image. Updates may | ||
| fail when the installation host is authenticated with the registry | ||
| but the pull secret is not in the bootc image |
There was a problem hiding this comment.
The descriptions for --run-fetch-check and --skip-fetch-check are identical, which is confusing. The description for --skip-fetch-check should indicate that it disables verification.
| **\--run-fetch-check** | ||
|
|
||
| : Verify the target image can be pulled using the bootc image. | ||
| : Verify the image can be fetched from the bootc image. Updates may | ||
| fail when the installation host is authenticated with the registry | ||
| but the pull secret is not in the bootc image |
There was a problem hiding this comment.
The descriptions for --run-fetch-check and --skip-fetch-check are identical, which is confusing. The description for --skip-fetch-check should indicate that it disables verification.
| : Verify the target image can be pulled using the bootc image. | ||
| : Verify the image can be fetched from the bootc image. Updates may | ||
| fail when the installation host is authenticated with the registry | ||
| but the pull secret is not in the bootc image |
There was a problem hiding this comment.
The descriptions for --run-fetch-check and --skip-fetch-check are identical, which is confusing. The description for --skip-fetch-check should indicate that it disables verification.
docs/src/man/bootc-container-lint.md
Outdated
| : Dont truncate the output. By default, only a limited number of | ||
| entries are shown for each lint, followed by a count of remaining | ||
| entries |
There was a problem hiding this comment.
Typo: "Dont" should be "Don't".
| : Dont truncate the output. By default, only a limited number of | |
| entries are shown for each lint, followed by a count of remaining | |
| entries | |
| : Don't truncate the output. By default, only a limited number of | |
| entries are shown for each lint, followed by a count of remaining | |
| entries |
There was a problem hiding this comment.
I'm somewhat confused where the ' is getting lost, since it's clearly there in the docstring - https://github.com/bootc-dev/bootc/blob/main/lib/src/cli.rs#L283
There was a problem hiding this comment.
I guess this is a bug in pandoc because the generated manpage under target looks ok, but we use pandoc to translate that to the markdown file.
There was a problem hiding this comment.
Actually I think we need to escape them in the manpage, so pandoc is fine but the way we generate from clap -> man is not quite right.
There was a problem hiding this comment.
But if I just run man target/man/bootc-container-lint.8 it displays ok with the single quotes... so I guess that's valid? 🤷
There was a problem hiding this comment.
Yeah the pipeline is a beast and when I tried to debug it it was really painful.
Honestly I'm fine to try to just bite the bullet and also stop generating the docs automatically. There has to be some projects that are doing this at least in a semi-automated way. I could definitely imagine scripts to keep it in sync.
There was a problem hiding this comment.
Well I have to admit I spent a little bit too much time today looking at this, but this is kind of a wild rabbit hole.
clap_mangen is ultimately using the roff crate to do the rendering. There's this lovely bit in the roff code that explains this special handling of apostrophes with \*(Aq in order to be portable. And then it looks like because it's being rendered behind a macro(?), that pandoc can't quite follow so it just drops it when it encounters it:
$ pandoc --verbose --from=man --to=markdown --output /dev/null target/man/bootc-container-lint.8
[INFO] Skipped '.ie' at target/man/bootc-container-lint.8 line 1 column 4
[INFO] Skipped '.el' at target/man/bootc-container-lint.8 line 2 column 4
[INFO] Skipped 'unknown string Aq' at target/man/bootc-container-lint.8 line 21 column 130
[INFO] Skipped 'unknown string Aq' at target/man/bootc-container-lint.8 line 29 column 6
[INFO] Skipped 'unknown string Aq' at target/man/bootc-container-lint.8 line 32 column 34
[INFO] Skipped 'unknown string Aq' at target/man/bootc-container-lint.8 line 32 column 42
TIL pandoc is haskell, so I only roughly understand what is going on, but here is where it seems to be skipping over it.
Fun.
There was a problem hiding this comment.
Hmm, yeah let's look at what other projects do. I did some digging on that and found
generate approaches
- https://github.com/BurntSushi/ripgrep/blob/cbc598f245f3c157a872b69102653e2e349b6d92/crates/core/flags/doc/man.rs (unclear to me whether it's better than clap_mangen? It may just predate it)
manual approaches
In this approach we stop trying to generate from clap, and just require a copy. (But I'm sure some tools (esp with a bit of AI) could help us keep them in sync)
- https://github.com/containers/podman/tree/main/docs/source/markdown looks pretty reasonable as a framework
There was a problem hiding this comment.
Alternate simple approach that I think will work if i can just convince xshell to stop being dumb with quoting: use sed to rewrite the header on the generated manpage to remove the conditional definition and then pandoc can do the right thing.
In general I think the clap generation works pretty well, this is just some silly edge case.
docs/src/man/bootc-rollback.md
Outdated
| from that previous deployment instead. | ||
|
|
||
| This is because \`bootc rollback\` just reorders the existing | ||
| deployments. It doesnt create new deployments. The \`/etc\` merges |
There was a problem hiding this comment.
Typo: "doesnt" should be "doesn't".
| deployments. It doesnt create new deployments. The \`/etc\` merges | |
| deployments. It doesn't create new deployments. The `/etc` merges |
jeckersb
force-pushed
the
update-generated
branch
from
1f40c5c to
5fe13c2
Compare
|
I added a commit to post-process with sed and then regenerated everything with that. We'll see how that goes. |
xtask/src/xtask.rs
Outdated
| cmd!(sh, "sed -i") | ||
| .arg("-e") | ||
| .arg(r"1i .ds Aq \\(aq") // insert the groff variant unconditionally | ||
| .arg("-e") | ||
| .arg(r"/\.g \.ds Aq/d") // remove generated if conditional | ||
| .arg("-e") | ||
| .arg(r"/.el .ds Aq '/d") // remove generated else conditional | ||
| .arg(&path) | ||
| .run()?; |
There was a problem hiding this comment.
I'm ok with this as but note a big part of the idea of cmd! is that it properly shell quotes automatically, so this would be more readable like
let groffsub = r"1i .ds Aq \\(aq";
let dropif = r"/\.g \.ds Aq/d";
let dropelse = r"/.el .ds Aq '/d";
cmd!(sh, "sed -i -e {groffsub} -e {dropif} -e {dropelse} {path}).run()?;
or so
There was a problem hiding this comment.
When I had it as part of the macro command itself and not chained off of the returned Cmd, it was giving some silly error about an unmatched ' or something along those lines. May have very well been something silly I was doing, though. I'll double check it.
There was a problem hiding this comment.
👍 ok it does work now like that, so obviously it was something I had wrong at the beginning. Updated!
Signed-off-by: John Eckersberg <[email protected]>
Signed-off-by: John Eckersberg <[email protected]>
jeckersb
force-pushed
the
update-generated
branch
from
5fe13c2 to
1b3f141
Compare
2 participants
Signed-off-by: John Eckersberg [email protected]