From daba442ec4da950d8dd6e15615c19e050188e619 Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Sat, 17 Feb 2024 05:37:55 +0000 Subject: [PATCH] preprocessor: ellipsis sanitiser should always add trailing dots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some commands like 'go mod tidy' might not produce lots of output under certain conditions, e.g. when the modules they require are part of a cache. In that scenario, CI might well produce a different output because no cache is being used. The author however wants to use a cache to ensure that writing the guide does not take forever (the write-save-rerun cycle can get very expensive). In such a situation, an ellipsis sanitiser can be used to indicate this command _might_ produce lots of output, which disguises whether there was any output when it can vary based on cache state. This CL makes the change to an ellipsis sanitiser to always add a trailing '…'. Which now has the meaning "there might be lots of output" rather than "there is lots of output". This CL also adds quicktest as a dependency to help make our test code more succinct and readable. We also fix a bug in the transformation of script nodes, where a clear line was printed before a doc comment, ignoring whether that doc comment actually had any non-tag directive lines (because if a doc comment is only made up of tag directives, no comment will be printed and so the clear line is redundant). Preprocessor-No-Write-Cache: true Signed-off-by: Paul Jolly Change-Id: I3097711c62d78b925efd4297b817a5bbef2e12b8 Dispatch-Trailer: {"type":"trybot","CL":1177002,"patchset":8,"ref":"refs/changes/02/1177002/8","targetBranch":"alpha"} --- go.mod | 4 + go.sum | 3 + internal/cmd/preprocessor/cmd/sanitisers.go | 41 ++++++++-- .../cmd/preprocessor/cmd/sanitisers_test.go | 76 +++++++++++++++++++ internal/cmd/preprocessor/cmd/script_node.go | 14 ++-- .../testdata/execute_ellipsis_sanitiser.txtar | 10 +++ 6 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 internal/cmd/preprocessor/cmd/sanitisers_test.go diff --git a/go.mod b/go.mod index 29c7322cc..fcb6e4018 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/aws/aws-lambda-go v1.32.1 github.com/cockroachdb/apd/v2 v2.0.2 github.com/fsnotify/fsnotify v1.6.0 + github.com/go-quicktest/qt v1.101.0 github.com/rogpeppe/go-internal v1.11.1-0.20231026093722-fa6a31e0812c github.com/rogpeppe/testscript v1.1.0 github.com/spf13/cobra v1.7.0 @@ -23,8 +24,11 @@ require ( cuelabs.dev/go/oci/ociregistry v0.0.0-20231103182354-93e78c079a13 // indirect github.com/cockroachdb/apd/v3 v3.2.1 // indirect github.com/emicklei/proto v1.10.0 // indirect + github.com/google/go-cmp v0.5.9 // indirect github.com/google/uuid v1.3.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/kr/pretty v0.3.1 // indirect + github.com/kr/text v0.2.0 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de // indirect github.com/opencontainers/go-digest v1.0.0 // indirect diff --git a/go.sum b/go.sum index 1e2d2ab1d..3bb9483fb 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,7 @@ github.com/cockroachdb/apd/v3 v3.2.1 h1:U+8j7t0axsIgvQUqthuNm82HIrYXodOV2iWLWtEa github.com/cockroachdb/apd/v3 v3.2.1/go.mod h1:klXJcjp+FffLTHlhIG69tezTDvdP065naDsHzKhYSqc= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -52,6 +53,7 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0-rc4 h1:oOxKUJWnFC4YGHCCMNql1x4YaDfYBTS5Y4x/Cgeo1E0= github.com/opencontainers/image-spec v1.1.0-rc4/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -59,6 +61,7 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0 h1:sadMIsgmHpEOGbUs6VtHBXRR1OHevnj7hLx9ZcdNGW4= github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0/go.mod h1:jgxiZysxFPM+iWKwQwPR+y+Jvo54ARd4EisXxKYpB5c= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.11.1-0.20231026093722-fa6a31e0812c h1:fPpdjePK1atuOg28PXfNSqgwf9I/qD1Hlo39JFwKBXk= github.com/rogpeppe/go-internal v1.11.1-0.20231026093722-fa6a31e0812c/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/rogpeppe/testscript v1.1.0 h1:NxTsoOBQ1zibxf6NDtzrjPbK56hDAteIcOTSINZHtow= diff --git a/internal/cmd/preprocessor/cmd/sanitisers.go b/internal/cmd/preprocessor/cmd/sanitisers.go index f293d9e02..a509a0a31 100644 --- a/internal/cmd/preprocessor/cmd/sanitisers.go +++ b/internal/cmd/preprocessor/cmd/sanitisers.go @@ -209,9 +209,16 @@ func (p *patternSanitiserMatcher) init() error { return nil } -// An ellipsisSanitiser allows very long output to be removed and replaced with -// the canonical '...' which is intended to indicate "and there is is more not -// shown here". +// An ellipsisSanitiser allows output that _might_ be very long to be removed +// and replaced with the canonical '...' which is intended to indicate "and +// there is is more not shown here". The key here is that we add a '...' +// regardless of whether the output exceeds Start and leave it to the caller to +// do what makes most sense. This is because in some situations, for example go +// mod tidy, a command might output lots of text. This can happen, for example, +// when a cache is empty. If the same command is run in a situation where there +// is no output, then there might be no output. In this case we would use an +// ellipsis sanitiser to always print only a '...' so that the output is +// stable. type ellipsisSanitiser struct { Start int `json:"start"` } @@ -219,12 +226,34 @@ type ellipsisSanitiser struct { func (e *ellipsisSanitiser) init() error { return nil } func (e *ellipsisSanitiser) sanitise(cmd *commandStmt) error { - if strings.Count(cmd.Output, "\n") <= e.Start { + // TODO this can likely be optimised once we have it working :) + + if cmd.Output == "" { + cmd.Output = "...\n" return nil } + lines := strings.Split(cmd.Output, "\n") - lines = append(lines[:e.Start], "...") - cmd.Output = strings.Join(lines, "\n") + "\n" // re-add trailing newline + + // "Clip" the lines based on the start or number of lines, whichever is + // smallest. + end := min(len(lines), e.Start) + lines = lines[:end] + + // Drop any empty strings which will appear odd as blank lines just before + // the ... + var i int + for i = end - 1; i >= 0; i-- { + if lines[i] != "" { + break + } + } + lines = lines[:i+1] + + // Add the '...' regardless. + lines = append(lines, "...") + + cmd.Output = strings.Join(lines, "\n") + "\n" return nil } diff --git a/internal/cmd/preprocessor/cmd/sanitisers_test.go b/internal/cmd/preprocessor/cmd/sanitisers_test.go new file mode 100644 index 000000000..44bf87288 --- /dev/null +++ b/internal/cmd/preprocessor/cmd/sanitisers_test.go @@ -0,0 +1,76 @@ +// Copyright 2024 The CUE Authors +// +// 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 cmd + +import ( + "testing" + + "github.com/go-quicktest/qt" +) + +func TestEllipsisSanitiser(t *testing.T) { + cases := []struct { + name string + input string + start int + want string + }{ + { + name: "empty start at zero", + input: "", + start: 0, + want: "...\n", + }, + { + name: "empty start at 1", + input: "", + start: 1, + want: "...\n", + }, + { + name: "empty but non-zero start", + input: "", + start: 10, + want: "...\n", + }, + { + name: "ensure no blank lines before ...", + input: "test\n\nsomething", + start: 2, + want: "test\n...\n", + }, + { + name: "ellipsis at zero", + input: "this\nis\na\ntest\n", + start: 0, + want: "...\n", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cmd := commandStmt{ + Output: c.input, + } + san := ellipsisSanitiser{ + Start: c.start, + } + err := san.sanitise(&cmd) + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.Equals(cmd.Output, c.want)) + }) + } + +} diff --git a/internal/cmd/preprocessor/cmd/script_node.go b/internal/cmd/preprocessor/cmd/script_node.go index a99640687..2f6e975ce 100644 --- a/internal/cmd/preprocessor/cmd/script_node.go +++ b/internal/cmd/preprocessor/cmd/script_node.go @@ -221,15 +221,19 @@ func (s *scriptNode) writeTransformTo(b *bytes.Buffer) error { var lastOutput string for i, stmt := range s.stmts { if stmt.Doc != "" { - if i > 0 && !strings.HasSuffix(lastOutput, "\n\n") { - // Add a clear line to separate the start of the comment - p("\n") - } + var docLines []string for _, line := range strings.Split(stmt.Doc, "\n") { if tagPrefix.MatchString(line) { continue } - p("%s\n", line) + docLines = append(docLines, line) + } + if len(docLines) > 0 && i > 0 && !strings.HasSuffix(lastOutput, "\n\n") { + // Add a clear line to separate the start of the comment + p("\n") + } + for _, l := range docLines { + p("%s\n", l) } } p("$ %s\n", stmt.Cmd) diff --git a/internal/cmd/preprocessor/cmd/testdata/execute_ellipsis_sanitiser.txtar b/internal/cmd/preprocessor/cmd/testdata/execute_ellipsis_sanitiser.txtar index ff7f28041..df920b1fc 100644 --- a/internal/cmd/preprocessor/cmd/testdata/execute_ellipsis_sanitiser.txtar +++ b/internal/cmd/preprocessor/cmd/testdata/execute_ellipsis_sanitiser.txtar @@ -45,6 +45,11 @@ content: dir: page: { >#ellipsis 2 >seq 1 20 >{{{end}}} +> +>{{{with script "en" "no output"}}} +>#ellipsis 1 +>true +>{{{end}}} -- golden/hugo/content/en/dir/index.md -- --- title: JSON Superset @@ -71,3 +76,8 @@ $ seq 1 20 2 ... ``` + +```text { title="TERMINAL" codeToCopy="dHJ1ZQo=" } +$ true +... +```