From 8886a9940d96a6f937abc8891163c500676fbef2 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 20 Jul 2015 11:45:58 -0500 Subject: [PATCH] Rewrite how the munger works The basic idea is that in the main mungedocs we run the entirefile and create an annotated set of lines about that file. All mungers then act on a struct mungeLines instead of on a bytes array. Making use of the metadata where appropriete. Helper functions exist to make updating a 'macro block' extremely easy. --- cmd/mungedocs/README.md | 22 ++ cmd/mungedocs/analytics.go | 67 +++--- cmd/mungedocs/analytics_test.go | 64 +++--- cmd/mungedocs/example_syncer.go | 129 ++++++----- cmd/mungedocs/example_syncer_test.go | 21 +- cmd/mungedocs/headers.go | 75 +++--- cmd/mungedocs/headers_test.go | 12 +- cmd/mungedocs/kubectl_dash_f.go | 38 ++-- cmd/mungedocs/kubectl_dash_f_test.go | 6 +- cmd/mungedocs/links.go | 83 ++++--- cmd/mungedocs/links_test.go | 76 +++++++ cmd/mungedocs/mungedocs.go | 34 +-- cmd/mungedocs/preformatted.go | 48 ++-- cmd/mungedocs/preformatted_test.go | 57 +++++ cmd/mungedocs/toc.go | 55 ++--- cmd/mungedocs/toc_test.go | 37 +-- cmd/mungedocs/unversioned_warning.go | 36 ++- cmd/mungedocs/unversioned_warning_test.go | 17 +- cmd/mungedocs/util.go | 263 +++++++++++++++------- cmd/mungedocs/util_test.go | 126 +++++------ docs/design/security_context.md | 2 +- docs/user-guide/persistent-volumes.md | 2 +- 22 files changed, 758 insertions(+), 512 deletions(-) create mode 100644 cmd/mungedocs/README.md create mode 100644 cmd/mungedocs/links_test.go create mode 100644 cmd/mungedocs/preformatted_test.go diff --git a/cmd/mungedocs/README.md b/cmd/mungedocs/README.md new file mode 100644 index 00000000000..7adef4c4cce --- /dev/null +++ b/cmd/mungedocs/README.md @@ -0,0 +1,22 @@ +# Documentation Mungers + +Basically this is like lint/gofmt for md docs. + +It basically does the following: +- iterate over all files in the given doc root. +- for each file split it into a slice (mungeLines) of lines (mungeLine) +- a mungeline has metadata about each line typically determined by a 'fast' regex. + - metadata contains things like 'is inside a preformmatted block' + - contains a markdown header + - has a link to another file + - etc.. + - if you have a really slow regex with a lot of backtracking you might want to write a fast one to limit how often you run the slow one. +- each munger is then called in turn + - they are given the mungeLines + - they create an entirely new set of mungeLines with their modifications + - the new set is returned +- the new set is then fed into the next munger. +- in the end we might commit the end mungeLines to the file or not (--verify) + + +[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/cmd/mungedocs/README.md?pixel)]() diff --git a/cmd/mungedocs/analytics.go b/cmd/mungedocs/analytics.go index eb82b64e068..311f134c639 100644 --- a/cmd/mungedocs/analytics.go +++ b/cmd/mungedocs/analytics.go @@ -17,43 +17,42 @@ limitations under the License. package main import ( - "bytes" "fmt" - "os" - "regexp" + "strings" ) -var ( - beginMungeExp = regexp.QuoteMeta(beginMungeTag("GENERATED_ANALYTICS")) - endMungeExp = regexp.QuoteMeta(endMungeTag("GENERATED_ANALYTICS")) - analyticsExp = regexp.QuoteMeta("[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/") + - "[^?]*" + - regexp.QuoteMeta("?pixel)]()") +const analyticsMungeTag = "GENERATED_ANALYTICS" +const analyticsLinePrefix = "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/" - // Matches the analytics blurb, with or without the munge headers. - analyticsRE = regexp.MustCompile(`[\n]*` + analyticsExp + `[\n]?` + - `|` + `[\n]*` + beginMungeExp + `[^<]*` + endMungeExp) -) - -// This adds the analytics link to every .md file. -func checkAnalytics(fileName string, fileBytes []byte) (output []byte, err error) { - fileName = makeRepoRelative(fileName) - desired := fmt.Sprintf(` - - -`+beginMungeTag("GENERATED_ANALYTICS")+` -[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/%s?pixel)]() -`+endMungeTag("GENERATED_ANALYTICS")+` -`, fileName) - if !analyticsRE.MatchString(desired) { - fmt.Printf("%q does not match %q", analyticsRE.String(), desired) - os.Exit(1) +func updateAnalytics(fileName string, mlines mungeLines) (mungeLines, error) { + var out mungeLines + fileName, err := makeRepoRelative(fileName, fileName) + if err != nil { + return mlines, err } - //output = replaceNonPreformattedRegexp(fileBytes, analyticsRE, func(in []byte) []byte { - output = analyticsRE.ReplaceAllFunc(fileBytes, func(in []byte) []byte { - return []byte{} - }) - output = bytes.TrimRight(output, "\n") - output = append(output, []byte(desired)...) - return output, nil + + link := fmt.Sprintf(analyticsLinePrefix+"%s?pixel)]()", fileName) + insertLines := getMungeLines(link) + mlines, err = removeMacroBlock(analyticsMungeTag, mlines) + if err != nil { + return mlines, err + } + + // Remove floating analytics links not surrounded by the munge tags. + for _, mline := range mlines { + if mline.preformatted || mline.header || mline.beginTag || mline.endTag { + out = append(out, mline) + continue + } + if strings.HasPrefix(mline.data, analyticsLinePrefix) { + continue + } + out = append(out, mline) + } + out = appendMacroBlock(out, analyticsMungeTag) + out, err = updateMacroBlock(out, analyticsMungeTag, insertLines) + if err != nil { + return mlines, err + } + return out, nil } diff --git a/cmd/mungedocs/analytics_test.go b/cmd/mungedocs/analytics_test.go index aeccae58f56..37db7971705 100644 --- a/cmd/mungedocs/analytics_test.go +++ b/cmd/mungedocs/analytics_test.go @@ -23,67 +23,71 @@ import ( ) func TestAnalytics(t *testing.T) { + b := beginMungeTag("GENERATED_ANALYTICS") + e := endMungeTag("GENERATED_ANALYTICS") var cases = []struct { - in string - out string + in string + expected string }{ { "aoeu", - "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + "aoeu" + "\n" + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, { "aoeu" + "\n" + "\n" + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()", "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, { "aoeu" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n", - "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + e + "\n", + "aoeu" + "\n" + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, { "aoeu" + "\n" + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n", - "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + e + "\n", + "aoeu" + "\n" + "\n" + "\n" + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, { "prefix" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + + e + "\n" + "suffix", - "prefix" + "\n" + "suffix" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + "prefix" + "\n" + "suffix" + "\n" + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, { "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n", + e + "\n", "aoeu" + "\n" + "\n" + "\n" + - beginMungeTag("GENERATED_ANALYTICS") + "\n" + + b + "\n" + "[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/path/to/file-name.md?pixel)]()" + "\n" + - endMungeTag("GENERATED_ANALYTICS") + "\n"}, + e + "\n"}, } - for _, c := range cases { - out, err := checkAnalytics("path/to/file-name.md", []byte(c.in)) + for i, c := range cases { + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + out, err := updateAnalytics("path/to/file-name.md", in) assert.NoError(t, err) - if string(out) != c.out { - t.Errorf("Expected \n\n%v\n\n but got \n\n%v\n\n", c.out, string(out)) + if !expected.Equal(out) { + t.Errorf("Case %d Expected \n\n%v\n\n but got \n\n%v\n\n", i, expected.String(), out.String()) } } } diff --git a/cmd/mungedocs/example_syncer.go b/cmd/mungedocs/example_syncer.go index b00c385c35f..8df6a968e45 100644 --- a/cmd/mungedocs/example_syncer.go +++ b/cmd/mungedocs/example_syncer.go @@ -17,15 +17,17 @@ limitations under the License. package main import ( - "bytes" "fmt" "io/ioutil" - "path" "regexp" "strings" ) -const exampleMungeTag = "EXAMPLE" +const exampleToken = "EXAMPLE" + +const exampleLineStart = " -func syncExamples(filePath string, markdown []byte) ([]byte, error) { - // find the example syncer begin tag - header := beginMungeTag(fmt.Sprintf("%s %s", exampleMungeTag, `(([^ ])*.(yaml|json))`)) - exampleLinkRE := regexp.MustCompile(header) - lines := splitLines(markdown) - updatedMarkdown, err := updateExampleMacroBlock(filePath, lines, exampleLinkRE, endMungeTag(exampleMungeTag)) - if err != nil { - return updatedMarkdown, err +func syncExamples(filePath string, mlines mungeLines) (mungeLines, error) { + var err error + type exampleTag struct { + token string + linkText string + fileType string } - return updatedMarkdown, nil + exampleTags := []exampleTag{} + + // collect all example Tags + for _, mline := range mlines { + if mline.preformatted || !mline.beginTag { + continue + } + line := mline.data + if !strings.HasPrefix(line, exampleLineStart) { + continue + } + match := exampleMungeTagRE.FindStringSubmatch(line) + if len(match) < 4 { + err = fmt.Errorf("Found unparsable EXAMPLE munge line %v", line) + return mlines, err + } + tag := exampleTag{ + token: exampleToken + " " + match[1], + linkText: match[1], + fileType: match[3], + } + exampleTags = append(exampleTags, tag) + } + // update all example Tags + for _, tag := range exampleTags { + example, err := exampleContent(filePath, tag.linkText, tag.fileType) + if err != nil { + return mlines, err + } + mlines, err = updateMacroBlock(mlines, tag.token, example) + if err != nil { + return mlines, err + } + } + return mlines, nil } // exampleContent retrieves the content of the file at linkPath -func exampleContent(filePath, linkPath, fileType string) (content string, err error) { - realRoot := path.Join(*rootDir, *repoRoot) + "/" - path := path.Join(realRoot, path.Dir(filePath), linkPath) - dat, err := ioutil.ReadFile(path) +func exampleContent(filePath, linkPath, fileType string) (mungeLines, error) { + repoRel, err := makeRepoRelative(linkPath, filePath) if err != nil { - return content, err + return nil, err } + + fileRel, err := makeFileRelative(linkPath, filePath) + if err != nil { + return nil, err + } + + dat, err := ioutil.ReadFile(repoRel) + if err != nil { + return nil, err + } + // remove leading and trailing spaces and newlines trimmedFileContent := strings.TrimSpace(string(dat)) - content = fmt.Sprintf("\n```%s\n%s\n```\n\n[Download example](%s)", fileType, trimmedFileContent, linkPath) - return -} - -// updateExampleMacroBlock sync the yaml/json example between begin tag and end tag -func updateExampleMacroBlock(filePath string, lines []string, beginMarkExp *regexp.Regexp, endMark string) ([]byte, error) { - var buffer bytes.Buffer - betweenBeginAndEnd := false - for _, line := range lines { - trimmedLine := strings.Trim(line, " \n") - if beginMarkExp.Match([]byte(trimmedLine)) { - if betweenBeginAndEnd { - return nil, fmt.Errorf("found second begin mark while updating macro blocks") - } - betweenBeginAndEnd = true - buffer.WriteString(line) - buffer.WriteString("\n") - match := beginMarkExp.FindStringSubmatch(line) - if len(match) < 4 { - return nil, fmt.Errorf("failed to parse the link in example header") - } - // match[0] is the entire expression; [1] is the link text and [3] is the file type (yaml or json). - linkText := match[1] - fileType := match[3] - example, err := exampleContent(filePath, linkText, fileType) - if err != nil { - return nil, err - } - buffer.WriteString(example) - } else if trimmedLine == endMark { - if !betweenBeginAndEnd { - return nil, fmt.Errorf("found end mark without being mark while updating macro blocks") - } - // 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 + content := fmt.Sprintf("\n```%s\n%s\n```\n\n[Download example](%s)", fileType, trimmedFileContent, fileRel) + out := getMungeLines(content) + return out, nil } diff --git a/cmd/mungedocs/example_syncer_test.go b/cmd/mungedocs/example_syncer_test.go index 76b90728402..84fd8854a1e 100644 --- a/cmd/mungedocs/example_syncer_test.go +++ b/cmd/mungedocs/example_syncer_test.go @@ -35,24 +35,27 @@ spec: - containerPort: 80 ` var cases = []struct { - in string - out string + in string + expected string }{ {"", ""}, { - "\n\n", - "\n\n```yaml\n" + podExample + "```\n\n[Download example](testdata/pod.yaml)\n\n", + "\n\n", + "\n\n```yaml\n" + podExample + "```\n\n[Download example](testdata/pod.yaml)\n\n", }, { - "\n\n", - "\n\n```yaml\n" + podExample + "```\n\n[Download example](../mungedocs/testdata/pod.yaml)\n\n", + "\n\n", + "\n\n```yaml\n" + podExample + "```\n\n[Download example](../mungedocs/testdata/pod.yaml)\n\n", }, } + repoRoot = "" for _, c := range cases { - actual, err := syncExamples("mungedocs/filename.md", []byte(c.in)) + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := syncExamples("filename.md", in) assert.NoError(t, err) - if c.out != string(actual) { - t.Errorf("Expected example \n'%v' but got \n'%v'", c.out, string(actual)) + if !expected.Equal(actual) { + t.Errorf("Expected example \n'%q' but got \n'%q'", expected.String(), actual.String()) } } } diff --git a/cmd/mungedocs/headers.go b/cmd/mungedocs/headers.go index 0f45f609423..313714afd4d 100644 --- a/cmd/mungedocs/headers.go +++ b/cmd/mungedocs/headers.go @@ -19,53 +19,56 @@ package main import ( "fmt" "regexp" - "strings" ) var headerRegex = regexp.MustCompile(`^(#+)\s*(.*)$`) -var whitespaceRegex = regexp.MustCompile(`^\s*$`) -func fixHeaderLines(fileBytes []byte) []byte { - lines := splitLines(fileBytes) - out := []string{} - for i := range lines { - matches := headerRegex.FindStringSubmatch(lines[i]) - if matches == nil { - out = append(out, lines[i]) - continue - } - if i > 0 && !whitespaceRegex.Match([]byte(out[len(out)-1])) { - out = append(out, "") - } - out = append(out, fmt.Sprintf("%s %s", matches[1], matches[2])) - if i+1 < len(lines) && !whitespaceRegex.Match([]byte(lines[i+1])) { - out = append(out, "") +func fixHeaderLine(mlines mungeLines, newlines mungeLines, linenum int) mungeLines { + var out mungeLines + + mline := mlines[linenum] + line := mlines[linenum].data + + matches := headerRegex.FindStringSubmatch(line) + if matches == nil { + out = append(out, mline) + return out + } + + // There must be a blank line before the # (unless first line in file) + if linenum != 0 { + newlen := len(newlines) + if newlines[newlen-1].data != "" { + out = append(out, blankMungeLine) } } - final := strings.Join(out, "\n") - // Preserve the end of the file. - if len(fileBytes) > 0 && fileBytes[len(fileBytes)-1] == '\n' { - final += "\n" + + // There must be a space AFTER the ##'s + newline := fmt.Sprintf("%s %s", matches[1], matches[2]) + newmline := newMungeLine(newline) + out = append(out, newmline) + + // The next line needs to be a blank line (unless last line in file) + if len(mlines) > linenum+1 && mlines[linenum+1].data != "" { + out = append(out, blankMungeLine) } - return []byte(final) + return out } // Header lines need whitespace around them and after the #s. -func checkHeaderLines(filePath string, fileBytes []byte) ([]byte, error) { - fbs := splitByPreformatted(fileBytes) - fbs = append([]fileBlock{{false, []byte{}}}, fbs...) - fbs = append(fbs, fileBlock{false, []byte{}}) - - for i := range fbs { - block := &fbs[i] - if block.preformatted { +func checkHeaderLines(filePath string, mlines mungeLines) (mungeLines, error) { + var out mungeLines + for i, mline := range mlines { + if mline.preformatted { + out = append(out, mline) continue } - block.data = fixHeaderLines(block.data) + if !mline.header { + out = append(out, mline) + continue + } + newLines := fixHeaderLine(mlines, out, i) + out = append(out, newLines...) } - output := []byte{} - for _, block := range fbs { - output = append(output, block.data...) - } - return output, nil + return out, nil } diff --git a/cmd/mungedocs/headers_test.go b/cmd/mungedocs/headers_test.go index d2864377072..7b348b2f7f5 100644 --- a/cmd/mungedocs/headers_test.go +++ b/cmd/mungedocs/headers_test.go @@ -24,8 +24,8 @@ import ( func TestHeaderLines(t *testing.T) { var cases = []struct { - in string - out string + in string + expected string }{ {"", ""}, { @@ -62,10 +62,12 @@ func TestHeaderLines(t *testing.T) { }, } for i, c := range cases { - actual, err := checkHeaderLines("filename.md", []byte(c.in)) + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := checkHeaderLines("filename.md", in) assert.NoError(t, err) - if string(actual) != c.out { - t.Errorf("case[%d]: expected %q got %q", i, c.out, string(actual)) + if !actual.Equal(expected) { + t.Errorf("case[%d]: expected %q got %q", i, c.expected, actual.String()) } } } diff --git a/cmd/mungedocs/kubectl_dash_f.go b/cmd/mungedocs/kubectl_dash_f.go index 79b42ba5bd5..cf196c12575 100644 --- a/cmd/mungedocs/kubectl_dash_f.go +++ b/cmd/mungedocs/kubectl_dash_f.go @@ -25,29 +25,25 @@ import ( // Looks for lines that have kubectl commands with -f flags and files that // don't exist. -func checkKubectlFileTargets(file string, markdown []byte) ([]byte, error) { - inside := false - lines := splitLines(markdown) - errors := []string{} - for i := range lines { - if strings.HasPrefix(lines[i], "```") { - inside = !inside +func checkKubectlFileTargets(file string, mlines mungeLines) (mungeLines, error) { + var errors []string + for i, mline := range mlines { + if !mline.preformatted { + continue } - if inside { - if err := lookForKubectl(lines, i); err != nil { - errors = append(errors, err.Error()) - } + if err := lookForKubectl(mline.data, i); err != nil { + errors = append(errors, err.Error()) } } err := error(nil) if len(errors) != 0 { err = fmt.Errorf("%s", strings.Join(errors, "\n")) } - return markdown, err + return mlines, err } -func lookForKubectl(lines []string, lineNum int) error { - fields := strings.Fields(lines[lineNum]) +func lookForKubectl(line string, lineNum int) error { + fields := strings.Fields(line) for i := range fields { if fields[i] == "kubectl" { return gotKubectl(lineNum, fields, i) @@ -56,26 +52,26 @@ func lookForKubectl(lines []string, lineNum int) error { return nil } -func gotKubectl(line int, fields []string, fieldNum int) error { +func gotKubectl(lineNum int, fields []string, fieldNum int) error { for i := fieldNum + 1; i < len(fields); i++ { switch fields[i] { case "create", "update", "replace", "delete": - return gotCommand(line, fields, i) + return gotCommand(lineNum, fields, i) } } return nil } -func gotCommand(line int, fields []string, fieldNum int) error { +func gotCommand(lineNum int, fields []string, fieldNum int) error { for i := fieldNum + 1; i < len(fields); i++ { if strings.HasPrefix(fields[i], "-f") { - return gotDashF(line, fields, i) + return gotDashF(lineNum, fields, i) } } return nil } -func gotDashF(line int, fields []string, fieldNum int) error { +func gotDashF(lineNum int, fields []string, fieldNum int) error { target := "" if fields[fieldNum] == "-f" { if fieldNum+1 == len(fields) { @@ -112,9 +108,9 @@ func gotDashF(line int, fields []string, fieldNum int) error { } // If we got here we expect the file to exist. - _, err := os.Stat(path.Join(*rootDir, *repoRoot, target)) + _, err := os.Stat(path.Join(repoRoot, target)) if os.IsNotExist(err) { - return fmt.Errorf("%d: target file %q does not exist", line, target) + return fmt.Errorf("%d: target file %q does not exist", lineNum, target) } return err } diff --git a/cmd/mungedocs/kubectl_dash_f_test.go b/cmd/mungedocs/kubectl_dash_f_test.go index c2b72343780..cf9a668856c 100644 --- a/cmd/mungedocs/kubectl_dash_f_test.go +++ b/cmd/mungedocs/kubectl_dash_f_test.go @@ -130,9 +130,9 @@ func TestKubectlDashF(t *testing.T) { }, } for i, c := range cases { - *rootDir = "" - *repoRoot = "" - _, err := checkKubectlFileTargets("filename.md", []byte(c.in)) + repoRoot = "" + in := getMungeLines(c.in) + _, err := checkKubectlFileTargets("filename.md", in) if err != nil && c.ok { t.Errorf("case[%d]: expected success, got %v", i, err) } diff --git a/cmd/mungedocs/links.go b/cmd/mungedocs/links.go index 9cccbb2ea93..a8afce8a666 100644 --- a/cmd/mungedocs/links.go +++ b/cmd/mungedocs/links.go @@ -29,20 +29,20 @@ var ( // Finds markdown links of the form [foo](bar "alt-text"). linkRE = regexp.MustCompile(`\[([^]]*)\]\(([^)]*)\)`) // Splits the link target into link target and alt-text. - altTextRE = regexp.MustCompile(`(.*)( ".*")`) + altTextRE = regexp.MustCompile(`([^)]*)( ".*")`) ) -// checkLinks assumes fileBytes has links in markdown syntax, and verifies that -// any relative links actually point to files that exist. -func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { - dir := path.Dir(filePath) - errors := []string{} - - output := replaceNonPreformattedRegexp(fileBytes, linkRE, func(in []byte) (out []byte) { - match := linkRE.FindSubmatch(in) - // match[0] is the entire expression; [1] is the visible text and [2] is the link text. - visibleText := string(match[1]) - linkText := string(match[2]) +func processLink(in string, filePath string) (string, error) { + var err error + out := linkRE.ReplaceAllStringFunc(in, func(in string) string { + match := linkRE.FindStringSubmatch(in) + if match == nil { + err = fmt.Errorf("Detected this line had a link, but unable to parse, %v", in) + return "" + } + // match[0] is the entire expression; + visibleText := match[1] + linkText := match[2] altText := "" if parts := altTextRE.FindStringSubmatch(linkText); parts != nil { linkText = parts[1] @@ -54,13 +54,10 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { linkText = strings.Trim(linkText, "\n") linkText = strings.Trim(linkText, " ") - u, err := url.Parse(linkText) - if err != nil { - errors = append( - errors, - fmt.Sprintf("link %q is unparsable: %v", linkText, err), - ) - return in + u, terr := url.Parse(linkText) + if terr != nil { + err = fmt.Errorf("link %q is unparsable: %v", linkText, terr) + return "" } if u.Host != "" && u.Host != "github.com" { @@ -72,10 +69,8 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { if u.Path != "" && !strings.HasPrefix(linkText, "TODO:") { newPath, targetExists := checkPath(filePath, path.Clean(u.Path)) if !targetExists { - errors = append( - errors, - fmt.Sprintf("%q: target not found", linkText), - ) + err = fmt.Errorf("%q: target not found", linkText) + return "" } u.Path = newPath if strings.HasPrefix(u.Path, "/") { @@ -89,11 +84,16 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { // Make the visible text show the absolute path if it's // not nested in or beneath the current directory. if strings.HasPrefix(u.Path, "..") { - suggestedVisibleText = makeRepoRelative(path.Join(dir, u.Path)) + dir := path.Dir(filePath) + suggestedVisibleText, err = makeRepoRelative(path.Join(dir, u.Path), filePath) + if err != nil { + return "" + } } else { suggestedVisibleText = u.Path } - if unescaped, err := url.QueryUnescape(u.String()); err != nil { + var unescaped string + if unescaped, err = url.QueryUnescape(u.String()); err != nil { // Remove %28 type stuff, be nice to humans. // And don't fight with the toc generator. linkText = unescaped @@ -107,18 +107,37 @@ func checkLinks(filePath string, fileBytes []byte) ([]byte, error) { visibleText = suggestedVisibleText } - return []byte(fmt.Sprintf("[%s](%s)", visibleText, linkText+altText)) + return fmt.Sprintf("[%s](%s)", visibleText, linkText+altText) }) + if out == "" { + return in, err + } + return out, nil +} + +// checkLinks assumes lines has links in markdown syntax, and verifies that +// any relative links actually point to files that exist. +func checkLinks(filePath string, mlines mungeLines) (mungeLines, error) { + var out mungeLines + errors := []string{} + + for _, mline := range mlines { + if mline.preformatted || !mline.link { + out = append(out, mline) + continue + } + line, err := processLink(mline.data, filePath) + if err != nil { + errors = append(errors, err.Error()) + } + ml := newMungeLine(line) + out = append(out, ml) + } err := error(nil) if len(errors) != 0 { err = fmt.Errorf("%s", strings.Join(errors, "\n")) } - return output, err -} - -func makeRepoRelative(filePath string) string { - realRoot := path.Join(*rootDir, *repoRoot) + "/" - return strings.TrimPrefix(filePath, realRoot) + return out, err } // We have to append together before path.Clean will be able to tell that stuff diff --git a/cmd/mungedocs/links_test.go b/cmd/mungedocs/links_test.go new file mode 100644 index 00000000000..b28d5742ab5 --- /dev/null +++ b/cmd/mungedocs/links_test.go @@ -0,0 +1,76 @@ +/* +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 ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +var _ = fmt.Printf + +func TestBadLinks(t *testing.T) { + var cases = []struct { + in string + }{ + {"[NOTREADME](https://github.com/GoogleCloudPlatform/kubernetes/tree/master/NOTREADME.md)"}, + {"[NOTREADME](https://github.com/GoogleCloudPlatform/kubernetes/tree/master/docs/NOTREADME.md)"}, + {"[NOTREADME](../NOTREADME.md)"}, + } + for _, c := range cases { + in := getMungeLines(c.in) + _, err := checkLinks("filename.md", in) + assert.Error(t, err) + } +} +func TestGoodLinks(t *testing.T) { + var cases = []struct { + in string + expected string + }{ + {"", ""}, + {"[README](https://github.com/GoogleCloudPlatform/kubernetes/tree/master/README.md)", + "[README](README.md)"}, + {"[README](../README.md)", + "[README](README.md)"}, + {"[README](https://lwn.net)", + "[README](https://lwn.net)"}, + // _ to - + {"[README](https://github.com/GoogleCloudPlatform/kubernetes/tree/master/docs/devel/cli_roadmap.md)", + "[README](../../docs/devel/cli-roadmap.md)"}, + // - to _ + {"[README](../../docs/devel/api-changes.md)", + "[README](../../docs/devel/api_changes.md)"}, + + // Does this even make sense? i dunno + {"[README](/docs/README.md)", + "[README](https://github.com/docs/README.md)"}, + {"[README](/GoogleCloudPlatform/kubernetes/tree/master/docs/README.md)", + "[README](../../docs/README.md)"}, + } + for i, c := range cases { + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := checkLinks("filename.md", in) + assert.NoError(t, err) + if !actual.Equal(expected) { + t.Errorf("case[%d]: expected %q got %q", i, c.expected, actual.String()) + } + } +} diff --git a/cmd/mungedocs/mungedocs.go b/cmd/mungedocs/mungedocs.go index 4a8ca460afb..a1bcce392e3 100644 --- a/cmd/mungedocs/mungedocs.go +++ b/cmd/mungedocs/mungedocs.go @@ -17,7 +17,6 @@ limitations under the License. package main import ( - "bytes" "errors" "fmt" "io/ioutil" @@ -30,15 +29,17 @@ import ( ) var ( - verify = flag.Bool("verify", false, "Exit with status 1 if files would have needed changes but do not change.") - rootDir = flag.String("root-dir", "", "Root directory containing documents to be processed.") - repoRoot = flag.String("repo-root", "..", `Appended to --root-dir to get the repository root. + verify = flag.Bool("verify", false, "Exit with status 1 if files would have needed changes but do not change.") + rootDir = flag.String("root-dir", "", "Root directory containing documents to be processed.") + // "repo-root" seems like a dumb name, this is the relative path (from rootDir) to get to the repoRoot + relRoot = flag.String("repo-root", "..", `Appended to --root-dir to get the repository root. It's done this way so that generally you just have to set --root-dir. Examples: * --root-dir=docs/ --repo-root=.. means the repository root is ./ * --root-dir=/usr/local/long/path/repo/docs/ --repo-root=.. means the repository root is /usr/local/long/path/repo/ * --root-dir=/usr/local/long/path/repo/docs/admin --repo-root=../.. means the repository root is /usr/local/long/path/repo/`) skipMunges = flag.String("skip-munges", "", "Comma-separated list of munges to *not* run. Available munges are: "+availableMungeList) + repoRoot string ErrChangesNeeded = errors.New("mungedocs: changes required") @@ -50,7 +51,7 @@ Examples: {"check-links", checkLinks}, {"blank-lines-surround-preformatted", checkPreformatted}, {"header-lines", checkHeaderLines}, - {"analytics", checkAnalytics}, + {"analytics", updateAnalytics}, {"kubectl-dash-f", checkKubectlFileTargets}, {"sync-examples", syncExamples}, } @@ -68,7 +69,7 @@ Examples: // data into a new byte array and return that. type munge struct { name string - fn func(filePath string, before []byte) (after []byte, err error) + fn func(filePath string, mlines mungeLines) (after mungeLines, err error) } type fileProcessor struct { @@ -90,12 +91,14 @@ func (f fileProcessor) visit(path string) error { return err } + mungeLines := getMungeLines(string(fileBytes)) + modificationsMade := false errFound := false filePrinted := false for _, munge := range f.munges { - after, err := munge.fn(path, fileBytes) - if err != nil || !bytes.Equal(after, fileBytes) { + after, err := munge.fn(path, mungeLines) + if err != nil || !after.Equal(mungeLines) { if !filePrinted { fmt.Printf("%s\n----\n", path) filePrinted = true @@ -110,7 +113,7 @@ func (f fileProcessor) visit(path string) error { } fmt.Println("") } - fileBytes = after + mungeLines = after } // Write out new file with any changes. @@ -119,7 +122,7 @@ func (f fileProcessor) visit(path string) error { // We're not allowed to make changes. return ErrChangesNeeded } - ioutil.WriteFile(path, fileBytes, 0644) + ioutil.WriteFile(path, mungeLines.Bytes(), 0644) } if errFound { return ErrChangesNeeded @@ -165,6 +168,7 @@ func wantedMunges() (filtered []munge) { } func main() { + var err error flag.Parse() if *rootDir == "" { @@ -172,11 +176,9 @@ func main() { os.Exit(1) } - // Split the root dir of "foo/docs" into "foo" and "docs". We - // chdir into "foo" and walk "docs" so the walk is always at a - // relative path. - stem, leaf := path.Split(strings.TrimRight(*rootDir, "/")) - if err := os.Chdir(stem); err != nil { + repoRoot = path.Join(*rootDir, *relRoot) + repoRoot, err = filepath.Abs(repoRoot) + if err != nil { fmt.Fprintf(os.Stderr, "ERROR: %v\n", err) os.Exit(2) } @@ -194,7 +196,7 @@ func main() { // changes needed, exit 1 if manual changes are needed. var changesNeeded bool - err := filepath.Walk(leaf, newWalkFunc(&fp, &changesNeeded)) + err = filepath.Walk(*rootDir, newWalkFunc(&fp, &changesNeeded)) if err != nil { fmt.Fprintf(os.Stderr, "ERROR: %v\n", err) os.Exit(2) diff --git a/cmd/mungedocs/preformatted.go b/cmd/mungedocs/preformatted.go index 515b6e3f2c8..00e6a9f3238 100644 --- a/cmd/mungedocs/preformatted.go +++ b/cmd/mungedocs/preformatted.go @@ -16,40 +16,26 @@ limitations under the License. package main -import "bytes" - // Blocks of ``` need to have blank lines on both sides or they don't look // right in HTML. -func checkPreformatted(filePath string, fileBytes []byte) ([]byte, error) { - f := splitByPreformatted(fileBytes) - f = append(fileBlocks{{false, []byte{}}}, f...) - f = append(f, fileBlock{false, []byte{}}) - - output := []byte(nil) - for i := 1; i < len(f)-1; i++ { - prev := &f[i-1] - block := &f[i] - next := &f[i+1] - if !block.preformatted { - continue - } - neededSuffix := []byte("\n\n") - for !bytes.HasSuffix(prev.data, neededSuffix) { - prev.data = append(prev.data, '\n') - } - for !bytes.HasSuffix(block.data, neededSuffix) { - block.data = append(block.data, '\n') - if bytes.HasPrefix(next.data, []byte("\n")) { - // don't change the number of newlines unless needed. - next.data = next.data[1:] - if len(next.data) == 0 { - f = append(f[:i+1], f[i+2:]...) - } +func checkPreformatted(filePath string, mlines mungeLines) (mungeLines, error) { + var out mungeLines + inpreformat := false + for i, mline := range mlines { + if !inpreformat && mline.preformatted { + if i == 0 || out[len(out)-1].data != "" { + out = append(out, blankMungeLine) } + // start of a preformat block + inpreformat = true + } + out = append(out, mline) + if inpreformat && !mline.preformatted { + if i >= len(mlines)-2 || mlines[i+1].data != "" { + out = append(out, blankMungeLine) + } + inpreformat = false } } - for _, block := range f { - output = append(output, block.data...) - } - return output, nil + return out, nil } diff --git a/cmd/mungedocs/preformatted_test.go b/cmd/mungedocs/preformatted_test.go new file mode 100644 index 00000000000..637d3598643 --- /dev/null +++ b/cmd/mungedocs/preformatted_test.go @@ -0,0 +1,57 @@ +/* +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 TestPreformatted(t *testing.T) { + var cases = []struct { + in string + expected string + }{ + {"", ""}, + { + "```\nbob\n```", + "\n```\nbob\n```\n\n", + }, + { + "```\nbob\n```\n```\nnotbob\n```\n", + "\n```\nbob\n```\n\n```\nnotbob\n```\n\n", + }, + { + "```bob```\n", + "```bob```\n", + }, + { + " ```\n bob\n ```", + "\n ```\n bob\n ```\n\n", + }, + } + for i, c := range cases { + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := checkPreformatted("filename.md", in) + assert.NoError(t, err) + if !actual.Equal(expected) { + t.Errorf("case[%d]: expected %q got %q", i, c.expected, actual.String()) + } + } +} diff --git a/cmd/mungedocs/toc.go b/cmd/mungedocs/toc.go index d625c16dd1b..8a244eaf0c0 100644 --- a/cmd/mungedocs/toc.go +++ b/cmd/mungedocs/toc.go @@ -17,8 +17,6 @@ limitations under the License. package main import ( - "bufio" - "bytes" "fmt" "regexp" "strings" @@ -26,6 +24,8 @@ import ( const tocMungeTag = "GENERATED_TOC" +var r = regexp.MustCompile("[^A-Za-z0-9-]") + // inserts/updates a table of contents in markdown file. // // First, builds a ToC. @@ -33,15 +33,11 @@ const tocMungeTag = "GENERATED_TOC" // the ToC, thereby updating any previously inserted ToC. // // TODO(erictune): put this in own package with tests -func updateTOC(filePath string, markdown []byte) ([]byte, error) { - toc, err := buildTOC(markdown) +func updateTOC(filePath string, mlines mungeLines) (mungeLines, error) { + toc := buildTOC(mlines) + updatedMarkdown, err := updateMacroBlock(mlines, tocMungeTag, toc) if err != nil { - return nil, err - } - lines := splitLines(markdown) - updatedMarkdown, err := updateMacroBlock(lines, tocMungeTag, string(toc)) - if err != nil { - return nil, err + return mlines, err } return updatedMarkdown, nil } @@ -52,25 +48,19 @@ func updateTOC(filePath string, markdown []byte) ([]byte, error) { // and builds a table of contents from those. Assumes bookmarks for those will be // like #each-word-in-heading-in-lowercases-with-dashes-instead-of-spaces. // builds the ToC. -func buildTOC(markdown []byte) ([]byte, error) { - var buffer bytes.Buffer - buffer.WriteString("\n") - scanner := bufio.NewScanner(bytes.NewReader(markdown)) - inBlockQuotes := false - blockQuoteRegex, err := regexp.Compile("^```") - if err != nil { - return nil, err - } - for scanner.Scan() { - line := scanner.Text() - match := blockQuoteRegex.Match([]byte(line)) - if match { - inBlockQuotes = !inBlockQuotes + +func buildTOC(mlines mungeLines) mungeLines { + var out mungeLines + + for _, mline := range mlines { + if mline.preformatted || !mline.header { continue } - if inBlockQuotes { - continue + // Add a blank line after the munge start tag + if len(out) == 0 { + out = append(out, blankMungeLine) } + line := mline.data noSharps := strings.TrimLeft(line, "#") numSharps := len(line) - len(noSharps) heading := strings.Trim(noSharps, " \n") @@ -78,16 +68,15 @@ func buildTOC(markdown []byte) ([]byte, error) { indent := strings.Repeat(" ", numSharps-1) bookmark := strings.Replace(strings.ToLower(heading), " ", "-", -1) // remove symbols (except for -) in bookmarks - r := regexp.MustCompile("[^A-Za-z0-9-]") bookmark = r.ReplaceAllString(bookmark, "") - tocLine := fmt.Sprintf("%s- [%s](#%s)\n", indent, heading, bookmark) - buffer.WriteString(tocLine) + tocLine := fmt.Sprintf("%s- [%s](#%s)", indent, heading, bookmark) + out = append(out, newMungeLine(tocLine)) } } - if err := scanner.Err(); err != nil { - return []byte{}, err + // Add a blank line before the munge end tag + if len(out) != 0 { + out = append(out, blankMungeLine) } - - return buffer.Bytes(), nil + return out } diff --git a/cmd/mungedocs/toc_test.go b/cmd/mungedocs/toc_test.go index b2f7f4d9925..7b5ddf9b817 100644 --- a/cmd/mungedocs/toc_test.go +++ b/cmd/mungedocs/toc_test.go @@ -24,37 +24,38 @@ import ( func Test_buildTOC(t *testing.T) { var cases = []struct { - in string - out string + in string + expected string }{ - {"", "\n"}, - {"Lorem ipsum\ndolor sit amet\n", "\n"}, + {"", ""}, + {"Lorem ipsum\ndolor sit amet\n", ""}, { "# Title\nLorem ipsum \n## Section Heading\ndolor sit amet\n", - "\n- [Title](#title)\n - [Section Heading](#section-heading)\n", + "\n- [Title](#title)\n - [Section Heading](#section-heading)\n\n", }, { "# Title\nLorem ipsum \n## Section Heading\ndolor sit amet\n```bash\n#!/bin/sh\n```", - "\n- [Title](#title)\n - [Section Heading](#section-heading)\n", + "\n- [Title](#title)\n - [Section Heading](#section-heading)\n\n", }, { "# Title\nLorem ipsum \n## Section Heading\n### Ok, why doesn't this work? ...add 4 *more* `symbols`!\ndolor sit amet\n", - "\n- [Title](#title)\n - [Section Heading](#section-heading)\n - [Ok, why doesn't this work? ...add 4 *more* `symbols`!](#ok-why-doesnt-this-work-add-4-more-symbols)\n", + "\n- [Title](#title)\n - [Section Heading](#section-heading)\n - [Ok, why doesn't this work? ...add 4 *more* `symbols`!](#ok-why-doesnt-this-work-add-4-more-symbols)\n\n", }, } - for _, c := range cases { - actual, err := buildTOC([]byte(c.in)) - assert.NoError(t, err) - if c.out != string(actual) { - t.Errorf("Expected TOC '%v' but got '%v'", c.out, string(actual)) + for i, c := range cases { + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual := buildTOC(in) + if !expected.Equal(actual) { + t.Errorf("Case[%d] Expected TOC '%v' but got '%v'", i, expected.String(), actual.String()) } } } func Test_updateTOC(t *testing.T) { var cases = []struct { - in string - out string + in string + expected string }{ {"", ""}, { @@ -67,10 +68,12 @@ func Test_updateTOC(t *testing.T) { }, } for _, c := range cases { - actual, err := updateTOC("filename.md", []byte(c.in)) + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := updateTOC("filename.md", in) assert.NoError(t, err) - if c.out != string(actual) { - t.Errorf("Expected TOC '%v' but got '%v'", c.out, string(actual)) + if !expected.Equal(actual) { + t.Errorf("Expected TOC '%v' but got '%v'", expected.String(), actual.String()) } } } diff --git a/cmd/mungedocs/unversioned_warning.go b/cmd/mungedocs/unversioned_warning.go index 9c760417f01..b86e72ebe57 100644 --- a/cmd/mungedocs/unversioned_warning.go +++ b/cmd/mungedocs/unversioned_warning.go @@ -20,7 +20,7 @@ import "fmt" const unversionedWarningTag = "UNVERSIONED_WARNING" -const unversionedWarningFmt = ` +const unversionedWarningPre = ` WARNING The latest 1.0.x release of this document can be found -[here](http://releases.k8s.io/release-1.0/%s). +` + +const unversionedWarningFmt = `[here](http://releases.k8s.io/release-1.0/%s).` + +const unversionedWarningPost = ` Documentation for other releases can be found at [releases.k8s.io](http://releases.k8s.io). @@ -49,21 +53,31 @@ Documentation for other releases can be found at -- + ` -func makeUnversionedWarning(fileName string) string { - return fmt.Sprintf(unversionedWarningFmt, fileName) +func makeUnversionedWarning(fileName string) mungeLines { + insert := unversionedWarningPre + fmt.Sprintf(unversionedWarningFmt, fileName) + unversionedWarningPost + return getMungeLines(insert) } // inserts/updates a warning for unversioned docs -func updateUnversionedWarning(file string, markdown []byte) ([]byte, error) { - lines := splitLines(markdown) - if hasLine(lines, "") { +func updateUnversionedWarning(file string, mlines mungeLines) (mungeLines, error) { + file, err := makeRepoRelative(file, file) + if err != nil { + return mlines, err + } + if hasLine(mlines, "") { // No warnings on release branches - return markdown, nil + return mlines, nil } - if !hasMacroBlock(lines, unversionedWarningTag) { - lines = prependMacroBlock(unversionedWarningTag, lines) + if !hasMacroBlock(mlines, unversionedWarningTag) { + mlines = prependMacroBlock(unversionedWarningTag, mlines) } - return updateMacroBlock(lines, unversionedWarningTag, makeUnversionedWarning(file)) + + mlines, err = updateMacroBlock(mlines, unversionedWarningTag, makeUnversionedWarning(file)) + if err != nil { + return mlines, err + } + return mlines, nil } diff --git a/cmd/mungedocs/unversioned_warning_test.go b/cmd/mungedocs/unversioned_warning_test.go index 1d66a4709a2..4d758bd2b98 100644 --- a/cmd/mungedocs/unversioned_warning_test.go +++ b/cmd/mungedocs/unversioned_warning_test.go @@ -26,15 +26,16 @@ func TestUnversionedWarning(t *testing.T) { beginMark := beginMungeTag(unversionedWarningTag) endMark := endMungeTag(unversionedWarningTag) - warningBlock := beginMark + "\n" + makeUnversionedWarning("filename.md") + "\n" + endMark + "\n" + warningString := makeUnversionedWarning("filename.md").String() + warningBlock := beginMark + "\n" + warningString + endMark + "\n" var cases = []struct { - in string - out string + in string + expected string }{ {"", warningBlock}, { "Foo\nBar\n", - warningBlock + "Foo\nBar\n", + warningBlock + "\nFoo\nBar\n", }, { "Foo\n\nBar", @@ -58,10 +59,12 @@ func TestUnversionedWarning(t *testing.T) { }, } for i, c := range cases { - actual, err := updateUnversionedWarning("filename.md", []byte(c.in)) + in := getMungeLines(c.in) + expected := getMungeLines(c.expected) + actual, err := updateUnversionedWarning("filename.md", in) assert.NoError(t, err) - if string(actual) != c.out { - t.Errorf("case[%d]: expected %q got %q", i, c.out, string(actual)) + if !expected.Equal(actual) { + t.Errorf("case[%d]: expected %v got %v", i, expected.String(), actual.String()) } } } diff --git a/cmd/mungedocs/util.go b/cmd/mungedocs/util.go index d7a88885279..01ac20a5d3e 100644 --- a/cmd/mungedocs/util.go +++ b/cmd/mungedocs/util.go @@ -17,102 +17,140 @@ limitations under the License. package main import ( - "bytes" "fmt" + "path" + "path/filepath" "regexp" "strings" + "unicode" ) -// 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, token, insertThis string) ([]byte, error) { +func updateMacroBlock(mlines mungeLines, token string, insertThis mungeLines) (mungeLines, error) { beginMark := beginMungeTag(token) endMark := endMungeTag(token) - var buffer bytes.Buffer + var out mungeLines betweenBeginAndEnd := false - for _, line := range lines { - trimmedLine := strings.Trim(line, " \n") - if trimmedLine == beginMark { + for _, mline := range mlines { + if mline.preformatted && !betweenBeginAndEnd { + out = append(out, mline) + continue + } + line := mline.data + if mline.beginTag && line == 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 { + out = append(out, mline) + } else if mline.endTag && line == endMark { if !betweenBeginAndEnd { - return nil, fmt.Errorf("found end mark without being mark while updating macro blocks") + return nil, fmt.Errorf("found end mark without begin 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 + out = append(out, insertThis...) + out = append(out, mline) } else { if !betweenBeginAndEnd { - buffer.WriteString(line) - buffer.WriteString("\n") + out = append(out, mline) } } } if betweenBeginAndEnd { return nil, fmt.Errorf("never found closing end mark while updating macro blocks") } - return buffer.Bytes(), nil + return out, 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 { +func hasLine(lines mungeLines, needle string) bool { + for _, mline := range lines { + haystack := strings.TrimSpace(mline.data) + if haystack == needle { return true } } return false } -// Add a macro block to the beginning of a set of lines -func prependMacroBlock(token string, lines []string) []string { +func removeMacroBlock(token string, mlines mungeLines) (mungeLines, error) { beginMark := beginMungeTag(token) endMark := endMungeTag(token) - return append([]string{beginMark, endMark}, lines...) + var out mungeLines + betweenBeginAndEnd := false + for _, mline := range mlines { + if mline.preformatted { + out = append(out, mline) + continue + } + line := mline.data + if mline.beginTag && line == beginMark { + if betweenBeginAndEnd { + return nil, fmt.Errorf("found second begin mark while updating macro blocks") + } + betweenBeginAndEnd = true + } else if mline.endTag && line == endMark { + if !betweenBeginAndEnd { + return nil, fmt.Errorf("found end mark without begin mark while updating macro blocks") + } + betweenBeginAndEnd = false + } else { + if !betweenBeginAndEnd { + out = append(out, mline) + } + } + } + if betweenBeginAndEnd { + return nil, fmt.Errorf("never found closing end mark while updating macro blocks") + } + return out, nil +} + +// Add a macro block to the beginning of a set of lines +func prependMacroBlock(token string, mlines mungeLines) mungeLines { + beginLine := newMungeLine(beginMungeTag(token)) + endLine := newMungeLine(endMungeTag(token)) + out := mungeLines{beginLine, endLine} + if len(mlines) > 0 && mlines[0].data != "" { + out = append(out, blankMungeLine) + } + return append(out, mlines...) } // Add a macro block to the end of a set of lines -func appendMacroBlock(token string, lines []string) []string { - beginMark := beginMungeTag(token) - endMark := endMungeTag(token) - return append(lines, beginMark, endMark) +func appendMacroBlock(mlines mungeLines, token string) mungeLines { + beginLine := newMungeLine(beginMungeTag(token)) + endLine := newMungeLine(endMungeTag(token)) + out := mlines + if len(mlines) > 0 && mlines[len(mlines)-1].data != "" { + out = append(out, blankMungeLine) + } + return append(out, beginLine, endLine) } // Tests that a document, represented as a slice of lines, has a macro block. -func hasMacroBlock(lines []string, token string) bool { +func hasMacroBlock(lines mungeLines, token string) bool { beginMark := beginMungeTag(token) endMark := endMungeTag(token) foundBegin := false - for _, line := range lines { - trimmedLine := strings.Trim(line, " \n") + for _, mline := range lines { + if mline.preformatted { + continue + } + if !mline.beginTag && !mline.endTag { + continue + } + line := mline.data switch { - case !foundBegin && trimmedLine == beginMark: + case !foundBegin && line == beginMark: foundBegin = true - case foundBegin && trimmedLine == endMark: + case foundBegin && line == endMark: return true } } @@ -131,72 +169,123 @@ func endMungeTag(desc string) string { return fmt.Sprintf("", desc) } -// Calls 'replace' for all sections of the document not in ``` / ``` blocks. So -// that you don't have false positives inside those blocks. -func replaceNonPreformatted(input []byte, replace func([]byte) []byte) []byte { - f := splitByPreformatted(input) - output := []byte(nil) - for _, block := range f { - if block.preformatted { - output = append(output, block.data...) - } else { - output = append(output, replace(block.data)...) +type mungeLine struct { + data string + preformatted bool + header bool + link bool + beginTag bool + endTag bool +} + +type mungeLines []mungeLine + +func (m1 mungeLines) Equal(m2 mungeLines) bool { + if len(m1) != len(m2) { + return false + } + for i := range m1 { + if m1[i].data != m2[i].data { + return false } } - return output + return true } -type fileBlock struct { - preformatted bool - data []byte +func (mlines mungeLines) String() string { + slice := []string{} + for _, mline := range mlines { + slice = append(slice, mline.data) + } + s := strings.Join(slice, "\n") + // We need to tack on an extra newline at the end of the file + return s + "\n" } -type fileBlocks []fileBlock +func (mlines mungeLines) Bytes() []byte { + return []byte(mlines.String()) +} var ( // Finds all preformatted block start/stops. preformatRE = regexp.MustCompile("^\\s*```") notPreformatRE = regexp.MustCompile("^\\s*```.*```") + // Is this line a header? + mlHeaderRE = regexp.MustCompile(`^#`) + // Is there a link on this line? + mlLinkRE = regexp.MustCompile(`\[[^]]*\]\([^)]*\)`) + beginTagRE = regexp.MustCompile(`