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

⚠️ Trim leading/trailing whitespace on lines in documentation #517

Merged
merged 5 commits into from Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/crd/markers/zz_generated.markerhelp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 30 additions & 2 deletions pkg/markers/collect_test.go
Expand Up @@ -31,7 +31,10 @@ type fieldPath struct {
var _ = Describe("Collecting", func() {
var col *Collector
var markersByType map[string]MarkerValues
var docsByType map[string]string

var markersByField map[fieldPath]MarkerValues
var docsByField map[fieldPath]string

JustBeforeEach(func() {
By("setting up the registry and collector")
Expand All @@ -45,15 +48,23 @@ var _ = Describe("Collecting", func() {

col = &Collector{Registry: reg}

By("gathering markers by type name")
By("gathering markers/docs by type/field name")
markersByType = make(map[string]MarkerValues)
docsByType = make(map[string]string)
markersByField = make(map[fieldPath]MarkerValues)
docsByField = make(map[fieldPath]string)

err := EachType(col, fakePkg, func(info *TypeInfo) {
markersByType[info.Name] = info.Markers
docsByType[info.Name] = info.Doc

for _, field := range info.Fields {
markersByField[fieldPath{typ: info.Name, field: field.Name}] = field.Markers
fieldPath := fieldPath{typ: info.Name, field: field.Name}
markersByField[fieldPath] = field.Markers
docsByField[fieldPath] = field.Doc
}
})

Expect(err).NotTo(HaveOccurred())
Expect(fakePkg.Errors).To(HaveLen(0))
})
Expand Down Expand Up @@ -94,6 +105,10 @@ var _ = Describe("Collecting", func() {
HaveKeyWithValue("testing:typelvl", ContainElement("here on type"))))
})

It("should have docs without markers", func() {
Expect(docsByType).To(HaveKeyWithValue("Foo", "normal godoc normal godoc"))
})

It("should associate markers in the closest non-godoc block", func() {
Expect(markersByType).To(HaveKeyWithValue("Foo",
HaveKeyWithValue("testing:typelvl", ContainElement("here before type"))))
Expand All @@ -109,6 +124,19 @@ var _ = Describe("Collecting", func() {
Expect(pkgMarkers).To(HaveKeyWithValue("testing:pkglvl", ContainElement("here reassociated")))
})
})

Context("types with /*…*/-style comments", func() {
It("should have doc without extraneous spaces", func() {
Expect(docsByType).To(HaveKeyWithValue("HasDocsWithSpaces",
"This type of doc has spaces preserved in go-ast, but we'd like to trim them."))
})

It("should have doc without extraneous spaces, even over multiple lines", func() {
Expect(docsByType).To(HaveKeyWithValue("HasDocsWithSpaces2",
"This type of doc has spaces preserved in go-ast, but we'd like to trim them, especially when formatted like this."))
})
})

Context("without godoc", func() {
It("should associate markers in the closest block", func() {
Expect(markersByType).To(HaveKeyWithValue("Quux",
Expand Down
11 changes: 11 additions & 0 deletions pkg/markers/markers_suite_test.go
Expand Up @@ -88,6 +88,17 @@ var _ = BeforeSuite(func() {
Bar = "foo"
)

/* This type of doc has spaces preserved in go-ast, but we'd like to trim them. */
type HasDocsWithSpaces struct {
}

/*
This type of doc has spaces preserved in go-ast, but we'd like to trim them,
especially when formatted like this.
*/
type HasDocsWithSpaces2 struct {
}

type Baz interface {
// +testing:pkglvl="not here in interface"
}
Expand Down
12 changes: 11 additions & 1 deletion pkg/markers/zip.go
Expand Up @@ -67,12 +67,22 @@ func extractDoc(node ast.Node, decl *ast.GenDecl) string {
// chop off the extraneous last part
outLines = outLines[:len(outLines)-1]
}
// respect double-newline meaning actual newline

for i, line := range outLines {
// Trim any extranous whitespace,
// for handling /*…*/-style comments,
// which have whitespace preserved in go/ast:
line = strings.Trim(line, " \t")
Porges marked this conversation as resolved.
Show resolved Hide resolved

// Respect that double-newline means
// actual newline:
if line == "" {
outLines[i] = "\n"
} else {
outLines[i] = line
}
}

return strings.Join(outLines, " ")
}

Expand Down