From 84ad1792ea9943158593d23fcdcd06f43ac119f4 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 10 Jul 2015 14:35:47 -0700 Subject: [PATCH 1/4] Run all munges on all docs Rather than terminating, Collect the errors and print them per-file-per-munge. --- cmd/mungedocs/links.go | 4 +- cmd/mungedocs/mungedocs.go | 103 ++++++++++++++++++++++--------------- 2 files changed, 64 insertions(+), 43 deletions(-) diff --git a/cmd/mungedocs/links.go b/cmd/mungedocs/links.go index 1ecdebf6418..e5934364882 100644 --- a/cmd/mungedocs/links.go +++ b/cmd/mungedocs/links.go @@ -58,7 +58,7 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { if err != nil { errors = append( errors, - fmt.Sprintf("%v, link %q is unparsable: %v", filePath, linkText, err), + fmt.Sprintf("link %q is unparsable: %v", linkText, err), ) return in } @@ -74,7 +74,7 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { if !targetExists { errors = append( errors, - fmt.Sprintf("%v, %q: target not found\n", filePath, linkText), + fmt.Sprintf("%q: target not found", linkText), ) } u.Path = newPath diff --git a/cmd/mungedocs/mungedocs.go b/cmd/mungedocs/mungedocs.go index 60f05849c7e..98910d44d0f 100644 --- a/cmd/mungedocs/mungedocs.go +++ b/cmd/mungedocs/mungedocs.go @@ -34,30 +34,32 @@ var ( ErrChangesNeeded = errors.New("mungedocs: changes required") + // All of the munge operations to perform. // TODO: allow selection from command line. (e.g., just check links in the examples directory.) - mungesToMake = munges{ - munger(updateTOC), - munger(checkLinks), + allMunges = []munge{ + {"table-of-contents", updateTOC}, + {"check-links", checkLinks}, } ) -// Munger processes a document, returning an updated document xor an error. -// Munger is NOT allowed to mutate 'before', if changes are needed it must copy -// data into a new byte array. -type munger func(filePath string, before []byte) (after []byte, err error) - -type munges []munger +// a munge processes a document, returning an updated document xor an error. +// The fn is NOT allowed to mutate 'before', if changes are needed it must copy +// data into a new byte array and return that. +type munge struct { + name string + fn func(filePath string, before []byte) (after []byte, err error) +} type fileProcessor struct { // Which munge functions should we call? - munges munges + munges []munge // Are we allowed to make changes? verifyOnly bool } // Either change a file or verify that it needs no changes (according to modify argument) -func (f fileProcessor) visit(path string, i os.FileInfo, e error) error { +func (f fileProcessor) visit(path string) error { if !strings.HasSuffix(path, ".md") { return nil } @@ -68,31 +70,55 @@ func (f fileProcessor) visit(path string, i os.FileInfo, e error) error { } modificationsMade := false + errFound := false + filePrinted := false for _, munge := range f.munges { - after, err := munge(path, fileBytes) - if err != nil { - return err - } - if !modificationsMade { - if !bytes.Equal(after, fileBytes) { - modificationsMade = true - if f.verifyOnly { - // We're not allowed to make changes. - return ErrChangesNeeded - } + after, err := munge.fn(path, fileBytes) + if err != nil || !bytes.Equal(after, fileBytes) { + if !filePrinted { + fmt.Printf("%s\n----\n", path) + filePrinted = true } + fmt.Printf("%s:\n", munge.name) + if err != nil { + fmt.Println(err) + errFound = true + } else { + fmt.Println("contents were modified") + modificationsMade = true + } + fmt.Println("") } fileBytes = after } // Write out new file with any changes. if modificationsMade { + if f.verifyOnly { + // We're not allowed to make changes. + return ErrChangesNeeded + } ioutil.WriteFile(path, fileBytes, 0644) } + if errFound { + return ErrChangesNeeded + } return nil } +func newWalkFunc(fp *fileProcessor, changesNeeded *bool) filepath.WalkFunc { + return func(path string, info os.FileInfo, err error) error { + if err := fp.visit(path); err != nil { + *changesNeeded = true + if err != ErrChangesNeeded { + return err + } + } + return nil + } +} + func main() { flag.Parse() @@ -102,30 +128,25 @@ func main() { } fp := fileProcessor{ - munges: mungesToMake, + munges: allMunges, verifyOnly: *verify, } // For each markdown file under source docs root, process the doc. - // If any error occurs, will exit with failure. - // If verify is true, then status is 0 for no changes needed, 1 for changes needed - // and >1 for an error during processing. - // If verify is false, then status is 0 if changes successfully made or no changes needed, - // 1 if changes were needed but require human intervention, and >1 for an unexpected - // error during processing. - err := filepath.Walk(*rootDir, fp.visit) + // - If any error occurs: exit with failure (exit >1). + // - If verify is true: exit 0 if no changes needed, exit 1 if changes + // needed. + // - If verify is false: exit 0 if changes successfully made or no + // changes needed. + var changesNeeded bool + + err := filepath.Walk(*rootDir, newWalkFunc(&fp, &changesNeeded)) if err != nil { - if err == ErrChangesNeeded { - if *verify { - fmt.Fprintf(os.Stderr, - "Some changes needed but not made due to --verify=true\n") - } else { - fmt.Fprintf(os.Stderr, - "Some changes needed but human intervention is required\n") - } - os.Exit(1) - } - fmt.Fprintf(os.Stderr, "filepath.Walk() returned %v\n", err) + fmt.Fprintf(os.Stderr, "ERROR: %v\n", err) os.Exit(2) } + if changesNeeded && *verify { + fmt.Fprintf(os.Stderr, "FAIL: changes needed but not made due to --verify\n") + os.Exit(1) + } } From 2e781fed498ad60d2d91570e0c7c5a296a2820f5 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 10 Jul 2015 15:42:54 -0700 Subject: [PATCH 2/4] Break util func into a new file --- cmd/mungedocs/toc.go | 44 ------------------------- cmd/mungedocs/toc_test.go | 39 ---------------------- cmd/mungedocs/util.go | 67 ++++++++++++++++++++++++++++++++++++++ cmd/mungedocs/util_test.go | 62 +++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 83 deletions(-) create mode 100644 cmd/mungedocs/util.go create mode 100644 cmd/mungedocs/util_test.go diff --git a/cmd/mungedocs/toc.go b/cmd/mungedocs/toc.go index 0636c8ba76a..86f9677f634 100644 --- a/cmd/mungedocs/toc.go +++ b/cmd/mungedocs/toc.go @@ -42,50 +42,6 @@ func updateTOC(filePath string, markdown []byte) ([]byte, error) { return updatedMarkdown, nil } -// Replaces the text between matching "beginMark" and "endMark" within "document" with "insertThis". -// -// Delimiters should occupy own line. -// Returns copy of document with modifications. -func updateMacroBlock(document []byte, beginMark, endMark, insertThis string) ([]byte, error) { - var buffer bytes.Buffer - lines := strings.Split(string(document), "\n") - // Skip trailing empty string from Split-ing - if len(lines) > 0 && lines[len(lines)-1] == "" { - lines = lines[:len(lines)-1] - } - betweenBeginAndEnd := false - for _, line := range lines { - trimmedLine := strings.Trim(line, " \n") - if trimmedLine == beginMark { - if betweenBeginAndEnd { - return nil, fmt.Errorf("found second begin mark while updating macro blocks") - } - betweenBeginAndEnd = true - buffer.WriteString(line) - buffer.WriteString("\n") - } else if trimmedLine == endMark { - if !betweenBeginAndEnd { - return nil, fmt.Errorf("found end mark without being mark while updating macro blocks") - } - buffer.WriteString(insertThis) - // Extra newline avoids github markdown bug where comment ends up on same line as last bullet. - buffer.WriteString("\n") - buffer.WriteString(line) - buffer.WriteString("\n") - betweenBeginAndEnd = false - } else { - if !betweenBeginAndEnd { - buffer.WriteString(line) - buffer.WriteString("\n") - } - } - } - if betweenBeginAndEnd { - return nil, fmt.Errorf("never found closing end mark while updating macro blocks") - } - return buffer.Bytes(), nil -} - // builds table of contents for markdown file // // First scans for all section headers (lines that begin with "#" but not within code quotes) diff --git a/cmd/mungedocs/toc_test.go b/cmd/mungedocs/toc_test.go index f52df24df75..5175d8089f1 100644 --- a/cmd/mungedocs/toc_test.go +++ b/cmd/mungedocs/toc_test.go @@ -22,45 +22,6 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_updateMacroBlock(t *testing.T) { - var cases = []struct { - in string - out string - }{ - {"", ""}, - {"Lorem ipsum\ndolor sit amet\n", - "Lorem ipsum\ndolor sit amet\n"}, - {"Lorem ipsum \n BEGIN\ndolor\nEND\nsit amet\n", - "Lorem ipsum \n BEGIN\nfoo\n\nEND\nsit amet\n"}, - } - for _, c := range cases { - actual, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo\n") - assert.NoError(t, err) - if c.out != string(actual) { - t.Errorf("Expected '%v' but got '%v'", c.out, string(actual)) - } - } -} - -func Test_updateMacroBlock_errors(t *testing.T) { - var cases = []struct { - in string - }{ - {"BEGIN\n"}, - {"blah\nBEGIN\nblah"}, - {"END\n"}, - {"blah\nEND\nblah\n"}, - {"END\nBEGIN"}, - {"BEGIN\nEND\nEND"}, - {"BEGIN\nBEGIN\nEND"}, - {"BEGIN\nBEGIN\nEND\nEND"}, - } - for _, c := range cases { - _, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo") - assert.Error(t, err) - } -} - func Test_buildTOC(t *testing.T) { var cases = []struct { in string diff --git a/cmd/mungedocs/util.go b/cmd/mungedocs/util.go new file mode 100644 index 00000000000..6a55f0aa9be --- /dev/null +++ b/cmd/mungedocs/util.go @@ -0,0 +1,67 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "bytes" + "fmt" + "strings" +) + +// Replaces the text between matching "beginMark" and "endMark" within "document" with "insertThis". +// +// Delimiters should occupy own line. +// Returns copy of document with modifications. +func updateMacroBlock(document []byte, beginMark, endMark, insertThis string) ([]byte, error) { + var buffer bytes.Buffer + lines := strings.Split(string(document), "\n") + // Skip trailing empty string from Split-ing + if len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + betweenBeginAndEnd := false + for _, line := range lines { + trimmedLine := strings.Trim(line, " \n") + if trimmedLine == beginMark { + if betweenBeginAndEnd { + return nil, fmt.Errorf("found second begin mark while updating macro blocks") + } + betweenBeginAndEnd = true + buffer.WriteString(line) + buffer.WriteString("\n") + } else if trimmedLine == endMark { + if !betweenBeginAndEnd { + return nil, fmt.Errorf("found end mark without being mark while updating macro blocks") + } + buffer.WriteString(insertThis) + // Extra newline avoids github markdown bug where comment ends up on same line as last bullet. + buffer.WriteString("\n") + buffer.WriteString(line) + buffer.WriteString("\n") + betweenBeginAndEnd = false + } else { + if !betweenBeginAndEnd { + buffer.WriteString(line) + buffer.WriteString("\n") + } + } + } + if betweenBeginAndEnd { + return nil, fmt.Errorf("never found closing end mark while updating macro blocks") + } + return buffer.Bytes(), nil +} diff --git a/cmd/mungedocs/util_test.go b/cmd/mungedocs/util_test.go new file mode 100644 index 00000000000..68454c1c779 --- /dev/null +++ b/cmd/mungedocs/util_test.go @@ -0,0 +1,62 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_updateMacroBlock(t *testing.T) { + var cases = []struct { + in string + out string + }{ + {"", ""}, + {"Lorem ipsum\ndolor sit amet\n", + "Lorem ipsum\ndolor sit amet\n"}, + {"Lorem ipsum \n BEGIN\ndolor\nEND\nsit amet\n", + "Lorem ipsum \n BEGIN\nfoo\n\nEND\nsit amet\n"}, + } + for _, c := range cases { + actual, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo\n") + assert.NoError(t, err) + if c.out != string(actual) { + t.Errorf("Expected '%v' but got '%v'", c.out, string(actual)) + } + } +} + +func Test_updateMacroBlock_errors(t *testing.T) { + var cases = []struct { + in string + }{ + {"BEGIN\n"}, + {"blah\nBEGIN\nblah"}, + {"END\n"}, + {"blah\nEND\nblah\n"}, + {"END\nBEGIN"}, + {"BEGIN\nEND\nEND"}, + {"BEGIN\nBEGIN\nEND"}, + {"BEGIN\nBEGIN\nEND\nEND"}, + } + for _, c := range cases { + _, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo") + assert.Error(t, err) + } +} From 95cd66d3a0e217ec79fecceb22ef098a4116f53d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 10 Jul 2015 15:54:12 -0700 Subject: [PATCH 3/4] use 'lines []string' for updateMacroBlock --- cmd/mungedocs/toc.go | 3 ++- cmd/mungedocs/util.go | 18 ++++++++++++------ cmd/mungedocs/util_test.go | 4 ++-- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cmd/mungedocs/toc.go b/cmd/mungedocs/toc.go index 86f9677f634..9011e117db9 100644 --- a/cmd/mungedocs/toc.go +++ b/cmd/mungedocs/toc.go @@ -35,7 +35,8 @@ func updateTOC(filePath string, markdown []byte) ([]byte, error) { if err != nil { return nil, err } - updatedMarkdown, err := updateMacroBlock(markdown, "", "", string(toc)) + lines := splitLines(markdown) + updatedMarkdown, err := updateMacroBlock(lines, "", "", string(toc)) if err != nil { return nil, err } diff --git a/cmd/mungedocs/util.go b/cmd/mungedocs/util.go index 6a55f0aa9be..f26df9b714c 100644 --- a/cmd/mungedocs/util.go +++ b/cmd/mungedocs/util.go @@ -22,17 +22,23 @@ import ( "strings" ) -// Replaces the text between matching "beginMark" and "endMark" within "document" with "insertThis". -// -// Delimiters should occupy own line. -// Returns copy of document with modifications. -func updateMacroBlock(document []byte, beginMark, endMark, insertThis string) ([]byte, error) { - var buffer bytes.Buffer +// Splits a document up into a slice of lines. +func splitLines(document []byte) []string { lines := strings.Split(string(document), "\n") // Skip trailing empty string from Split-ing if len(lines) > 0 && lines[len(lines)-1] == "" { lines = lines[:len(lines)-1] } + return lines +} + +// Replaces the text between matching "beginMark" and "endMark" within the +// document represented by "lines" with "insertThis". +// +// Delimiters should occupy own line. +// Returns copy of document with modifications. +func updateMacroBlock(lines []string, beginMark, endMark, insertThis string) ([]byte, error) { + var buffer bytes.Buffer betweenBeginAndEnd := false for _, line := range lines { trimmedLine := strings.Trim(line, " \n") diff --git a/cmd/mungedocs/util_test.go b/cmd/mungedocs/util_test.go index 68454c1c779..49fbcee6381 100644 --- a/cmd/mungedocs/util_test.go +++ b/cmd/mungedocs/util_test.go @@ -34,7 +34,7 @@ func Test_updateMacroBlock(t *testing.T) { "Lorem ipsum \n BEGIN\nfoo\n\nEND\nsit amet\n"}, } for _, c := range cases { - actual, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo\n") + actual, err := updateMacroBlock(splitLines([]byte(c.in)), "BEGIN", "END", "foo\n") assert.NoError(t, err) if c.out != string(actual) { t.Errorf("Expected '%v' but got '%v'", c.out, string(actual)) @@ -56,7 +56,7 @@ func Test_updateMacroBlock_errors(t *testing.T) { {"BEGIN\nBEGIN\nEND\nEND"}, } for _, c := range cases { - _, err := updateMacroBlock([]byte(c.in), "BEGIN", "END", "foo") + _, err := updateMacroBlock(splitLines([]byte(c.in)), "BEGIN", "END", "foo") assert.Error(t, err) } } From 8ff8559c4c0b56e67b0bec32f60baabc434dccfd Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 10 Jul 2015 16:17:51 -0700 Subject: [PATCH 4/4] Add util functions --- cmd/mungedocs/util.go | 27 +++++++++++++++++++++ cmd/mungedocs/util_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/cmd/mungedocs/util.go b/cmd/mungedocs/util.go index f26df9b714c..8581f5152b9 100644 --- a/cmd/mungedocs/util.go +++ b/cmd/mungedocs/util.go @@ -71,3 +71,30 @@ func updateMacroBlock(lines []string, beginMark, endMark, insertThis string) ([] } return buffer.Bytes(), nil } + +// Tests that a document, represented as a slice of lines, has a line. Ignores +// leading and trailing space. +func hasLine(lines []string, needle string) bool { + for _, line := range lines { + trimmedLine := strings.Trim(line, " \n") + if trimmedLine == needle { + return true + } + } + return false +} + +// Tests that a document, represented as a slice of lines, has a macro block. +func hasMacroBlock(lines []string, begin string, end string) bool { + foundBegin := false + for _, line := range lines { + trimmedLine := strings.Trim(line, " \n") + switch { + case !foundBegin && trimmedLine == begin: + foundBegin = true + case foundBegin && trimmedLine == end: + return true + } + } + return false +} diff --git a/cmd/mungedocs/util_test.go b/cmd/mungedocs/util_test.go index 49fbcee6381..30d85d5a2b4 100644 --- a/cmd/mungedocs/util_test.go +++ b/cmd/mungedocs/util_test.go @@ -60,3 +60,51 @@ func Test_updateMacroBlock_errors(t *testing.T) { assert.Error(t, err) } } + +func TestHasLine(t *testing.T) { + cases := []struct { + lines []string + needle string + expected bool + }{ + {[]string{"abc", "def", "ghi"}, "abc", true}, + {[]string{" abc", "def", "ghi"}, "abc", true}, + {[]string{"abc ", "def", "ghi"}, "abc", true}, + {[]string{"\n abc", "def", "ghi"}, "abc", true}, + {[]string{"abc \n", "def", "ghi"}, "abc", true}, + {[]string{"abc", "def", "ghi"}, "def", true}, + {[]string{"abc", "def", "ghi"}, "ghi", true}, + {[]string{"abc", "def", "ghi"}, "xyz", false}, + } + + for i, c := range cases { + if hasLine(c.lines, c.needle) != c.expected { + t.Errorf("case[%d]: %q, expected %t, got %t", i, c.needle, c.expected, !c.expected) + } + } +} + +func TestHasMacroBlock(t *testing.T) { + cases := []struct { + lines []string + begin string + end string + expected bool + }{ + {[]string{"<<<", ">>>"}, "<<<", ">>>", true}, + {[]string{"<<<", "abc", ">>>"}, "<<<", ">>>", true}, + {[]string{"<<<", "<<<", "abc", ">>>"}, "<<<", ">>>", true}, + {[]string{"<<<", "abc", ">>>", ">>>"}, "<<<", ">>>", true}, + {[]string{"<<<", ">>>", "<<<", ">>>"}, "<<<", ">>>", true}, + {[]string{"<<<"}, "<<<", ">>>", false}, + {[]string{">>>"}, "<<<", ">>>", false}, + {[]string{"<<<", "abc"}, "<<<", ">>>", false}, + {[]string{"abc", ">>>"}, "<<<", ">>>", false}, + } + + for i, c := range cases { + if hasMacroBlock(c.lines, c.begin, c.end) != c.expected { + t.Errorf("case[%d]: %q,%q, expected %t, got %t", i, c.begin, c.end, c.expected, !c.expected) + } + } +}