Skip to content

Commit

Permalink
preprocessor: ellipsis sanitiser should always add trailing dots
Browse files Browse the repository at this point in the history
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 <[email protected]>
Change-Id: I3097711c62d78b925efd4297b817a5bbef2e12b8
Dispatch-Trailer: {"type":"trybot","CL":1177002,"patchset":8,"ref":"refs/changes/02/1177002/8","targetBranch":"alpha"}
  • Loading branch information
myitcv authored and cueckoo committed Feb 20, 2024
1 parent ed8ddf1 commit daba442
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 11 deletions.
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -52,13 +53,15 @@ 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=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
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=
Expand Down
41 changes: 35 additions & 6 deletions internal/cmd/preprocessor/cmd/sanitisers.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,22 +209,51 @@ 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"`
}

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
}

Expand Down
76 changes: 76 additions & 0 deletions internal/cmd/preprocessor/cmd/sanitisers_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}

}
14 changes: 9 additions & 5 deletions internal/cmd/preprocessor/cmd/script_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -71,3 +76,8 @@ $ seq 1 20
2
...
```

```text { title="TERMINAL" codeToCopy="dHJ1ZQo=" }
$ true
...
```

0 comments on commit daba442

Please sign in to comment.