Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid spurious [] on JSON output in some cases #1528

Merged
merged 2 commits into from
Mar 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/input/record_reader_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
type RecordReaderJSON struct {
readerOptions *cli.TReaderOptions
recordsPerBatch int64 // distinct from readerOptions.RecordsPerBatch for join/repl
// XXX 1513
sawBrackets bool
}

func NewRecordReaderJSON(
Expand Down Expand Up @@ -65,6 +67,7 @@ func (reader *RecordReaderJSON) Read(
}
}
}
context.JSONHadBrackets = reader.sawBrackets
readerChannel <- types.NewEndOfStreamMarkerList(&context)
}

Expand Down Expand Up @@ -137,6 +140,9 @@ func (reader *RecordReaderJSON) processHandle(
}

} else if mlrval.IsArray() {

reader.sawBrackets = true

records := mlrval.GetArray()
if records == nil {
errorChannel <- fmt.Errorf("internal coding error detected in JSON record-reader")
Expand Down
6 changes: 4 additions & 2 deletions pkg/output/channel_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func channelWriterHandleBatch(

if !recordAndContext.EndOfStream {
record := recordAndContext.Record
context := &recordAndContext.Context

// XXX more
// XXX also make sure this results in exit 1 & goroutine cleanup
Expand Down Expand Up @@ -94,7 +95,7 @@ func channelWriterHandleBatch(
}

if record != nil {
err := recordWriter.Write(record, bufferedOutputStream, outputIsStdout)
err := recordWriter.Write(record, context, bufferedOutputStream, outputIsStdout)
if err != nil {
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
return true, true
Expand All @@ -115,7 +116,8 @@ func channelWriterHandleBatch(
// queued up. For example, PPRINT needs to see all same-schema
// records before printing any, since it needs to compute max width
// down columns.
err := recordWriter.Write(nil, bufferedOutputStream, outputIsStdout)
context := &recordAndContext.Context
err := recordWriter.Write(nil, context, bufferedOutputStream, outputIsStdout)
if err != nil {
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
return true, true
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bufio"

"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

// IRecordWriter is the abstract interface for all record-writers. They are
Expand All @@ -18,6 +19,7 @@ import (
type IRecordWriter interface {
Write(
outrec *mlrval.Mlrmap,
context *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterCSV struct {
Expand Down Expand Up @@ -41,6 +42,7 @@ func NewRecordWriterCSV(writerOptions *cli.TWriterOptions) (*RecordWriterCSV, er

func (writer *RecordWriterCSV) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_csvlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterCSVLite struct {
Expand All @@ -27,6 +28,7 @@ func NewRecordWriterCSVLite(writerOptions *cli.TWriterOptions) (*RecordWriterCSV

func (writer *RecordWriterCSVLite) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_dkvp.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterDKVP struct {
Expand All @@ -20,6 +21,7 @@ func NewRecordWriterDKVP(writerOptions *cli.TWriterOptions) (*RecordWriterDKVP,

func (writer *RecordWriterDKVP) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
36 changes: 23 additions & 13 deletions pkg/output/record_writer_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

// ----------------------------------------------------------------
Expand All @@ -17,7 +18,7 @@ type RecordWriterJSON struct {
jvQuoteAll bool

// State:
onFirst bool
wroteAnyRecords bool
}

// ----------------------------------------------------------------
Expand All @@ -27,16 +28,17 @@ func NewRecordWriterJSON(writerOptions *cli.TWriterOptions) (*RecordWriterJSON,
jsonFormatting = mlrval.JSON_MULTILINE
}
return &RecordWriterJSON{
writerOptions: writerOptions,
jsonFormatting: jsonFormatting,
jvQuoteAll: writerOptions.JVQuoteAll,
onFirst: true,
writerOptions: writerOptions,
jsonFormatting: jsonFormatting,
jvQuoteAll: writerOptions.JVQuoteAll,
wroteAnyRecords: false,
}, nil
}

// ----------------------------------------------------------------
func (writer *RecordWriterJSON) Write(
outrec *mlrval.Mlrmap,
context *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand All @@ -45,21 +47,22 @@ func (writer *RecordWriterJSON) Write(
}

if writer.writerOptions.WrapJSONOutputInOuterList {
writer.writeWithListWrap(outrec, bufferedOutputStream, outputIsStdout)
writer.writeWithListWrap(outrec, context, bufferedOutputStream, outputIsStdout)
} else {
writer.writeWithoutListWrap(outrec, bufferedOutputStream, outputIsStdout)
writer.writeWithoutListWrap(outrec, context, bufferedOutputStream, outputIsStdout)
}
return nil
}

// ----------------------------------------------------------------
func (writer *RecordWriterJSON) writeWithListWrap(
outrec *mlrval.Mlrmap,
context *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) {
if outrec != nil { // Not end of record stream
if writer.onFirst {
if !writer.wroteAnyRecords {
bufferedOutputStream.WriteString("[\n")
}

Expand All @@ -71,25 +74,32 @@ func (writer *RecordWriterJSON) writeWithListWrap(
os.Exit(1)
}

if !writer.onFirst {
if writer.wroteAnyRecords {
bufferedOutputStream.WriteString(",\n")
}

bufferedOutputStream.WriteString(s)

writer.onFirst = false
writer.wroteAnyRecords = true

} else { // End of record stream
if writer.onFirst { // zero records in the entire output stream
bufferedOutputStream.WriteString("[")

if !writer.wroteAnyRecords {
if context.JSONHadBrackets {
bufferedOutputStream.WriteString("[")
bufferedOutputStream.WriteString("\n]\n")
}
} else {
bufferedOutputStream.WriteString("\n]\n")
}
bufferedOutputStream.WriteString("\n]\n")

}
}

// ----------------------------------------------------------------
func (writer *RecordWriterJSON) writeWithoutListWrap(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterMarkdown struct {
Expand All @@ -29,6 +30,7 @@ func NewRecordWriterMarkdown(writerOptions *cli.TWriterOptions) (*RecordWriterMa
// ----------------------------------------------------------------
func (writer *RecordWriterMarkdown) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_nidx.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterNIDX struct {
Expand All @@ -19,6 +20,7 @@ func NewRecordWriterNIDX(writerOptions *cli.TWriterOptions) (*RecordWriterNIDX,

func (writer *RecordWriterNIDX) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_pprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterPPRINT struct {
Expand All @@ -35,6 +36,7 @@ func NewRecordWriterPPRINT(writerOptions *cli.TWriterOptions) (*RecordWriterPPRI
// ----------------------------------------------------------------
func (writer *RecordWriterPPRINT) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_tsv.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/lib"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

type RecordWriterTSV struct {
Expand All @@ -35,6 +36,7 @@ func NewRecordWriterTSV(writerOptions *cli.TWriterOptions) (*RecordWriterTSV, er

func (writer *RecordWriterTSV) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/output/record_writer_xtab.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/johnkerl/miller/pkg/cli"
"github.com/johnkerl/miller/pkg/colorizer"
"github.com/johnkerl/miller/pkg/mlrval"
"github.com/johnkerl/miller/pkg/types"
)

// ----------------------------------------------------------------
Expand Down Expand Up @@ -43,6 +44,7 @@ func NewRecordWriterXTAB(writerOptions *cli.TWriterOptions) (*RecordWriterXTAB,

func (writer *RecordWriterXTAB) Write(
outrec *mlrval.Mlrmap,
_ *types.Context,
bufferedOutputStream *bufio.Writer,
outputIsStdout bool,
) error {
Expand Down
3 changes: 2 additions & 1 deletion pkg/terminals/repl/verbs.go
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,8 @@ func writeRecord(repl *Repl, outrec *mlrval.Mlrmap) {
outrec.Unflatten(repl.options.WriterOptions.FLATSEP)
}
}
repl.recordWriter.Write(outrec, repl.bufferedRecordOutputStream, true /*outputIsStdout*/)
// XXX TEMP
repl.recordWriter.Write(outrec, nil, repl.bufferedRecordOutputStream, true /*outputIsStdout*/)
repl.bufferedRecordOutputStream.Flush()
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ type Context struct {
// NF int
NR int64
FNR int64

// XXX 1513
JSONHadBrackets bool
}

// TODO: comment: Remember command-line values to pass along to CST evaluators.
Expand Down
2 changes: 0 additions & 2 deletions test/cases/dsl-functional-tests/0051/expout
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,3 @@
"zsgnt": "int"
}
]
[
]
2 changes: 0 additions & 2 deletions test/cases/dsl-output-redirects/0071/expout
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ x
8
9
10
[
]
2 changes: 0 additions & 2 deletions test/cases/dsl-sorts/sorta-natural/expout
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,3 @@
["X200", "X20", "X2", "X100", "X10", "X1"]
["X1", "X2", "X10", "X20", "X100", "X200"]
["X200", "X100", "X20", "X10", "X2", "X1"]
[
]
2 changes: 0 additions & 2 deletions test/cases/dsl-sorts/sortmf-within/expout
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@
"b": 2,
"c": 1
}
[
]
2 changes: 0 additions & 2 deletions test/cases/io-multi/0053/expout
Original file line number Diff line number Diff line change
@@ -1,2 +0,0 @@
[
]
2 changes: 0 additions & 2 deletions test/cases/io-multi/0057/expout
Original file line number Diff line number Diff line change
@@ -1,2 +0,0 @@
[
]
Loading