-
Notifications
You must be signed in to change notification settings - Fork 1.6k
🐛 fix(docs): display license as code block #5183
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
🐛 fix(docs): display license as code block #5183
Conversation
The license is now displayed as code block, not as normal text.
|
Hi @vitorfloriano. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vitorfloriano The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The appache license I think we must to hidden 100% |
docs/book/utils/litgo/literate.go
Outdated
| } | ||
|
|
||
| // skip if it's a license comment | ||
| if strings.Contains(lit, "Copyright") || strings.Contains(lit, "Licensed under") { |
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 need to hidden that 100%
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.
@camilamacedo86 I pushed a change in the logic. We first check if the code block is a license so we don't extract it, then we check if the collapsible is empty and skip it, then we put the source citation on top of the first code block that is apparent:

WDYT?
The program now skips the license from being extracted, then skips empty collapsible from being rendered, then puts the source citation on top of the first apparent code block.
| } | ||
|
|
||
| if len(lit) < 3 || lit[0] != '/' || lit[1] != '*' { | ||
| return false |
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.
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.
All imports and license ones bring no value at all
That is why they are collapsed.
The workaround was collapse them out to hidden but we can hidden 100%
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.
@camilamacedo86 I double-checked #5175 and it is hiding the collapsed licenses and imports. I'm closing this PR as duplicate.
|
Honestly, @camilamacedo86 I think we should rethink the whole literate extraction. It is basically trying to replicate what mdBook already does natively. IMHO, the literate extraction system got overly complex, perhaps due to clever solutions (hacks) to work around past limitations in mdBook. I'm planning on writing a design proposal to change how we extract code from the testdata samples using only mdBook's mdBook newer versions have some nice features that we can't use because our custom preprocessors (literate.go and markerdocs.go) are not compatible with them. We should strive towards moving away from them and rely on mdBook's features. They are more than sufficient for our usecase and would greatly reduce the overhead to maintain the docs updated, IMO. Due to the gargantuan size of this refactor, it could even be a project for a new mentorship! 🤩 How does that sound to you? |


This PR fixes the case in which the license would be extracted and displayed as normal text. Now the license is rendered as a code block.
Related to #4862 and #5175.
Before:
After: