From 9a5c1e6d79b96e0f786a7686ef41845fd18e3fca Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Sat, 9 May 2026 14:54:02 -0700 Subject: [PATCH 1/6] feat(uninterpreted): add IsComplete callback for cumulative-buffer terminations Add optional IsComplete func(string) bool field to UninterpretedConfig. When non-nil, readUninterpreted dispatches accumulation through a new sibling readMultiLinesAccumFunc that exposes the cumulative buffer to the predicate, enabling consumers (such as the dolt SQL shell) to implement quote-, comment-, and escape-aware termination instead of the legacy per-line suffix check. The legacy line-suffix path is preserved for any consumer that leaves IsComplete nil, so existing consumers see no behavior change. Refs: dolthub/dolt#10866 --- ishell.go | 134 ++++++++++++++++++++++++++++++++++-------- uninterpreted_test.go | 74 +++++++++++++++++++++++ 2 files changed, 184 insertions(+), 24 deletions(-) create mode 100644 uninterpreted_test.go diff --git a/ishell.go b/ishell.go index ed4f4d2..384c1ec 100644 --- a/ishell.go +++ b/ishell.go @@ -65,6 +65,7 @@ type Shell struct { specialTerminators []string backSlashCmds []string quitKeywords []string + isComplete func(accumulated string) bool contextValues Actions } @@ -80,6 +81,14 @@ type UninterpretedConfig struct { BackSlashCmds []string // Quit keywords to exit the shell if discovered QuitKeywords []string + // IsComplete, when non-nil, replaces the per-line LineTerminator suffix + // check in readUninterpreted. It is called after each physical line is + // read with the cumulative buffer (lines joined by "\n", matching the + // final delivered string). Returning true ends accumulation. Special + // terminators, backslash commands, and quit keywords still apply on top + // of IsComplete; first-line DELIMITER detection and the leading "--" + // short-circuit also still apply. + IsComplete func(accumulated string) bool } // New creates a new shell with default settings. Uses standard output and default prompt ">> ". @@ -109,6 +118,7 @@ func NewUninterpreted(conf *UninterpretedConfig) *Shell { shell.specialTerminators = conf.SpecialTerminators shell.backSlashCmds = conf.BackSlashCmds shell.quitKeywords = conf.QuitKeywords + shell.isComplete = conf.IsComplete return shell } @@ -366,39 +376,72 @@ func (s *Shell) readUninterpreted() (string, error) { if s.lineTerminator != "" { firstLine := true - lines, err = s.readMultiLinesFunc(func(line string) (keepReading bool) { - if firstLine { - firstLine = false - if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { - s.lineTerminator = matches[1] - return false + if s.isComplete != nil { + lines, err = s.readMultiLinesAccumFunc(func(line, accumulated string) (keepReading bool) { + if firstLine { + firstLine = false + if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { + s.lineTerminator = matches[1] + return false + } + if strings.HasPrefix(line, "--") { + return false + } + for _, keyword := range s.quitKeywords { + if strings.TrimSpace(line) == keyword { + return false + } + } } - if strings.HasPrefix(line, "--") { - return false + + for _, sc := range s.specialTerminators { + if strings.HasSuffix(strings.TrimSpace(line), sc) { + return false + } } - for _, keyword := range s.quitKeywords { - if strings.TrimSpace(line) == keyword { + for _, sc := range s.backSlashCmds { + if strings.HasPrefix(strings.TrimSpace(line), sc) { return false } } - } - - if strings.HasSuffix(strings.TrimSpace(line), s.lineTerminator) { - return false - } - for _, sc := range s.specialTerminators { - if strings.HasSuffix(strings.TrimSpace(line), sc) { - return false + + return !s.isComplete(accumulated) + }) + } else { + lines, err = s.readMultiLinesFunc(func(line string) (keepReading bool) { + if firstLine { + firstLine = false + if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { + s.lineTerminator = matches[1] + return false + } + if strings.HasPrefix(line, "--") { + return false + } + for _, keyword := range s.quitKeywords { + if strings.TrimSpace(line) == keyword { + return false + } + } } - } - for _, sc := range s.backSlashCmds { - if strings.HasPrefix(strings.TrimSpace(line), sc) { + + if strings.HasSuffix(strings.TrimSpace(line), s.lineTerminator) { return false } - } + for _, sc := range s.specialTerminators { + if strings.HasSuffix(strings.TrimSpace(line), sc) { + return false + } + } + for _, sc := range s.backSlashCmds { + if strings.HasPrefix(strings.TrimSpace(line), sc) { + return false + } + } - return true - }) + return true + }) + } if err != nil { return "", err @@ -508,6 +551,49 @@ func (s *Shell) readMultiLinesFunc(f func(string) (keepReading bool)) (string, e return lines.String(), err } +// readMultiLinesAccumFunc is a sibling of readMultiLinesFunc that passes +// both the most recent physical line and the cumulative buffer to the +// predicate. The cumulative buffer matches what readMultiLinesFunc would +// produce: physical lines joined by "\n" with no trailing newline on the +// terminating line. +func (s *Shell) readMultiLinesAccumFunc(f func(line, accumulated string) (keepReading bool)) (string, error) { + return readMultiLinesAccumFromReader(s.readLine, s.reader.setMultiMode, f) +} + +// readMultiLinesAccumFromReader is the inner accumulation loop used by +// readMultiLinesAccumFunc. It is parameterized on the line reader and the +// multi-mode toggle so unit tests can drive it without a real readline +// instance. +func readMultiLinesAccumFromReader( + readLine func() (string, error), + setMulti func(bool), + f func(line, accumulated string) (keepReading bool), +) (string, error) { + var lines bytes.Buffer + currentLine := 0 + var err error + for { + if currentLine == 1 { + // from second line, enable next line prompt. + setMulti(true) + } + var line string + line, err = readLine() + fmt.Fprint(&lines, line) + if !f(line, lines.String()) || err != nil { + break + } + fmt.Fprintln(&lines) + currentLine++ + } + if currentLine > 0 { + // if more than one line is read + // revert to standard prompt. + setMulti(false) + } + return lines.String(), err +} + func (s *Shell) initCompleters() { s.setCompleter(iCompleter{cmd: s.rootCmd, disabled: func() bool { return s.multiChoiceActive }}) } diff --git a/uninterpreted_test.go b/uninterpreted_test.go new file mode 100644 index 0000000..6b8baf2 --- /dev/null +++ b/uninterpreted_test.go @@ -0,0 +1,74 @@ +package ishell + +import ( + "io" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// linesReader returns a function that yields the supplied lines one at a +// time, then returns io.EOF on subsequent calls. +func linesReader(lines []string) func() (string, error) { + i := 0 + return func() (string, error) { + if i >= len(lines) { + return "", io.EOF + } + l := lines[i] + i++ + return l, nil + } +} + +func TestReadMultiLinesAccumFromReaderTerminator(t *testing.T) { + // Predicate stops accumulation once the buffer contains "GO", regardless of + // per-line semicolon terminators. + read := linesReader([]string{"select 1;", "select 2;", "GO"}) + multiCalls := []bool{} + pred := func(line, accumulated string) bool { + return !strings.Contains(accumulated, "GO") + } + out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) + assert.NoError(t, err) + assert.Equal(t, "select 1;\nselect 2;\nGO", out) + // setMulti(true) once at currentLine==1, setMulti(false) once at end. + assert.Equal(t, []bool{true, false}, multiCalls) +} + +func TestReadMultiLinesAccumFromReaderSingleLine(t *testing.T) { + // First line ends accumulation; setMulti is never called. + read := linesReader([]string{"select 1;"}) + multiCalls := []bool{} + pred := func(line, accumulated string) bool { return false } + out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) + assert.NoError(t, err) + assert.Equal(t, "select 1;", out) + assert.Empty(t, multiCalls) +} + +func TestReadMultiLinesAccumFromReaderEmpty(t *testing.T) { + // First line is empty and predicate returns false: shell never enters + // multi-mode; output is empty. This mirrors the empty-Enter path. + read := linesReader([]string{""}) + multiCalls := []bool{} + pred := func(line, accumulated string) bool { return false } + out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) + assert.NoError(t, err) + assert.Equal(t, "", out) + assert.Empty(t, multiCalls) +} + +func TestReadMultiLinesAccumFromReaderEOFMidStream(t *testing.T) { + // EOF arriving after the predicate has chosen to keep reading surfaces + // the EOF error and returns the buffer collected so far. The trailing + // "\n" is the inter-line separator written by fmt.Fprintln before the + // next readLine call returned EOF. + read := linesReader([]string{"select 1;"}) + pred := func(line, accumulated string) bool { return true } + out, err := readMultiLinesAccumFromReader(read, func(b bool) {}, pred) + assert.Equal(t, io.EOF, err) + assert.Equal(t, "select 1;\n", out) +} + From 061915e865687442cd9cbcf8758f0d7dbfeadc6a Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Sun, 7 Jun 2026 14:54:12 -0700 Subject: [PATCH 2/6] refactor(uninterpreted): pass delimiter to IsComplete and DRY readUninterpreted Change UninterpretedConfig.IsComplete to func(accumulated, delimiter string) bool so the callback receives the delimiter currently in effect without the consumer having to capture the shell. Collapse the duplicated terminator logic in readUninterpreted into a single predicate plus a lineIsComplete helper, and make readMultiLinesFunc delegate to readMultiLinesAccumFunc to remove the duplicated accumulation loop. Refs: dolthub/dolt#10866 --- ishell.go | 148 +++++++++++++++++------------------------------------- 1 file changed, 47 insertions(+), 101 deletions(-) diff --git a/ishell.go b/ishell.go index 384c1ec..87812ee 100644 --- a/ishell.go +++ b/ishell.go @@ -65,7 +65,7 @@ type Shell struct { specialTerminators []string backSlashCmds []string quitKeywords []string - isComplete func(accumulated string) bool + isComplete func(accumulated, delimiter string) bool contextValues Actions } @@ -81,14 +81,10 @@ type UninterpretedConfig struct { BackSlashCmds []string // Quit keywords to exit the shell if discovered QuitKeywords []string - // IsComplete, when non-nil, replaces the per-line LineTerminator suffix - // check in readUninterpreted. It is called after each physical line is - // read with the cumulative buffer (lines joined by "\n", matching the - // final delivered string). Returning true ends accumulation. Special - // terminators, backslash commands, and quit keywords still apply on top - // of IsComplete; first-line DELIMITER detection and the leading "--" - // short-circuit also still apply. - IsComplete func(accumulated string) bool + // IsComplete, when set, replaces the LineTerminator suffix check: it is + // called with the cumulative buffer and current delimiter and returns true + // when accumulation should stop. + IsComplete func(accumulated, delimiter string) bool } // New creates a new shell with default settings. Uses standard output and default prompt ">> ". @@ -376,72 +372,24 @@ func (s *Shell) readUninterpreted() (string, error) { if s.lineTerminator != "" { firstLine := true - if s.isComplete != nil { - lines, err = s.readMultiLinesAccumFunc(func(line, accumulated string) (keepReading bool) { - if firstLine { - firstLine = false - if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { - s.lineTerminator = matches[1] - return false - } - if strings.HasPrefix(line, "--") { - return false - } - for _, keyword := range s.quitKeywords { - if strings.TrimSpace(line) == keyword { - return false - } - } - } - - for _, sc := range s.specialTerminators { - if strings.HasSuffix(strings.TrimSpace(line), sc) { - return false - } - } - for _, sc := range s.backSlashCmds { - if strings.HasPrefix(strings.TrimSpace(line), sc) { - return false - } - } - - return !s.isComplete(accumulated) - }) - } else { - lines, err = s.readMultiLinesFunc(func(line string) (keepReading bool) { - if firstLine { - firstLine = false - if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { - s.lineTerminator = matches[1] - return false - } - if strings.HasPrefix(line, "--") { - return false - } - for _, keyword := range s.quitKeywords { - if strings.TrimSpace(line) == keyword { - return false - } - } - } - - if strings.HasSuffix(strings.TrimSpace(line), s.lineTerminator) { + lines, err = s.readMultiLinesAccumFunc(func(line, accumulated string) (keepReading bool) { + if firstLine { + firstLine = false + if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { + s.lineTerminator = matches[1] return false } - for _, sc := range s.specialTerminators { - if strings.HasSuffix(strings.TrimSpace(line), sc) { - return false - } + if strings.HasPrefix(line, "--") { + return false } - for _, sc := range s.backSlashCmds { - if strings.HasPrefix(strings.TrimSpace(line), sc) { + for _, keyword := range s.quitKeywords { + if strings.TrimSpace(line) == keyword { return false } } - - return true - }) - } + } + return !s.lineIsComplete(line, accumulated) + }) if err != nil { return "", err @@ -480,6 +428,27 @@ func (s *Shell) readUninterpreted() (string, error) { return lines, nil } +// lineIsComplete reports whether reading should stop: special terminators and +// backslash commands always end input, otherwise it defers to IsComplete or the +// line-terminator suffix check. +func (s *Shell) lineIsComplete(line, accumulated string) bool { + trimmed := strings.TrimSpace(line) + for _, sc := range s.specialTerminators { + if strings.HasSuffix(trimmed, sc) { + return true + } + } + for _, sc := range s.backSlashCmds { + if strings.HasPrefix(trimmed, sc) { + return true + } + } + if s.isComplete != nil { + return s.isComplete(accumulated, s.lineTerminator) + } + return strings.HasSuffix(trimmed, s.lineTerminator) +} + func (s *Shell) read() ([]string, error) { s.rawArgs = nil eof := "" @@ -526,44 +495,21 @@ func (s *Shell) read() ([]string, error) { } func (s *Shell) readMultiLinesFunc(f func(string) (keepReading bool)) (string, error) { - var lines bytes.Buffer - currentLine := 0 - var err error - for { - if currentLine == 1 { - // from second line, enable next line prompt. - s.reader.setMultiMode(true) - } - var line string - line, err = s.readLine() - fmt.Fprint(&lines, line) - if !f(line) || err != nil { - break - } - fmt.Fprintln(&lines) - currentLine++ - } - if currentLine > 0 { - // if more than one line is read - // revert to standard prompt. - s.reader.setMultiMode(false) - } - return lines.String(), err + return s.readMultiLinesAccumFunc(func(line, _ string) bool { + return f(line) + }) } -// readMultiLinesAccumFunc is a sibling of readMultiLinesFunc that passes -// both the most recent physical line and the cumulative buffer to the -// predicate. The cumulative buffer matches what readMultiLinesFunc would -// produce: physical lines joined by "\n" with no trailing newline on the -// terminating line. +// readMultiLinesAccumFunc accumulates physical lines until the predicate +// returns false. The predicate receives the latest line and the cumulative +// buffer (lines joined by "\n"). func (s *Shell) readMultiLinesAccumFunc(f func(line, accumulated string) (keepReading bool)) (string, error) { return readMultiLinesAccumFromReader(s.readLine, s.reader.setMultiMode, f) } -// readMultiLinesAccumFromReader is the inner accumulation loop used by -// readMultiLinesAccumFunc. It is parameterized on the line reader and the -// multi-mode toggle so unit tests can drive it without a real readline -// instance. +// readMultiLinesAccumFromReader is the inner accumulation loop, parameterized +// on the line reader and multi-mode toggle so tests can drive it without a real +// readline instance. func readMultiLinesAccumFromReader( readLine func() (string, error), setMulti func(bool), From 65ae5c0e8079751f98a550e565b1e70eeb68e0bd Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Mon, 29 Jun 2026 00:00:16 -0700 Subject: [PATCH 3/6] refactor(uninterpreted): remove IsComplete hook; collapse predicate The IsComplete callback and supporting lineIsComplete helper were originally added to allow the accumulation loop to be tested w/ a standalone function (i.e., readMultiLinesAccumFromReader). Decided during PR review that this testing approach is unnecessary since the behavior is covered by Dolt BATS tests. Remove UninterpretedConfig.IsComplete, Shell.isComplete, and lineIsComplete. The SQL branch predicate in readUninterpreted now inlines the special-terminator, backslash-command, and line-terminator checks directly. Delete the standalone accumulation loop unit tests. Fix ReadMultiLinesFunc in actions.go to adapt to the two-argument readMultiLinesFunc predicate. Refs: dolthub/dolt#10866 --- actions.go | 4 ++- ishell.go | 79 +++++++++++++------------------------------ uninterpreted_test.go | 74 ---------------------------------------- 3 files changed, 26 insertions(+), 131 deletions(-) delete mode 100644 uninterpreted_test.go diff --git a/actions.go b/actions.go index c4e2505..f506a37 100644 --- a/actions.go +++ b/actions.go @@ -103,7 +103,9 @@ func (s *shellActionsImpl) ReadPasswordErr() (string, error) { } func (s *shellActionsImpl) ReadMultiLinesFunc(f func(string) (keepReading bool)) string { - lines, _ := s.readMultiLinesFunc(f) + lines, _ := s.readMultiLinesFunc(func(line, _ string) (keepReading bool) { + return f(line) + }) return lines } diff --git a/ishell.go b/ishell.go index 87812ee..103cfe3 100644 --- a/ishell.go +++ b/ishell.go @@ -65,7 +65,6 @@ type Shell struct { specialTerminators []string backSlashCmds []string quitKeywords []string - isComplete func(accumulated, delimiter string) bool contextValues Actions } @@ -81,10 +80,6 @@ type UninterpretedConfig struct { BackSlashCmds []string // Quit keywords to exit the shell if discovered QuitKeywords []string - // IsComplete, when set, replaces the LineTerminator suffix check: it is - // called with the cumulative buffer and current delimiter and returns true - // when accumulation should stop. - IsComplete func(accumulated, delimiter string) bool } // New creates a new shell with default settings. Uses standard output and default prompt ">> ". @@ -114,7 +109,6 @@ func NewUninterpreted(conf *UninterpretedConfig) *Shell { shell.specialTerminators = conf.SpecialTerminators shell.backSlashCmds = conf.BackSlashCmds shell.quitKeywords = conf.QuitKeywords - shell.isComplete = conf.IsComplete return shell } @@ -372,7 +366,7 @@ func (s *Shell) readUninterpreted() (string, error) { if s.lineTerminator != "" { firstLine := true - lines, err = s.readMultiLinesAccumFunc(func(line, accumulated string) (keepReading bool) { + lines, err = s.readMultiLinesFunc(func(line, accumulated string) (keepReading bool) { if firstLine { firstLine = false if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { @@ -388,9 +382,19 @@ func (s *Shell) readUninterpreted() (string, error) { } } } - return !s.lineIsComplete(line, accumulated) + trimmed := strings.TrimSpace(line) + for _, sc := range s.specialTerminators { + if strings.HasSuffix(trimmed, sc) { + return false + } + } + for _, sc := range s.backSlashCmds { + if strings.HasPrefix(trimmed, sc) { + return false + } + } + return !strings.HasSuffix(trimmed, s.lineTerminator) }) - if err != nil { return "", err } @@ -399,7 +403,7 @@ func (s *Shell) readUninterpreted() (string, error) { heredoc := false // heredoc multiline - lines, err = s.readMultiLinesFunc(func(line string) (keepReading bool) { + lines, err = s.readMultiLinesFunc(func(line, _ string) (keepReading bool) { if !heredoc { if strings.Contains(line, "<<") { s := strings.SplitN(line, "<<", 2) @@ -428,34 +432,13 @@ func (s *Shell) readUninterpreted() (string, error) { return lines, nil } -// lineIsComplete reports whether reading should stop: special terminators and -// backslash commands always end input, otherwise it defers to IsComplete or the -// line-terminator suffix check. -func (s *Shell) lineIsComplete(line, accumulated string) bool { - trimmed := strings.TrimSpace(line) - for _, sc := range s.specialTerminators { - if strings.HasSuffix(trimmed, sc) { - return true - } - } - for _, sc := range s.backSlashCmds { - if strings.HasPrefix(trimmed, sc) { - return true - } - } - if s.isComplete != nil { - return s.isComplete(accumulated, s.lineTerminator) - } - return strings.HasSuffix(trimmed, s.lineTerminator) -} - func (s *Shell) read() ([]string, error) { s.rawArgs = nil eof := "" heredoc := false // heredoc multiline - lines, err := s.readMultiLinesFunc(func(line string) (keepReading bool) { + lines, err := s.readMultiLinesFunc(func(line, _ string) (keepReading bool) { if !heredoc { if strings.Contains(line, "<<") { s := strings.SplitN(line, "<<", 2) @@ -494,37 +477,21 @@ func (s *Shell) read() ([]string, error) { return args, err } -func (s *Shell) readMultiLinesFunc(f func(string) (keepReading bool)) (string, error) { - return s.readMultiLinesAccumFunc(func(line, _ string) bool { - return f(line) - }) -} - -// readMultiLinesAccumFunc accumulates physical lines until the predicate -// returns false. The predicate receives the latest line and the cumulative -// buffer (lines joined by "\n"). -func (s *Shell) readMultiLinesAccumFunc(f func(line, accumulated string) (keepReading bool)) (string, error) { - return readMultiLinesAccumFromReader(s.readLine, s.reader.setMultiMode, f) -} - -// readMultiLinesAccumFromReader is the inner accumulation loop, parameterized -// on the line reader and multi-mode toggle so tests can drive it without a real -// readline instance. -func readMultiLinesAccumFromReader( - readLine func() (string, error), - setMulti func(bool), - f func(line, accumulated string) (keepReading bool), -) (string, error) { +// readMultiLinesFunc accumulates physical lines until the predicate returns +// false. The predicate receives the latest line and the cumulative buffer +// (lines joined by "\n"); callers that only need the line can ignore the +// second argument. +func (s *Shell) readMultiLinesFunc(f func(line, accumulated string) (keepReading bool)) (string, error) { var lines bytes.Buffer currentLine := 0 var err error for { if currentLine == 1 { // from second line, enable next line prompt. - setMulti(true) + s.reader.setMultiMode(true) } var line string - line, err = readLine() + line, err = s.readLine() fmt.Fprint(&lines, line) if !f(line, lines.String()) || err != nil { break @@ -535,7 +502,7 @@ func readMultiLinesAccumFromReader( if currentLine > 0 { // if more than one line is read // revert to standard prompt. - setMulti(false) + s.reader.setMultiMode(false) } return lines.String(), err } diff --git a/uninterpreted_test.go b/uninterpreted_test.go deleted file mode 100644 index 6b8baf2..0000000 --- a/uninterpreted_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package ishell - -import ( - "io" - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -// linesReader returns a function that yields the supplied lines one at a -// time, then returns io.EOF on subsequent calls. -func linesReader(lines []string) func() (string, error) { - i := 0 - return func() (string, error) { - if i >= len(lines) { - return "", io.EOF - } - l := lines[i] - i++ - return l, nil - } -} - -func TestReadMultiLinesAccumFromReaderTerminator(t *testing.T) { - // Predicate stops accumulation once the buffer contains "GO", regardless of - // per-line semicolon terminators. - read := linesReader([]string{"select 1;", "select 2;", "GO"}) - multiCalls := []bool{} - pred := func(line, accumulated string) bool { - return !strings.Contains(accumulated, "GO") - } - out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) - assert.NoError(t, err) - assert.Equal(t, "select 1;\nselect 2;\nGO", out) - // setMulti(true) once at currentLine==1, setMulti(false) once at end. - assert.Equal(t, []bool{true, false}, multiCalls) -} - -func TestReadMultiLinesAccumFromReaderSingleLine(t *testing.T) { - // First line ends accumulation; setMulti is never called. - read := linesReader([]string{"select 1;"}) - multiCalls := []bool{} - pred := func(line, accumulated string) bool { return false } - out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) - assert.NoError(t, err) - assert.Equal(t, "select 1;", out) - assert.Empty(t, multiCalls) -} - -func TestReadMultiLinesAccumFromReaderEmpty(t *testing.T) { - // First line is empty and predicate returns false: shell never enters - // multi-mode; output is empty. This mirrors the empty-Enter path. - read := linesReader([]string{""}) - multiCalls := []bool{} - pred := func(line, accumulated string) bool { return false } - out, err := readMultiLinesAccumFromReader(read, func(b bool) { multiCalls = append(multiCalls, b) }, pred) - assert.NoError(t, err) - assert.Equal(t, "", out) - assert.Empty(t, multiCalls) -} - -func TestReadMultiLinesAccumFromReaderEOFMidStream(t *testing.T) { - // EOF arriving after the predicate has chosen to keep reading surfaces - // the EOF error and returns the buffer collected so far. The trailing - // "\n" is the inter-line separator written by fmt.Fprintln before the - // next readLine call returned EOF. - read := linesReader([]string{"select 1;"}) - pred := func(line, accumulated string) bool { return true } - out, err := readMultiLinesAccumFromReader(read, func(b bool) {}, pred) - assert.Equal(t, io.EOF, err) - assert.Equal(t, "select 1;\n", out) -} - From 4bf4a1cafc9543bb856f9f16888a4f63687376df Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Tue, 30 Jun 2026 23:18:57 -0700 Subject: [PATCH 4/6] refactor(uninterpreted): scan shell input in a single streaming pass Replace per-line readMultiLinesFunc predicate loop with a single streaming scan of uninterpreted-shell input. Before, each new line re-evaluated a completeness predicate over the whole accumulated buffer, and the consumer re-scanned the assembled command, an O(n^2) re-scan plus a redundant second scan. Relocate the SQL statement scanner into ishell as StreamScanner and add a line-at-a-time to io.Reader adapter so the scanner consumes terminal input directly, blocking for continuation lines only while mid-statement, and stopping at a special terminator. Parsed statements are stashed on the shell and delivered to handlers via the new Context .Statements field, so consumers execute them without scanning again. - Add StreamScanner: per-byte scan of quotes, comments, escapes, and multi-byte DELIMITER; relocated from dolt - Add uninterpretedReader: adapts shell's line reader into an io.Reader, strips \g and \G terminators from the scan feed while preserving them in Raw, and switches to the continuation prompt for pulled lines - Add readUninterpretedCommand: resolve line-level cases (empty, DELIMITER, --, quit keywords, backslash commands) then scan the rest - Add Context.Statements to deliver the pre-scanned statements to handlers - IgnoreDelimiterStatements avoids a blocking lookahead read on short complete statements - Add unit tests for the scanner and the reader adapter Refs: #10866 --- context.go | 6 + ishell.go | 108 ++++++--- statement_scanner.go | 450 +++++++++++++++++++++++++++++++++++ statement_scanner_test.go | 360 ++++++++++++++++++++++++++++ uninterpreted_reader.go | 96 ++++++++ uninterpreted_reader_test.go | 124 ++++++++++ 6 files changed, 1114 insertions(+), 30 deletions(-) create mode 100644 statement_scanner.go create mode 100644 statement_scanner_test.go create mode 100644 uninterpreted_reader.go create mode 100644 uninterpreted_reader_test.go diff --git a/context.go b/context.go index 94df6b0..2795d8d 100644 --- a/context.go +++ b/context.go @@ -12,6 +12,12 @@ type Context struct { // RawArgs is unprocessed command arguments. RawArgs []string + // Statements holds the complete statements parsed from an uninterpreted + // command by the configured statement scanner, in order. It is empty for + // non-statement input (e.g., slash commands, DELIMITER lines, empty input) and + // for interpreted (non-uninterpreted) shells. + Statements []string + // Cmd is the currently executing command. This is empty for NotFound and Interrupt. Cmd Cmd diff --git a/ishell.go b/ishell.go index 103cfe3..ebc4ddf 100644 --- a/ishell.go +++ b/ishell.go @@ -57,6 +57,7 @@ type Shell struct { historyFile string autoHelp bool rawArgs []string + statements []string progressBar ProgressBar pager string pagerArgs []string @@ -361,40 +362,12 @@ func (s *Shell) readLine() (line string, err error) { func (s *Shell) readUninterpreted() (string, error) { s.rawArgs = nil + s.statements = nil var lines string var err error if s.lineTerminator != "" { - firstLine := true - lines, err = s.readMultiLinesFunc(func(line, accumulated string) (keepReading bool) { - if firstLine { - firstLine = false - if matches := delimiterRegex.FindStringSubmatch(line); len(matches) == 2 { - s.lineTerminator = matches[1] - return false - } - if strings.HasPrefix(line, "--") { - return false - } - for _, keyword := range s.quitKeywords { - if strings.TrimSpace(line) == keyword { - return false - } - } - } - trimmed := strings.TrimSpace(line) - for _, sc := range s.specialTerminators { - if strings.HasSuffix(trimmed, sc) { - return false - } - } - for _, sc := range s.backSlashCmds { - if strings.HasPrefix(trimmed, sc) { - return false - } - } - return !strings.HasSuffix(trimmed, s.lineTerminator) - }) + lines, err = s.readUninterpretedCommand() if err != nil { return "", err } @@ -432,6 +405,80 @@ func (s *Shell) readUninterpreted() (string, error) { return lines, nil } +// readUninterpretedCommand reads one complete command from a delimiter-based +// uninterpreted shell using a single streaming scan. It reads the first line, +// handles the line-level cases that produce no statements (empty input, +// DELIMITER, leading "--", quit keywords, backslash commands), and otherwise +// feeds the input through one StreamScanner to collect the command's +// statements. The statements are stashed in s.statements for delivery on the +// Context; the returned string is the raw command text (lines joined by "\n"). +func (s *Shell) readUninterpretedCommand() (string, error) { + firstLine, err := s.readLine() + if err != nil { + return firstLine, err + } + + trimmed := strings.TrimSpace(firstLine) + + if trimmed == "" { + return firstLine, nil + } + + if matches := delimiterRegex.FindStringSubmatch(firstLine); len(matches) == 2 { + s.lineTerminator = matches[1] + return firstLine, nil + } + + if strings.HasPrefix(firstLine, "--") { + return firstLine, nil + } + + for _, keyword := range s.quitKeywords { + if trimmed == keyword { + return firstLine, nil + } + } + + for _, sc := range s.backSlashCmds { + if strings.HasPrefix(trimmed, sc) { + return firstLine, nil + } + } + + // Scan the input (possibly multi-line) into statements with a single streaming pass. + // The reader blocks for continuation lines while the scanner is mid-statement, and + // stops at a special terminator (\g/\G). + lr := newUninterpretedReader(firstLine, s.readLine, s.reader.setMultiMode, s.specialTerminators) + scanner := NewStreamScannerWithDelimiter(lr, s.lineTerminator) + // DELIMITER is handled at the line level above; disable in-stream detection + // so a short complete statement does not block on the lookahead read. + scanner.IgnoreDelimiterStatements() + var statements []string + for scanner.Scan() { + if t := scanner.Text(); strings.TrimSpace(t) != "" { + statements = append(statements, t) + } + // The command is complete once the scanner is at a clean boundary with + // no further buffered input; stop before triggering a blocking read. + if !scanner.InsideQuote() && !scanner.InsideBlockComment() && !scanner.HasBufferedToken() { + break + } + } + // Revert to the primary prompt for the next command. + s.reader.setMultiMode(false) + if lr.aborted { + // EOF (e.g. Ctrl-D) arrived at the continuation prompt before the command + // was terminated. Discard the partial statement and surface EOF so the + // shell handles it as end-of-input rather than executing the fragment. + s.statements = nil + return "", io.EOF + } + // DELIMITER is handled at the line level above; the scanner's delimiter is + // unchanged here, so there is nothing to sync back. + s.statements = statements + return lr.Raw(), scanner.Err() +} + func (s *Shell) read() ([]string, error) { s.rawArgs = nil eof := "" @@ -824,6 +871,7 @@ func newContext(s *Shell, cmd *Cmd, args []string) *Context { progressBar: copyShellProgressBar(s), Args: args, RawArgs: s.rawArgs, + Statements: s.statements, Cmd: *cmd, contextValues: func() contextValues { values := contextValues{} diff --git a/statement_scanner.go b/statement_scanner.go new file mode 100644 index 0000000..2229006 --- /dev/null +++ b/statement_scanner.go @@ -0,0 +1,450 @@ +// Copyright 2020 Dolthub, Inc. +// +// 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 ishell + +import ( + "bytes" + "fmt" + "io" + "unicode" +) + +const maxStatementBufferBytes = 100*1024*1024 + 4096 +const pageSize = 2 << 11 + +const ( + sQuote byte = '\'' + dQuote = '"' + backslash = '\\' + backtick = '`' + hyphen = '-' + asterisk = '*' + slash = '/' + newline = '\n' +) + +const delimPrefixLen = 10 + +var delimPrefix = []byte("delimiter ") + +// StreamScanner is an iterator that reads bytes from |inp| until either +// (1) we match a DELIMITER statement, (2) we match the |delimiter| token, +// or (3) we EOF the file. After each Scan() call, the valid token will +// span from the buffer beginning to |state.end|. +type StreamScanner struct { + inp io.Reader + err error + state *qState + buf []byte + delimiter []byte + maxSize int + i int // current byte pointer + fill int + lineNum int + isEOF bool + // endedAtEOF reports whether the last Scan() ended at EOF before the delimiter. + endedAtEOF bool + // ignoreDelimiterStatements disables in-stream "DELIMITER x" detection. The + // interactive shell sets this because it handles DELIMITER at the line level, + // and the speculative lookahead read DELIMITER detection performs would block + // a streaming reader on short complete statements. + ignoreDelimiterStatements bool +} + +// NewStreamScanner returns a new StreamScanner that splits on ";". +func NewStreamScanner(r io.Reader) *StreamScanner { + return NewStreamScannerWithDelimiter(r, ";") +} + +// NewStreamScannerWithDelimiter returns a new StreamScanner that splits on +// |delimiter| (defaulting to ";" when empty). +func NewStreamScannerWithDelimiter(r io.Reader, delimiter string) *StreamScanner { + if delimiter == "" { + delimiter = ";" + } + return &StreamScanner{inp: r, buf: make([]byte, pageSize), maxSize: maxStatementBufferBytes, delimiter: []byte(delimiter), state: new(qState)} +} + +type qState struct { + start int + end int // token end, usually i - len(delimiter) + numConsecutiveBackslashes int // the number of consecutive backslashes encountered + numConsecutiveDelimiterMatches int // the consecutive number of characters that have been matched to the delimiter + statementStartLine int + lineCommentStart int + quoteChar byte // the opening quote character of the current quote being parsed, or 0 if the current parse location isn't inside a quoted string + lastChar byte // the last character parsed + ignoreNextChar bool // whether to ignore the next character + seenNonWhitespaceChar bool // whether we have encountered a non-whitespace character since we returned the last token + insideBlockComment bool + insideLineComment bool +} + +func (qs qState) insideComment() bool { + return qs.insideLineComment || qs.insideBlockComment +} + +func (qs qState) insideQuote() bool { + return qs.quoteChar != 0 +} + +// ignoreDelimiters returns if delimiters should be ignored. If inside a comment or a quote, delimiters, including +// comment delimiters, should be ignored +func (qs qState) ignoreDelimiters() bool { + return qs.insideComment() || qs.insideQuote() +} + +func (s *StreamScanner) Scan() bool { + s.resetState() + s.endedAtEOF = false + + if s.i >= s.fill { + // initialize buffer + if err := s.read(); err != nil { + s.err = err + return false + } + } + + if s.isEOF || s.i == s.fill { + // no token + return false + } + + // discard leading whitespace + if !s.skipWhitespace() { + return false + } + s.truncate() + + s.state.statementStartLine = s.lineNum + 1 + + if !s.ignoreDelimiterStatements { + if err, ok := s.isDelimiterExpr(); err != nil { + s.err = err + return false + } else if ok { + // empty token acks DELIMITER + return true + } + } + + for { + if err, ok := s.seekDelimiter(); err != nil { + s.err = err + return false + } else if ok { + // delimiter found, scanner holds valid token state + return true + } else if s.isEOF && s.i == s.fill { + // token terminates with file + s.state.end = s.fill + s.endedAtEOF = true + return true + } + // haven't found delimiter yet, keep reading + if err := s.read(); err != nil { + s.err = err + return false + } + } +} + +func (s *StreamScanner) skipWhitespace() bool { + for { + if s.i >= s.fill { + if err := s.read(); err != nil { + s.err = err + return false + } + } + if s.isEOF { + return true + } + if !unicode.IsSpace(rune(s.buf[s.i])) { + break + } + if s.buf[s.i] == '\n' { + s.lineNum++ + } + s.i++ + } + return true +} + +func (s *StreamScanner) truncate() { + // copy size should be 4k or less + s.state.start = s.i + s.state.end = s.i +} + +func (s *StreamScanner) resetState() { + s.state = &qState{} +} + +func (s *StreamScanner) read() error { + if s.fill >= s.maxSize { + // if script exceeds buffer that's OK, if + // a single query exceeds buffer that's not OK + if s.state.start == 0 { + return fmt.Errorf("exceeded max query size") + } + // discard previous queries, resulting buffer will start + // at the current |start| + s.fill -= s.state.start + s.i -= s.state.start + s.state.end = s.state.start + copy(s.buf[:], s.buf[s.state.start:]) + s.state.start = 0 + return s.read() + } + if s.fill == len(s.buf) { + newBufSize := len(s.buf) * 2 + if newBufSize > s.maxSize { + newBufSize = s.maxSize + } + newBuf := make([]byte, newBufSize) + copy(newBuf, s.buf) + s.buf = newBuf + } + n, err := s.inp.Read(s.buf[s.fill:]) + if err == io.EOF { + s.isEOF = true + } else if err != nil { + return err + } + s.fill += n + return nil +} + +func (s *StreamScanner) Err() error { + return s.err +} + +func (s *StreamScanner) Bytes() []byte { + return s.buf[s.state.start:s.state.end] +} + +// Text returns the most recent token generated by a call to [Scanner.Scan] +// as a newly allocated string holding its bytes. +func (s *StreamScanner) Text() string { + return string(s.Bytes()) +} + +// StatementStartLine returns the 1-based line number on which the most recent +// token returned by Scan() began. +func (s *StreamScanner) StatementStartLine() int { + return s.state.statementStartLine +} + +func (s *StreamScanner) isDelimiterExpr() (error, bool) { + if s.i == 0 && s.fill-s.i < delimPrefixLen { + // need to see first |delimPrefixLen| characters + if err := s.read(); err != nil { + s.err = err + return err, false + } + } + + // valid delimiter state machine check + // "DELIMITER " -> 0+ spaces -> -> 1 space + if s.fill-s.i >= delimPrefixLen && bytes.EqualFold(s.buf[s.i:s.i+delimPrefixLen], delimPrefix) { + delimTokenIdx := s.i + s.i += delimPrefixLen + if !s.skipWhitespace() { + return nil, false + } + if s.isEOF { + // invalid delimiter + s.i = delimTokenIdx + return nil, false + } + delimStart := s.i + for ; !s.isEOF && !unicode.IsSpace(rune(s.buf[s.i])); s.i++ { + if s.i >= s.fill { + if err := s.read(); err != nil { + s.err = err + return err, false + } + } + } + delimEnd := s.i + s.delimiter = make([]byte, delimEnd-delimStart) + copy(s.delimiter, s.buf[delimStart:delimEnd]) + + // discard delimiter token, return empty token + s.truncate() + return nil, true + } + return nil, false +} + +func (s *StreamScanner) seekDelimiter() (error, bool) { + for ; s.i < s.fill; s.i++ { + i := s.i + if !s.state.ignoreNextChar { + // this doesn't handle unicode characters correctly and will break on some things, but it's only used for line + // number reporting. + if !s.state.seenNonWhitespaceChar && !unicode.IsSpace(rune(s.buf[i])) { + s.state.seenNonWhitespaceChar = true + } + + // check if we've matched the delimiter string + if !s.state.ignoreDelimiters() && s.buf[i] == s.delimiter[s.state.numConsecutiveDelimiterMatches] { + s.state.numConsecutiveDelimiterMatches++ + if s.state.numConsecutiveDelimiterMatches == len(s.delimiter) { + s.state.end = s.i - len(s.delimiter) + 1 + s.i++ + s.state.lastChar = 0 + return nil, true + } + s.state.lastChar = s.buf[i] + continue + } else { + s.state.numConsecutiveDelimiterMatches = 0 + } + + switch s.buf[i] { + case newline: + s.lineNum++ + if s.state.insideLineComment { + s.state.insideLineComment = false + // if the entire statement is a line comment, truncate and return as empty + if s.state.start == s.state.lineCommentStart { + s.i++ + s.truncate() + s.state.lastChar = 0 + return nil, true + } + } + case hyphen: + // If inside quote or already inside comment, ignore. Otherwise, if previous character is also a hyphen, + // ie "--", begin line comment. + if !s.state.ignoreDelimiters() && s.state.lastChar == hyphen { + s.state.lineCommentStart = i - 1 + s.state.insideLineComment = true + } + case asterisk: + // If inside quote or already inside comment, ignore. Otherwise, if previous character is a slash, ie + // "/*", begin block comment. + if !s.state.ignoreDelimiters() && s.state.lastChar == slash { + s.state.insideBlockComment = true + } + case slash: + // If previous character is an asterisk, ie "*/", end block comment. + if s.state.insideBlockComment && s.state.lastChar == asterisk { + s.state.insideBlockComment = false + } + case backslash: + s.state.numConsecutiveBackslashes++ + case sQuote, dQuote, backtick: + // ignore quotes inside comments + if s.state.insideComment() { + break + } + + prevNumConsecutiveBackslashes := s.state.numConsecutiveBackslashes + s.state.numConsecutiveBackslashes = 0 + + // escaped quote character + if s.state.lastChar == backslash && prevNumConsecutiveBackslashes%2 == 1 { + break + } + + // currently in a quoted string + if s.state.insideQuote() { + if i+1 >= s.fill { + // require lookahead or EOF + if err := s.read(); err != nil { + return err, false + } + } + + // end quote or two consecutive quote characters (a form of escaping quote chars) + if s.state.quoteChar == s.buf[i] { + var nextChar byte = 0 + if i+1 < s.fill { + nextChar = s.buf[i+1] + } + + if nextChar == s.state.quoteChar { + // escaped quote. skip the next character + s.state.ignoreNextChar = true + } else { + // end quote + s.state.quoteChar = 0 + } + } + + // embedded quote ('"' or "'") + break + } + + // open quote + s.state.quoteChar = s.buf[i] + default: + s.state.numConsecutiveBackslashes = 0 + } + } else { + s.state.ignoreNextChar = false + } + + s.state.lastChar = s.buf[i] + } + return nil, false +} + +// InsideQuote reports whether the scanner is currently inside a quoted string. +func (s *StreamScanner) InsideQuote() bool { + return s.state.quoteChar != 0 +} + +// InsideBlockComment reports whether the scanner is currently inside an +// unclosed /* ... */ block comment. +func (s *StreamScanner) InsideBlockComment() bool { + return s.state.insideBlockComment +} + +// EndedAtEOF reports whether the last Scan() ended at EOF before the delimiter. +func (s *StreamScanner) EndedAtEOF() bool { + return s.endedAtEOF +} + +// HasBufferedToken reports whether non-whitespace bytes remain buffered after +// the most recent token, i.e. another statement is already available without +// reading more input. Used by the interactive shell to decide whether a +// command is complete without triggering a blocking read. +func (s *StreamScanner) HasBufferedToken() bool { + for j := s.i; j < s.fill; j++ { + if !unicode.IsSpace(rune(s.buf[j])) { + return true + } + } + return false +} + +// Delimiter returns the scanner's current statement delimiter, which may have +// changed from the initial value if a DELIMITER statement was scanned. +func (s *StreamScanner) Delimiter() string { + return string(s.delimiter) +} + +// IgnoreDelimiterStatements disables in-stream "DELIMITER x" detection. Callers +// that read from a blocking, line-at-a-time source (the interactive shell) set +// this so a short complete statement does not block on the DELIMITER lookahead; +// such callers handle DELIMITER changes themselves. +func (s *StreamScanner) IgnoreDelimiterStatements() { + s.ignoreDelimiterStatements = true +} diff --git a/statement_scanner_test.go b/statement_scanner_test.go new file mode 100644 index 0000000..6466194 --- /dev/null +++ b/statement_scanner_test.go @@ -0,0 +1,360 @@ +// Copyright 2020 Dolthub, Inc. +// +// 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 ishell + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestScanStatements(t *testing.T) { + type testcase struct { + input string + statements []string + lineNums []int + } + + // Some of these include malformed input (e.g. strings that aren't properly terminated) + testcases := []testcase{ + { + input: `insert into foo values (";;';'");`, + statements: []string{ + `insert into foo values (";;';'")`, + }, + }, + { + input: `select ''';;'; select ";\;"`, + statements: []string{ + `select ''';;'`, + `select ";\;"`, + }, + }, + { + input: `select ''';;'; select ";\;`, + statements: []string{ + `select ''';;'`, + `select ";\;`, + }, + }, + { + input: `select ''';;'; select ";\; +;`, + statements: []string{ + `select ''';;'`, + `select ";\; +;`, + }, + }, + { + input: `select '\\'''; select '";";'; select 1`, + statements: []string{ + `select '\\'''`, + `select '";";'`, + `select 1`, + }, + }, + { + input: `select '\\''; select '";";'; select 1`, + statements: []string{ + `select '\\''; select '";"`, + `'; select 1`, + }, + }, + { + input: `insert into foo values(''); select 1`, + statements: []string{ + `insert into foo values('')`, + `select 1`, + }, + }, + { + input: `insert into foo values('''); select 1`, + statements: []string{ + `insert into foo values('''); select 1`, + }, + }, + { + input: `insert into foo values(''''); select 1`, + statements: []string{ + `insert into foo values('''')`, + `select 1`, + }, + }, + { + input: `insert into foo values(""); select 1`, + statements: []string{ + `insert into foo values("")`, + `select 1`, + }, + }, + { + input: `insert into foo values("""); select 1`, + statements: []string{ + `insert into foo values("""); select 1`, + }, + }, + { + input: `insert into foo values(""""); select 1`, + statements: []string{ + `insert into foo values("""")`, + `select 1`, + }, + }, + { + input: `select '\''; select "hell\"o"`, + statements: []string{ + `select '\''`, + `select "hell\"o"`, + }, + }, + { + input: `select * from foo; select baz from foo; +select +a from b; select 1`, + statements: []string{ + "select * from foo", + "select baz from foo", + "select\na from b", + "select 1", + }, + lineNums: []int{ + 1, 1, 2, 3, + }, + }, + { + input: "create table dumb (`hell\\`o;` int primary key);", + statements: []string{ + "create table dumb (`hell\\`o;` int primary key)", + }, + }, + { + input: "create table dumb (`hell``o;` int primary key); select \n" + + "baz from foo;\n" + + "\n" + + "select\n" + + "a from b; select 1\n\n", + statements: []string{ + "create table dumb (`hell``o;` int primary key)", + "select \nbaz from foo", + "select\na from b", + "select 1", + }, + lineNums: []int{ + 1, 1, 4, 5, + }, + }, + { + input: `insert into foo values ('a', "b;", 'c;;"" +'); update foo set baz = bar, +qux = '"hello"""' where xyzzy = ";;';'"; + + +create table foo (a int not null default ';', +primary key (a));`, + statements: []string{ + `insert into foo values ('a', "b;", 'c;;"" +')`, + `update foo set baz = bar, +qux = '"hello"""' where xyzzy = ";;';'"`, + `create table foo (a int not null default ';', +primary key (a))`, + }, + lineNums: []int{ + 1, 2, 6, + }, + }, + { + input: `DELIMITER | +insert into foo values (1,2,3)|`, + statements: []string{ + "", + "insert into foo values (1,2,3)", + }, + lineNums: []int{1, 2}, + }, + { + // https://github.com/dolthub/dolt/issues/10828 + input: `-- comment asdfasdf + delimiter // + select current_user() //`, + statements: []string{ + "", + "", + "select current_user()", + }, + lineNums: []int{1, 2, 3}, + }, + { + // https://github.com/dolthub/dolt/issues/8495 + input: strings.Repeat(" ", 4096) + `insert into foo values (1,2,3)`, + statements: []string{ + "insert into foo values (1,2,3)", + }, + lineNums: []int{1, 2}, + }, + { + input: "DELIMITER" + strings.Repeat(" ", 4096) + `| +insert into foo values (1,2,3)|`, + statements: []string{ + "", + "insert into foo values (1,2,3)", + }, + lineNums: []int{1, 2}, + }, + { + // https://github.com/dolthub/dolt/issues/10694 + input: `-- ' +-- can have intermediate comments +CALL dolt_commit('-m', 'message', '--allow-empty'); +CALL dolt_checkout('main');`, + statements: []string{ + "", "", + "CALL dolt_commit('-m', 'message', '--allow-empty')", + "CALL dolt_checkout('main')", + }, + lineNums: []int{1, 2, 3, 4}, + }, + { + input: `/* block comment with lone quote ' +*/ +-- can have intermediate comments +CALL dolt_commit('-m', 'message', '--allow-empty'); +CALL dolt_checkout('main');`, + statements: []string{ + `/* block comment with lone quote ' +*/ +-- can have intermediate comments +CALL dolt_commit('-m', 'message', '--allow-empty')`, + "CALL dolt_checkout('main')", + }, + lineNums: []int{1, 5}, + }, + { + input: `select * /* -- ignore line comment inside block comment */ from xy; +select x from xy; -- select y from xy; +select * /* ignore multi-line comment with ; +comment; +comment; +*/ from foo; +select '-- ignore line comment +in quote';`, + statements: []string{ + "select * /* -- ignore line comment inside block comment */ from xy", + "select x from xy", + "", + `select * /* ignore multi-line comment with ; +comment; +comment; +*/ from foo`, + `select '-- ignore line comment +in quote'`, + }, + lineNums: []int{1, 2, 2, 3, 7}, + }, + } + + for _, tt := range testcases { + t.Run(tt.input, func(t *testing.T) { + reader := strings.NewReader(tt.input) + scanner := NewStreamScanner(reader) + var i int + for scanner.Scan() { + require.True(t, i < len(tt.statements)) + assert.Equal(t, tt.statements[i], strings.TrimSpace(scanner.Text())) + if tt.lineNums != nil { + assert.Equal(t, tt.lineNums[i], scanner.StatementStartLine()) + } else { + assert.Equal(t, 1, scanner.StatementStartLine()) + } + i++ + } + + require.NoError(t, scanner.Err()) + }) + } +} + +// TestEndedAtEOFFlag covers the per-Scan termination distinction that the +// shell completion check relies on. A delimiter-terminated statement must +// leave EndedAtEOF false; an unterminated trailing statement must leave it +// true. +func TestEndedAtEOFFlag(t *testing.T) { + t.Run("delimiter terminated", func(t *testing.T) { + scanner := NewStreamScanner(strings.NewReader("select 1;")) + require.True(t, scanner.Scan()) + assert.False(t, scanner.EndedAtEOF()) + assert.False(t, scanner.Scan()) + }) + t.Run("unterminated EOF", func(t *testing.T) { + scanner := NewStreamScanner(strings.NewReader("select 1")) + require.True(t, scanner.Scan()) + assert.True(t, scanner.EndedAtEOF()) + assert.False(t, scanner.Scan()) + }) + t.Run("flag resets across scans", func(t *testing.T) { + // First statement terminates; second hits EOF unterminated. + scanner := NewStreamScanner(strings.NewReader("select 1; select 2")) + require.True(t, scanner.Scan()) + assert.False(t, scanner.EndedAtEOF()) + require.True(t, scanner.Scan()) + assert.True(t, scanner.EndedAtEOF()) + }) +} + +// TestHasBufferedToken verifies the accessor the interactive shell uses to +// decide a command is complete without triggering a blocking read. +func TestHasBufferedToken(t *testing.T) { + scanner := NewStreamScanner(strings.NewReader("select 1; select 2;")) + require.True(t, scanner.Scan()) + // "select 2;" is still buffered after the first statement. + assert.True(t, scanner.HasBufferedToken()) + require.True(t, scanner.Scan()) + // Only trailing (no) content remains. + assert.False(t, scanner.HasBufferedToken()) +} + +// TestDelimiterAccessor checks the current delimiter is reported, and that it +// follows a DELIMITER statement when in-stream detection is enabled. +func TestDelimiterAccessor(t *testing.T) { + assert.Equal(t, "//", NewStreamScannerWithDelimiter(strings.NewReader(""), "//").Delimiter()) + + scanner := NewStreamScanner(strings.NewReader("DELIMITER |\nselect 1|")) + require.True(t, scanner.Scan()) // empty token acks DELIMITER + assert.Equal(t, "|", scanner.Delimiter()) +} + +// TestIgnoreDelimiterStatements verifies that disabling in-stream DELIMITER +// detection makes the scanner treat "DELIMITER x" as ordinary statement text +// rather than a delimiter change. +func TestIgnoreDelimiterStatements(t *testing.T) { + // With detection (default): the DELIMITER is processed and the delimiter + // becomes "|", so "select 1|" is its own statement. + on := NewStreamScanner(strings.NewReader("DELIMITER |\nselect 1|")) + require.True(t, on.Scan()) + assert.Equal(t, "", strings.TrimSpace(on.Text())) // empty DELIMITER ack + require.True(t, on.Scan()) + assert.Equal(t, "select 1", strings.TrimSpace(on.Text())) + + // With detection disabled: no DELIMITER processing; with the default ";" + // delimiter and no ";" present, the whole input is one statement. + off := NewStreamScanner(strings.NewReader("DELIMITER |\nselect 1|")) + off.IgnoreDelimiterStatements() + require.True(t, off.Scan()) + assert.Equal(t, "DELIMITER |\nselect 1|", off.Text()) + assert.Equal(t, ";", off.Delimiter()) +} diff --git a/uninterpreted_reader.go b/uninterpreted_reader.go new file mode 100644 index 0000000..c2d95cf --- /dev/null +++ b/uninterpreted_reader.go @@ -0,0 +1,96 @@ +// Copyright 2026 Dolthub, Inc. +// +// 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 ishell + +import ( + "io" + "strings" +) + +// uninterpretedReader adapts a line-at-a-time line source into an io.Reader +// that a StreamScanner can consume one statement at a time. It hands back the +// already-read seed (first) line first, then pulls subsequent lines on demand +// via readLine, switching to the continuation prompt for those lines. If a line +// ends with one of |specials| (e.g. "\g", "\G"), that terminator is stripped +// from the bytes fed to the scanner and the reader reports io.EOF once drained, +// so the command completes at that line. Raw returns the original lines +// (terminators intact) joined by "\n". +type uninterpretedReader struct { + readLine func() (string, error) + setMulti func(bool) + specials []string + + seed string + seedUsed bool + pending []byte + rawLines []string + done bool // a special terminator ended the command + err error // a deferred read error to surface after pending drains +} + +func newUninterpretedReader(seed string, readLine func() (string, error), setMulti func(bool), specials []string) *uninterpretedReader { + return &uninterpretedReader{readLine: readLine, setMulti: setMulti, specials: specials, seed: seed} +} + +func (r *uninterpretedReader) nextLine() (string, error) { + if !r.seedUsed { + r.seedUsed = true + return r.seed, nil + } + // every line after the first is a continuation line + r.setMulti(true) + return r.readLine() +} + +func (r *uninterpretedReader) Read(p []byte) (int, error) { + if len(r.pending) == 0 { + // Surface a deferred read error before a special-terminator EOF, so a + // real error is not lost when a line both errors and ends in \g/\G. + if r.err != nil { + return 0, r.err + } + if r.done { + return 0, io.EOF + } + line, err := r.nextLine() + if err != nil { + if line == "" { + return 0, err + } + // surface the error after the line's bytes are delivered + r.err = err + } + r.rawLines = append(r.rawLines, line) + feed := line + trimmed := strings.TrimRight(line, " \t") + for _, sc := range r.specials { + if strings.HasSuffix(trimmed, sc) { + feed = strings.TrimSuffix(trimmed, sc) + r.done = true + break + } + } + r.pending = append([]byte(feed), '\n') + } + n := copy(p, r.pending) + r.pending = r.pending[n:] + return n, nil +} + +// Raw returns the original input lines (with any special terminators intact) +// joined by "\n", matching the buffer the line-accumulation path produced. +func (r *uninterpretedReader) Raw() string { + return strings.Join(r.rawLines, "\n") +} diff --git a/uninterpreted_reader_test.go b/uninterpreted_reader_test.go new file mode 100644 index 0000000..75e9b80 --- /dev/null +++ b/uninterpreted_reader_test.go @@ -0,0 +1,124 @@ +// Copyright 2026 Dolthub, Inc. +// +// 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 ishell + +import ( + "io" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// linesFunc returns a readLine func that yields the supplied lines, then io.EOF. +func linesFunc(lines ...string) func() (string, error) { + i := 0 + return func() (string, error) { + if i >= len(lines) { + return "", io.EOF + } + l := lines[i] + i++ + return l, nil + } +} + +// drain reads the reader to EOF and returns the bytes consumed. +func drain(t *testing.T, r io.Reader) string { + t.Helper() + b, err := io.ReadAll(r) + require.NoError(t, err) + return string(b) +} + +func TestUninterpretedReaderSingleLine(t *testing.T) { + // Seed line is returned first; readLine then EOFs. + r := newUninterpretedReader("select 1;", linesFunc(), func(bool) {}, nil) + assert.Equal(t, "select 1;\n", drain(t, r)) + assert.Equal(t, "select 1;", r.Raw()) +} + +func TestUninterpretedReaderContinuation(t *testing.T) { + // Seed plus a pulled continuation line. + r := newUninterpretedReader("select 1", linesFunc(";"), func(bool) {}, nil) + assert.Equal(t, "select 1\n;\n", drain(t, r)) + assert.Equal(t, "select 1\n;", r.Raw()) +} + +func TestUninterpretedReaderSeedServedAtPrimaryPrompt(t *testing.T) { + // The first read returns the seed line without switching to the multi-line + // prompt; the continuation prompt is only engaged when pulling later lines. + var multi []bool + r := newUninterpretedReader("select 1", linesFunc(";"), func(b bool) { multi = append(multi, b) }, nil) + buf := make([]byte, 64) + n, err := r.Read(buf) + require.NoError(t, err) + assert.Equal(t, "select 1\n", string(buf[:n])) + assert.Empty(t, multi) +} + +func TestUninterpretedReaderSpecialTerminatorStripped(t *testing.T) { + // A trailing special terminator is stripped from the scanner feed and ends + // the command; Raw keeps the original line. + r := newUninterpretedReader(`select 1\G`, linesFunc("should not be read"), func(bool) {}, []string{`\g`, `\G`}) + assert.Equal(t, "select 1\n", drain(t, r)) + assert.Equal(t, `select 1\G`, r.Raw()) +} + +func TestUninterpretedReaderSpecialTerminatorWithTrailingSpace(t *testing.T) { + r := newUninterpretedReader(`select 1 \G`, linesFunc(), func(bool) {}, []string{`\g`, `\G`}) + assert.Equal(t, "select 1 \n", drain(t, r)) + assert.Equal(t, `select 1 \G`, r.Raw()) +} + +func TestUninterpretedReaderSpecialTerminatorOnContinuation(t *testing.T) { + // Special terminator can arrive on a continuation line. + r := newUninterpretedReader("select 1", linesFunc(`\G`), func(bool) {}, []string{`\g`, `\G`}) + assert.Equal(t, "select 1\n\n", drain(t, r)) + assert.Equal(t, "select 1\n\\G", r.Raw()) +} + +func TestUninterpretedReaderEOFFromReadLine(t *testing.T) { + // EOF (Ctrl-D) from readLine on a continuation surfaces after the seed. + r := newUninterpretedReader("select 1", linesFunc(), func(bool) {}, nil) + got := drain(t, r) + assert.Equal(t, "select 1\n", got) +} + +func TestUninterpretedReaderAbortedOnContinuationEOF(t *testing.T) { + // Ctrl-D (EOF) at the continuation prompt, before any terminator completes + // the command, marks the read aborted so the caller discards the partial. + r := newUninterpretedReader("select 1", linesFunc(), func(bool) {}, nil) + drain(t, r) + assert.True(t, r.aborted) +} + +func TestUninterpretedReaderNotAbortedOnSpecialTerminator(t *testing.T) { + // A \g/\G terminator also ends the reader via EOF, but it is a completed + // statement, not an abort, so aborted stays false and the caller runs it. + r := newUninterpretedReader(`select 1\G`, linesFunc("should not be read"), func(bool) {}, []string{`\g`, `\G`}) + drain(t, r) + assert.False(t, r.aborted) +} + +func TestUninterpretedReaderErrorSurvivesSpecialTerminator(t *testing.T) { + // A non-EOF read error on a line that also ends in a special terminator must + // still surface; the deferred error takes precedence over the terminator EOF. + boom := io.ErrUnexpectedEOF + readLine := func() (string, error) { return `2\G`, boom } + r := newUninterpretedReader("select 1", readLine, func(bool) {}, []string{`\g`, `\G`}) + _, err := io.ReadAll(r) + assert.Equal(t, boom, err) +} From 8cd80da5f6ff60a5eff3b27abef0700e095182de Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Wed, 1 Jul 2026 00:08:09 -0700 Subject: [PATCH 5/6] fix(uninterpreted): discard partial statement on Ctrl-D Ctrl-D at the continuation prompt was executing whatever had been typed so far instead of cancelling it. The new uninterpretedReader feeds a live, blocking readLine directly into the StreamScanner, so when readLine returns EOF mid-statement the scanner treated the buffered bytes as a normal EOF-terminated token and ran it. A user that aborts a half typed statement (e.g. an unterminated DELETE) would have it executed instead of cancelled, which diverges from the previous line-accumulation behavior and the mysql client. Add an aborted flag to uninterpretedReader, and set only when a genuine EOF (not a \g and \G special terminator, which also ends the reader via EOF) is reached while pulling a continuation line. When the flag is set, readUninterpretedCommand discards the collected statements and returns io.EOF so the shell's run loop routes to its EOF handler instead of executing the fragment. - Add uninterpretedReader.aborted: set in Read on a genuine EOF from readLine while continuing a command - Check it in readUninterpretedCommand: clear s.statements and return io.EOF instead of the partial command - Add TestUninterpretedReaderAbortedOnContinuationEOF and TestUninterpretedReaderNotAbortedOnSpecialTerminator to pin the discard-on-Ctrl-D behavior and its \g and \G exception Refs: dolthub/dolt#10866 --- uninterpreted_reader.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/uninterpreted_reader.go b/uninterpreted_reader.go index c2d95cf..e1559a4 100644 --- a/uninterpreted_reader.go +++ b/uninterpreted_reader.go @@ -37,6 +37,7 @@ type uninterpretedReader struct { pending []byte rawLines []string done bool // a special terminator ended the command + aborted bool // genuine EOF (e.g. Ctrl-D) reached before the command completed err error // a deferred read error to surface after pending drains } @@ -66,6 +67,12 @@ func (r *uninterpretedReader) Read(p []byte) (int, error) { } line, err := r.nextLine() if err != nil { + if err == io.EOF { + // Genuine end of input (e.g. Ctrl-D) reached while pulling a + // continuation line, before any delimiter or special terminator + // completed the command. The caller discards the partial input. + r.aborted = true + } if line == "" { return 0, err } From 950b93b95974e7616eb5f3ff82a1886372c1d73f Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:29:57 -0700 Subject: [PATCH 6/6] refactor(uninterpreted): trim stored stmt text Trim each statement before collecting it into Context.Statements so that entries carry no trailing whitespace or newlines. The scan loop already computed strings.TrimSpace to skip whitespace-only tokens, but then stored the untrimmed token. Store the trimmed value instead. Refs: dolthub/dolt#10866 --- ishell.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ishell.go b/ishell.go index ebc4ddf..0e637df 100644 --- a/ishell.go +++ b/ishell.go @@ -455,7 +455,7 @@ func (s *Shell) readUninterpretedCommand() (string, error) { scanner.IgnoreDelimiterStatements() var statements []string for scanner.Scan() { - if t := scanner.Text(); strings.TrimSpace(t) != "" { + if t := strings.TrimSpace(scanner.Text()); t != "" { statements = append(statements, t) } // The command is complete once the scanner is at a clean boundary with