Skip to content
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 usage and hyperlinks in kubectl reference #142

Merged
merged 2 commits into from
Mar 28, 2020

Conversation

brianpursley
Copy link
Member

@brianpursley brianpursley commented Mar 26, 2020

Fixes command usage missing kubectl
Fixes subcommand usage missing kubectl <maincommand>
Fixes problem with hyperlinks surrounded by [ ] not displaying correctly...
Fixes kubernetes/website#18444

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 26, 2020
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 26, 2020
@brianpursley
Copy link
Member Author

Generating a preview now...

@brianpursley
Copy link
Member Author

Here is a kubectl reference docs preview generated using the code in this PR:

https://deploy-preview-19864--kubernetes-io-master-staging.netlify.com/docs/reference/generated/kubectl/kubectl-commands

@brianpursley
Copy link
Member Author

/assign @steveperry-53

@brianpursley
Copy link
Member Author

/cc @kbhawkey

@tengqm
Copy link
Contributor

tengqm commented Mar 27, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brianpursley, tengqm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 27, 2020
@kbhawkey
Copy link
Contributor

👀

@@ -250,7 +250,9 @@ func WriteCommandFile(manifest *Manifest, t *template.Template, params TopLevelC
"|", "&#124;",
"<", "&lt;",
">", "&gt;",
)
"[", "<span>[</span>",
"]", "<span>]</span>",
Copy link
Contributor

@kbhawkey kbhawkey Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously:

[<a href="http://golang.org/pkg/text/template/#pkg-overview]">http://golang.org/pkg/text/template/#pkg-overview]</a>. </td>

Now (from generated sample (https://deploy-preview-19864--kubernetes-io-master-staging.netlify.com/docs/reference/generated/kubectl/kubectl-commands#-em-poddisruptionbudget-em-):

<td>Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates <span>[</span><a href="http://golang.org/pkg/text/template/#pkg-overview">http://golang.org/pkg/text/template/#pkg-overview</a><span>]</span>. </td>
</tr>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just wraps the square brackets in spans, so they don't get merged into the hyperlink url. Probably only needed to do that for the closing square bracket but wanted to be consistent.

Do you think that will cause a problem for it to render that way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way this should remove the need to do the manual fix that used to have the be performed every time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. What happened to the middle ] bracket in the original ref?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The middle ], the one that got attached to #pkg-overview] is gone... it wasn't supposed to be part of the URL.

The problem was that having ] directly after the URL, with no spaces, was messing up the markdown processor, and it was considering it to be part of the URL.

The problem arises because kubectl's cobra command is not trying to generate markdown, but the reference-docs generator is taking the output from kubectl and running it through a markdown processor and some things like [ ] are being interpreted as markdown's URL syntax when really it should be treated more like parentheses.

There isn't a great way to escape ] in markdown, so a solution (the one I chose in this PR) is to wrap it with so that markdown won't try to interpret it as a symbol to be processed.

For reference, if you run kubectl apply --help here is the part where this shows up:

--template='': Template string or path to template file to use when -o=go-template, -o=go-template-file. The
template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].

But again, this is in the command-line output, which is not markdown, so it displays fine in the command line because there is no markdown processor to mess it up.

I hope that I have understood and answered your question. Let me know if I haven't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @brianpursley . I wanted to know what happened to the extra ] and assumed that the parser no longer generated this character because of the new formatting. Your solution works around the problem. Ideally, the kubectl text formatting corrections could be made in the upstream code. There are a number of fixes in this code for formatting issues.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2020
@k8s-ci-robot k8s-ci-robot merged commit 83d98bb into kubernetes-sigs:master Mar 28, 2020
@brianpursley brianpursley deleted the kubectl-fixes branch March 28, 2020 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with kubectl-commands.html
5 participants