DKVPX reader: honor --ifs and --ips instead of hard-coding comma and equals (#2172)

The DKVPX record reader set dkvpxReader.Comma = ',' unconditionally,
ignoring the resolved --ifs value; the key-value separator '=' was
likewise hard-coded in the dkvpx parser, ignoring --ips. So e.g.

  printf 'a=1;b=2\n' | mlr -i dkvpx --ifs semicolon --ojson cat

produced a single field {"a": "1;b=2"} instead of {"a": 1, "b": 2}.

Fixes:

* pkg/dkvpx.Reader gains an Equals field (default '=') alongside Comma.
* The DKVPX record reader now passes the resolved IFS/IPS through,
  validating that each is a single character (mirroring the CSV
  reader's "IFS can only be a single character" error) rather than
  silently ignoring or truncating them.
* The DKVPX record writer already emitted OFS/OPS, but its quoting
  decisions were based on the default separators, so with --ofs
  semicolon a value containing ';' went out unquoted (ambiguous) while
  a value containing ',' was quoted needlessly. Quoting now keys off
  the actual OFS/OPS.

Adds unit tests, regression cases under test/cases/io-dkvpx/, and a
note in the separators reference table.

Discovered while investigating #369; tracked in #2170.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

docs/src/reference-main-separators.md

@@ -278,7 +278,7 @@ Notes:
 | [**YAML**](file-formats.md#yaml)   | N/A; documents separated by `---` or single array   | N/A; not alterable    | Always `:`; not alterable |
 | [**DCF**](file-formats.md#dcf-debian-control-file)   | N/A; paragraphs separated by blank lines   | N/A; not alterable    | Always `:`; not alterable |
 | [**DKVP**](file-formats.md#dkvp-key-value-pairs)   | Default `\n`    | Default `,`    | Default `=` |
-| [**DKVPX**](file-formats.md#dkvpx-key-value-pairs-with-csv-style-quoting)   | Default `\n`    | Default `,`    | Default `=` |
+| [**DKVPX**](file-formats.md#dkvpx-key-value-pairs-with-csv-style-quoting)   | Default `\n`    | Default `,`; must be single-character for input    | Default `=`; must be single-character for input |
 | [**NIDX**](file-formats.md#nidx-index-numbered-toolkit-style)   | Default `\n`    | Default space    | None     |
 | [**XTAB**](file-formats.md#xtab-vertical-tabular)   | Not used; records are separated by an extra FS    | `\n` *    | Default: space with repeats  |
 | [**PPRINT**](file-formats.md#pprint-pretty-printed-tabular) | Default `\n` *    | Space with repeats    | None     |

docs/src/reference-main-separators.md.in

@@ -168,7 +168,7 @@ Notes:
 | [**YAML**](file-formats.md#yaml)   | N/A; documents separated by `---` or single array   | N/A; not alterable    | Always `:`; not alterable |
 | [**DCF**](file-formats.md#dcf-debian-control-file)   | N/A; paragraphs separated by blank lines   | N/A; not alterable    | Always `:`; not alterable |
 | [**DKVP**](file-formats.md#dkvp-key-value-pairs)   | Default `\n`    | Default `,`    | Default `=` |
-| [**DKVPX**](file-formats.md#dkvpx-key-value-pairs-with-csv-style-quoting)   | Default `\n`    | Default `,`    | Default `=` |
+| [**DKVPX**](file-formats.md#dkvpx-key-value-pairs-with-csv-style-quoting)   | Default `\n`    | Default `,`; must be single-character for input    | Default `=`; must be single-character for input |
 | [**NIDX**](file-formats.md#nidx-index-numbered-toolkit-style)   | Default `\n`    | Default space    | None     |
 | [**XTAB**](file-formats.md#xtab-vertical-tabular)   | Not used; records are separated by an extra FS    | `\n` *    | Default: space with repeats  |
 | [**PPRINT**](file-formats.md#pprint-pretty-printed-tabular) | Default `\n` *    | Space with repeats    | None     |

pkg/dkvpx/dkvpx_reader.go

@@ -26,6 +26,9 @@ type Reader struct {
 	// Comma is the pair delimiter (default ',').
 	Comma rune
 
+	// Equals is the key-value separator within a pair (default '=').
+	Equals rune
+
 	// Comment, if not 0, causes lines beginning with this character to be skipped.
 	Comment rune
 
@@ -42,8 +45,9 @@ type Reader struct {
 // NewReader returns a new Reader that reads from r.
 func NewReader(r io.Reader) *Reader {
 	return &Reader{
-		Comma: ',',
-		r:     bufio.NewReader(r),
+		Comma:  ',',
+		Equals: '=',
+		r:      bufio.NewReader(r),
 	}
 }
 
@@ -209,7 +213,7 @@ func (r *Reader) readRecord() (*lib.OrderedMap[string], error) {
 			continue
 		}
 
-		if rn == '=' && !haveKey {
+		if rn == r.Equals && !haveKey {
 			haveKey = true
 			line = line[rnLen:]
 			continue

pkg/dkvpx/dkvpx_reader_test.go

@@ -105,6 +105,22 @@ func TestRead_CommentSkipped(t *testing.T) {
 	assert.Equal(t, map[string]string{"a": "1", "b": "2"}, orderedMapToMap(rec))
 }
 
+func TestRead_NonDefaultComma(t *testing.T) {
+	r := NewReader(strings.NewReader("a=1;b=2;c=\"x;y\"\n"))
+	r.Comma = ';'
+	rec, err := r.Read()
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{"a": "1", "b": "2", "c": "x;y"}, orderedMapToMap(rec))
+}
+
+func TestRead_NonDefaultEquals(t *testing.T) {
+	r := NewReader(strings.NewReader("a:1,b:2,c:\"x:y\"\n"))
+	r.Equals = ':'
+	rec, err := r.Read()
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{"a": "1", "b": "2", "c": "x:y"}, orderedMapToMap(rec))
+}
+
 func TestRead_OrderPreserved(t *testing.T) {
 	r := NewReader(strings.NewReader("z=3,x=1,y=2\n"))
 	rec, err := r.Read()

pkg/dkvpx/dkvpx_writer.go

@@ -7,17 +7,25 @@ import (
 	"strings"
 )
 
-// needsQuoting reports whether the string must be quoted (contains comma,
-// equals, newline, or double-quote).
-func needsQuoting(s string) bool {
-	return strings.ContainsAny(s, ",\n\r=\"")
+// needsQuoting reports whether the string must be quoted (contains the
+// pair separator, the key-value separator, newline, or double-quote).
+func needsQuoting(s, ofs, ops string) bool {
+	return strings.ContainsAny(s, "\n\r\"") || strings.Contains(s, ofs) || strings.Contains(s, ops)
 }
 
-// FormatField returns the string formatted for DKVPX output: quoted and escaped
-// if it contains comma, equals, newline, or quote; otherwise unchanged.
-// Useful for callers that need to apply colorization or other wrapping.
+// FormatField returns the string formatted for DKVPX output with the default
+// separators (comma and equals): quoted and escaped if it contains a
+// separator, newline, or quote; otherwise unchanged.
 func FormatField(s string) string {
-	if !needsQuoting(s) {
+	return FormatFieldWithSeparators(s, ",", "=")
+}
+
+// FormatFieldWithSeparators returns the string formatted for DKVPX output:
+// quoted and escaped if it contains the given pair separator (OFS), the given
+// key-value separator (OPS), newline, or quote; otherwise unchanged.
+// Useful for callers that need to apply colorization or other wrapping.
+func FormatFieldWithSeparators(s, ofs, ops string) string {
+	if !needsQuoting(s, ofs, ops) {
 		return s
 	}
 	var b strings.Builder

pkg/dkvpx/dkvpx_writer_test.go

@@ -26,6 +26,15 @@ func TestFormatField_EscapedQuotes(t *testing.T) {
 	assert.Equal(t, `"the ""word"""`, FormatField(`the "word"`))
 }
 
+func TestFormatFieldWithSeparators_NonDefault(t *testing.T) {
+	// With OFS=";" and OPS=":", those characters require quoting; comma and equals do not.
+	assert.Equal(t, `"x;y"`, FormatFieldWithSeparators("x;y", ";", ":"))
+	assert.Equal(t, `"u:v"`, FormatFieldWithSeparators("u:v", ";", ":"))
+	assert.Equal(t, "p,q", FormatFieldWithSeparators("p,q", ";", ":"))
+	assert.Equal(t, "b=c", FormatFieldWithSeparators("b=c", ";", ":"))
+	assert.Equal(t, `"the ""word"""`, FormatFieldWithSeparators(`the "word"`, ";", ":"))
+}
+
 func TestFormatField_RoundTrip(t *testing.T) {
 	input := `"x,y"="a,b,c",z=3` + "\n"
 	rdr := NewReader(strings.NewReader(input))

pkg/input/record_reader_dkvpx.go

@@ -1,10 +1,12 @@
-// RecordReaderDKVPX reads DKVPX format: comma-delimited key=value pairs with
-// CSV-style quoting. It uses the dkvpx package for parsing.
+// RecordReaderDKVPX reads DKVPX format: IFS-delimited (default comma) key=value
+// pairs (default pair separator "=") with CSV-style quoting. It uses the dkvpx
+// package for parsing.
 package input
 
 import (
 	"fmt"
 	"io"
+	"unicode/utf8"
 
 	"github.com/johnkerl/miller/v6/pkg/cli"
 	"github.com/johnkerl/miller/v6/pkg/dkvpx"
@@ -16,6 +18,8 @@ import (
 type RecordReaderDKVPX struct {
 	readerOptions   *cli.TReaderOptions
 	recordsPerBatch int64
+	ifs             rune // The dkvpx reader requires single-character IFS
+	ips             rune // The dkvpx reader requires single-character IPS
 }
 
 func NewRecordReaderDKVPX(
@@ -25,9 +29,19 @@ func NewRecordReaderDKVPX(
 	if readerOptions.IRS != "\n" && readerOptions.IRS != "\r\n" {
 		return nil, fmt.Errorf("for DKVPX, IRS cannot be altered; LF vs CR/LF is autodetected")
 	}
+	if utf8.RuneCountInString(readerOptions.IFS) != 1 {
+		return nil, fmt.Errorf("for DKVPX, IFS can only be a single character")
+	}
+	if utf8.RuneCountInString(readerOptions.IPS) != 1 {
+		return nil, fmt.Errorf("for DKVPX, IPS can only be a single character")
+	}
+	ifs, _ := utf8.DecodeRuneInString(readerOptions.IFS)
+	ips, _ := utf8.DecodeRuneInString(readerOptions.IPS)
 	return &RecordReaderDKVPX{
 		readerOptions:   readerOptions,
 		recordsPerBatch: recordsPerBatch,
+		ifs:             ifs,
+		ips:             ips,
 	}, nil
 }
 
@@ -82,7 +96,8 @@ func (reader *RecordReaderDKVPX) processHandle(
 	recordsPerBatch := reader.recordsPerBatch
 
 	dkvpxReader := dkvpx.NewReader(NewBOMStrippingReader(handle))
-	dkvpxReader.Comma = ','
+	dkvpxReader.Comma = reader.ifs
+	dkvpxReader.Equals = reader.ips
 	if reader.readerOptions.CommentHandling != cli.CommentsAreData &&
 		len(reader.readerOptions.CommentString) == 1 {
 		dkvpxReader.Comment = rune(reader.readerOptions.CommentString[0])

pkg/input/record_reader_dkvpx_test.go

@@ -20,6 +20,53 @@ func TestNewRecordReaderDKVPX(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestNewRecordReaderDKVPX_MultiCharIFSRejected(t *testing.T) {
+	readerOptions := cli.DefaultReaderOptions()
+	readerOptions.InputFileFormat = "dkvpx"
+	assert.NoError(t, cli.FinalizeReaderOptions(&readerOptions))
+	readerOptions.IFS = ";;"
+
+	reader, err := NewRecordReaderDKVPX(&readerOptions, 1)
+	assert.Nil(t, reader)
+	assert.Error(t, err)
+}
+
+func TestNewRecordReaderDKVPX_MultiCharIPSRejected(t *testing.T) {
+	readerOptions := cli.DefaultReaderOptions()
+	readerOptions.InputFileFormat = "dkvpx"
+	assert.NoError(t, cli.FinalizeReaderOptions(&readerOptions))
+	readerOptions.IPS = "::"
+
+	reader, err := NewRecordReaderDKVPX(&readerOptions, 1)
+	assert.Nil(t, reader)
+	assert.Error(t, err)
+}
+
+func TestRecordReaderDKVPX_NonDefaultSeparators(t *testing.T) {
+	readerOptions := cli.DefaultReaderOptions()
+	readerOptions.InputFileFormat = "dkvpx"
+	assert.NoError(t, cli.FinalizeReaderOptions(&readerOptions))
+	readerOptions.IFS = ";"
+	readerOptions.IPS = ":"
+
+	reader, err := NewRecordReaderDKVPX(&readerOptions, 1)
+	assert.NoError(t, err)
+
+	ctx := types.Context{}
+	readerChannel := make(chan []*types.RecordAndContext, 4)
+	errorChannel := make(chan error, 1)
+
+	input := strings.NewReader("x:1;y:\"a;b\"\n")
+	go reader.processHandle(input, "(test)", &ctx, readerChannel, errorChannel, nil)
+
+	records := <-readerChannel
+	assert.Len(t, records, 1)
+	assert.Equal(t, "x", records[0].Record.Head.Key)
+	assert.Equal(t, "1", records[0].Record.Head.Value.String())
+	assert.Equal(t, "y", records[0].Record.Head.Next.Key)
+	assert.Equal(t, "a;b", records[0].Record.Head.Next.Value.String())
+}
+
 func TestRecordReaderDKVPX_ReadStdin(t *testing.T) {
 	readerOptions := cli.DefaultReaderOptions()
 	readerOptions.InputFileFormat = "dkvpx"

pkg/output/record_writer_dkvpx.go

@@ -42,8 +42,8 @@ func (writer *RecordWriterDKVPX) Write(
 		}
 		first = false
 
-		keyStr := dkvpx.FormatField(pe.Key)
-		valStr := dkvpx.FormatField(pe.Value.String())
+		keyStr := dkvpx.FormatFieldWithSeparators(pe.Key, writer.writerOptions.OFS, writer.writerOptions.OPS)
+		valStr := dkvpx.FormatFieldWithSeparators(pe.Value.String(), writer.writerOptions.OFS, writer.writerOptions.OPS)
 
 		bufferedOutputStream.WriteString(colorizer.MaybeColorizeKey(keyStr, outputIsStdout))
 		bufferedOutputStream.WriteString(writer.writerOptions.OPS)

test/cases/io-dkvpx/0001/cmd

@@ -0,0 +1 @@
+mlr --dkvpx cat ${CASEDIR}/input

test/cases/io-dkvpx/0001/expout

@@ -0,0 +1,2 @@
+a=1,b=2,c="x,y"
+d="p=q",e="he said ""hi"""

test/cases/io-dkvpx/0001/input

@@ -0,0 +1,2 @@
+a=1,b=2,c="x,y"
+d="p=q",e="he said ""hi"""

test/cases/io-dkvpx/0002/cmd

@@ -0,0 +1 @@
+mlr -i dkvpx --ifs semicolon --ojson cat ${CASEDIR}/input

test/cases/io-dkvpx/0002/expout

@@ -0,0 +1,12 @@
+[
+{
+  "a": 1,
+  "b": 2,
+  "c": "x;y"
+},
+{
+  "a": 3,
+  "b": 4,
+  "c": "p,q"
+}
+]

test/cases/io-dkvpx/0002/input

@@ -0,0 +1,2 @@
+a=1;b=2;c="x;y"
+a=3;b=4;c="p,q"

test/cases/io-dkvpx/0003/cmd

@@ -0,0 +1 @@
+mlr -i dkvpx --ips colon --ojson cat ${CASEDIR}/input

test/cases/io-dkvpx/0003/expout

@@ -0,0 +1,7 @@
+[
+{
+  "a": 1,
+  "b": 2,
+  "c": "x:y"
+}
+]

test/cases/io-dkvpx/0003/input

@@ -0,0 +1 @@
+a:1,b:2,c:"x:y"

test/cases/io-dkvpx/0004/cmd

@@ -0,0 +1 @@
+mlr --ijson -o dkvpx --ofs semicolon --ops colon cat ${CASEDIR}/input

test/cases/io-dkvpx/0004/expout

@@ -0,0 +1 @@
+a:"x;y";b:p,q;c:"u:v"

test/cases/io-dkvpx/0004/input

@@ -0,0 +1 @@
+{"a": "x;y", "b": "p,q", "c": "u:v"}

test/cases/io-dkvpx/0005/cmd

@@ -0,0 +1 @@
+mlr -i dkvpx --ifs ';;' --ojson cat ${CASEDIR}/input

test/cases/io-dkvpx/0005/experr

@@ -0,0 +1 @@
+mlr: for DKVPX, IFS can only be a single character

test/cases/io-dkvpx/0005/input

@@ -0,0 +1 @@
+a=1;;b=2