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.

Sign up for GitHub

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

Jump to bottom

resourcedocs: Add Anchors to inlined definitions #186

Merged
merged 4 commits into from
Jan 5, 2021
Merged

resourcedocs: Add Anchors to inlined definitions #186

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42c01cb
resourcedocs: Add Anchors to inlined definitions
feloy Dec 11, 2020
6898e87
Explicitly set anchor names
feloy Dec 11, 2020
a7ec445
Add comments for fragments
feloy Dec 11, 2020
2c047b6
Fix tests
feloy Dec 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gen-resourcesdocs/pkg/config/output.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (o *TOC) OutputProperties(defname string, definition spec.Schema, outputSec
return err
}

err = outputSection.AddTypeDefinition(target.Description)
err = outputSection.AddTypeDefinition(property.TypeKey.ResourceName(), target.Description)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion gen-resourcesdocs/pkg/outputs/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Chapter interface {
// Section is an interface to a section of an output
type Section interface {
AddContent(s string) error
AddTypeDefinition(s string) error
AddTypeDefinition(typ string, description string) error
Copy link
Contributor

@kbhawkey kbhawkey Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

nit: typ is the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as type is a reserved keyword in Go, we cannot use type as variable name

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, typ is odd. t or something else might make it more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is quite common in Go, see for example https://golang.org/pkg/go/types/

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, interesting.

StartPropertyList() error
AddFieldCategory(name string) error
AddProperty(name string, property *kubernetes.Property, linkend []string, indent bool, defname string, shortName string) error
Expand Down
1 change: 1 addition & 0 deletions gen-resourcesdocs/pkg/outputs/kwebsite/chapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type FieldData struct {
Name string
Value string
Description string
Type string
TypeDefinition string
Indent int
}
Expand Down
3 changes: 1 addition & 2 deletions gen-resourcesdocs/pkg/outputs/kwebsite/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func escapeName(parts ...string) string {

// headingID returns the ID built by hugo for a given header
func headingID(s string) string {
result := strings.ToLower(s)
result = strings.ReplaceAll(result, " ", "-")
result := strings.ReplaceAll(s, " ", "-")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to normalize the heading?
I was under the impression that Hugo would do this.

Copy link
Contributor Author

@feloy feloy Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, Hugo does it by default, but puts the fragment in lowercase. I would prefer to keep the original case, as these are Resource names, and I would like to use the fragment name as text to display in the api-reference shortcode: (see https://github.com/kubernetes/website/pull/23294/files#diff-4992971bcbda91c2f2374a2e62fffda3d7852174780e027070e766c62bf87ac1 )

Copy link
Contributor

Choose a reason for hiding this comment

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

I need to look at the generated pages.
Could you update the shortcode to adapt the formatting of the resource name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The shortcode has been changed in this commit: kubernetes/website@d9b18d3

The result should be visible in the paragraph https://deploy-preview-23294--kubernetes-io-master-staging.netlify.app/docs/concepts/configuration/secret/#using-imagepullsecrets (see the PodSpec API ...)

The build of the preview website fails, I can't see why

Copy link
Contributor

Choose a reason for hiding this comment

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

looking now. See this message:
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I made a rebase to restart the tests in the meantime. Here is the commit:
kubernetes/website@99e1b41

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also get the message, but you can see the change in the api-reference.html file of this commit

return result
}
5 changes: 3 additions & 2 deletions gen-resourcesdocs/pkg/outputs/kwebsite/section.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (o Section) AddContent(s string) error {
}

// AddTypeDefinition adds the definition of a type to a section
func (o Section) AddTypeDefinition(s string) error {
func (o Section) AddTypeDefinition(typ string, description string) error {
i := len(o.chapter.data.Sections)
cats := o.chapter.data.Sections[i-1].FieldCategories
var fields *[]FieldData
Expand All @@ -34,7 +34,8 @@ func (o Section) AddTypeDefinition(s string) error {
fields = &cats[len(cats)-1].Fields
}
j := len(*fields)
(*fields)[j-1].TypeDefinition = "*" + s + "*"
(*fields)[j-1].Type = typ
(*fields)[j-1].TypeDefinition = "*" + description + "*"
return nil
}

Expand Down
4 changes: 3 additions & 1 deletion gen-resourcesdocs/templates/chapter-single-definition.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,21 @@ weight: {{.Metadata.Weight}}
{{.Description | replace "<" "\\<" | indent 2 | indent .Indent | indent .Indent}}
{{- end}}
{{if .TypeDefinition}}
{{ "" | indent .Indent | indent .Indent}} <a name="{{.Type}}"></a>
{{.TypeDefinition | indent 2 | indent .Indent | indent .Indent}}
{{end}}
{{- end}}{{/* range .Fields */}}

{{range .FieldCategories}}
### {{.Name}}
### {{.Name}} {#{{.Name}}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment about why the fragment identifier is set?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need the set the fragment identifier if the same as the heading?
Hugo should automatically generate the anchors for the headings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my comment below,I would like to keep the original case of the heading (which is a Resource Name)


{{range .Fields}}
{{ "" | indent .Indent | indent .Indent}}- {{.Name}}{{if .Value}}: {{.Value}}{{end}}
{{if .Description}}
{{.Description | replace "<" "\\<" | indent 2 | indent .Indent | indent .Indent}}
{{- end}}
{{if .TypeDefinition}}
{{ "" | indent .Indent | indent .Indent}} <a name="{{.Type}}"></a>
{{.TypeDefinition | indent 2 | indent .Indent | indent .Indent}}
{{end}}
{{- end}}{{/* range .Fields */}}
Expand Down
4 changes: 3 additions & 1 deletion gen-resourcesdocs/templates/chapter.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ weight: {{.Metadata.Weight}}
{{if .Import}}`import "{{.Import}}"`{{end}}

{{range .Sections}}
## {{.Name}}
## {{.Name}} {#{{.Name}}}

{{.Description | replace "<" "\\<" }}

Expand All @@ -24,6 +24,7 @@ weight: {{.Metadata.Weight}}
{{.Description | replace "<" "\\<" | indent 2 | indent .Indent | indent .Indent}}
{{- end}}
{{if .TypeDefinition}}
{{ "" | indent .Indent | indent .Indent}} <a name="{{.Type}}"></a>
{{.TypeDefinition | indent 2 | indent .Indent | indent .Indent}}
{{end}}
{{- end}}{{/* range .Fields */}}
Expand All @@ -37,6 +38,7 @@ weight: {{.Metadata.Weight}}
{{.Description | replace "<" "\\<" | indent 2 | indent .Indent | indent .Indent}}
{{- end}}
{{if .TypeDefinition}}
{{ "" | indent .Indent | indent .Indent}} <a name="{{.Type}}"></a>
{{.TypeDefinition | indent 2 | indent .Indent | indent .Indent}}
{{end}}
{{- end}}{{/* range .Fields */}}
Expand Down