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
upgrade Blackfriday to v2 and re-implement render #112731
Conversation
Skipping CI for Draft Pull Request. |
@pacoxu: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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/test-infra repository. |
6f380bf
to
baedc1c
Compare
where is this kubectl output used? is there any way to verify the updated output with the new implementation (alternatively, if it's not used, can we drop this impl altogether) |
Currently, all kubectl commands will use it like below.
https://github.com/kubernetes/kubernetes/search?q=templates.LongDesc kubeadm has some similar test cases in kubernetes/cmd/kubeadm/app/cmd/util/documentation_test.go Lines 23 to 84 in ea07644
We may follow that to add some unit tests if needed. Let me add some test cases. |
/hold cancel
|
1996c2d
to
507a41f
Compare
staging/src/k8s.io/kubectl/pkg/util/templates/normalizers_test.go
Outdated
Show resolved
Hide resolved
507a41f
to
b4a3c85
Compare
@@ -70,7 +70,7 @@ type normalizer struct { | |||
|
|||
func (s normalizer) markdown() normalizer { | |||
bytes := []byte(s.string) | |||
formatted := blackfriday.Markdown(bytes, &ASCIIRenderer{Indentation: Indentation}, blackfriday.EXTENSION_NO_INTRA_EMPHASIS) |
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.
was whatever prompted use of EXTENSION_NO_INTRA_EMPHASIS
tested, since it doesn't look like we're using it in v2
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 checked an example ./kubectl apply edit-last-applied --help
and there is no difference.
kubernetes/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_edit_last_applied.go
Lines 31 to 50 in c18c244
applyEditLastAppliedLong = templates.LongDesc(i18n.T(` | |
Edit the latest last-applied-configuration annotations of resources from the default editor. | |
The edit-last-applied command allows you to directly edit any API resource you can retrieve via the | |
command-line tools. It will open the editor defined by your KUBE_EDITOR, or EDITOR | |
environment variables, or fall back to 'vi' for Linux or 'notepad' for Windows. | |
You can edit multiple objects, although changes are applied one at a time. The command | |
accepts file names as well as command-line arguments, although the files you point to must | |
be previously saved versions of resources. | |
The default format is YAML. To edit in JSON, specify "-o json". | |
The flag --windows-line-endings can be used to force Windows line endings, | |
otherwise the default for your operating system will be used. | |
In the event an error occurs while updating, a temporary file will be created on disk | |
that contains your unapplied changes. The most common error when updating a resource | |
is another editor changing the resource on the server. When this occurs, you will have | |
to apply your changes to the newer version of the resource, or update your temporary | |
saved copy to include the latest resource version.`)) |
Some old issues before that may be related: spf13/cobra#518.
To keep it consistent, we can use blackfriday.WithExtensions(blackfriday.NoIntraEmphasis)
instead. Updated.
7253978
to
6e567dc
Compare
6e567dc
to
9d2be18
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, pacoxu 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 |
What type of PR is this?
/kind feature
/sig cli
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #112717
Special notes for your reviewer:
github.com/russross/blackfriday/v2
is already used in other components.github.com/russross/blackfriday
is removed with this PR.Does this PR introduce a user-facing change?