fix(REPL): Don't buffer lines until a trailing semicolon is found and add history hinting (#3630)

* fix(REPL): Don't buffer lines until a trailing semicolon is found

The repl would silently buffer all lines until a trailing semicolon were found which
resulted in some very confusing error messages as I would input invalid commands followed
by a command I thought were valid, except I'd still get an error due to the previous command being buffered.

This uses rustyline's helper feature to detect incomplete input (no trailing semicolon) and makes
it accept multiline input until the input is completed.

I also included some of rustyline's default hint and highlighting while I was at it.

* chore: cargo clippy

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Cargo.lock

@@ -1752,6 +1752,7 @@ dependencies = [
 name = "influxdb_iox"
 version = "0.1.0"
 dependencies = [
+ "ansi_term",
  "arrow",
  "arrow-flight",
  "arrow_util",

influxdb_iox/Cargo.toml

@@ -46,6 +46,7 @@ trogging = { path = "../trogging", default-features = false, features = ["clap"]
 write_buffer = { path = "../write_buffer" }
 
 # Crates.io dependencies, in alphabetical order
+ansi_term = "0.12"
 arrow = { version = "8.0", features = ["prettyprint"] }
 arrow-flight = "8.0"
 async-trait = "0.1"

influxdb_iox/src/commands/sql/repl.rs

@@ -1,4 +1,4 @@
-use std::{convert::TryInto, path::PathBuf, sync::Arc, time::Instant};
+use std::{borrow::Cow, convert::TryInto, path::PathBuf, sync::Arc, time::Instant};
 
 use arrow::{
     array::{ArrayRef, StringArray},
@@ -74,11 +74,95 @@ enum QueryEngine {
     Observer(super::observer::Observer),
 }
 
+struct RustylineHelper {
+    hinter: rustyline::hint::HistoryHinter,
+    highlighter: rustyline::highlight::MatchingBracketHighlighter,
+}
+
+impl Default for RustylineHelper {
+    fn default() -> Self {
+        Self {
+            hinter: rustyline::hint::HistoryHinter {},
+            highlighter: rustyline::highlight::MatchingBracketHighlighter::default(),
+        }
+    }
+}
+
+impl rustyline::Helper for RustylineHelper {}
+
+impl rustyline::validate::Validator for RustylineHelper {
+    fn validate(
+        &self,
+        ctx: &mut rustyline::validate::ValidationContext<'_>,
+    ) -> rustyline::Result<rustyline::validate::ValidationResult> {
+        let input = ctx.input();
+
+        if input.trim_end().ends_with(';') {
+            match ReplCommand::try_from(input) {
+                Ok(_) => Ok(rustyline::validate::ValidationResult::Valid(None)),
+                Err(err) => Ok(rustyline::validate::ValidationResult::Invalid(Some(err))),
+            }
+        } else {
+            Ok(rustyline::validate::ValidationResult::Incomplete)
+        }
+    }
+}
+
+impl rustyline::hint::Hinter for RustylineHelper {
+    type Hint = String;
+    fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option<String> {
+        self.hinter.hint(line, pos, ctx)
+    }
+}
+
+impl rustyline::highlight::Highlighter for RustylineHelper {
+    fn highlight<'l>(&self, line: &'l str, pos: usize) -> Cow<'l, str> {
+        self.highlighter.highlight(line, pos)
+    }
+
+    fn highlight_prompt<'b, 's: 'b, 'p: 'b>(
+        &'s self,
+        prompt: &'p str,
+        default: bool,
+    ) -> Cow<'b, str> {
+        self.highlighter.highlight_prompt(prompt, default)
+    }
+
+    fn highlight_hint<'h>(&self, hint: &'h str) -> Cow<'h, str> {
+        // TODO Detect when windows supports ANSI escapes
+        #[cfg(windows)]
+        {
+            Cow::Borrowed(hint)
+        }
+        #[cfg(not(windows))]
+        {
+            use ansi_term::Style;
+            Cow::Owned(Style::new().dimmed().paint(hint).to_string())
+        }
+    }
+
+    fn highlight_candidate<'c>(
+        &self,
+        candidate: &'c str,
+        completion: rustyline::CompletionType,
+    ) -> Cow<'c, str> {
+        self.highlighter.highlight_candidate(candidate, completion)
+    }
+
+    fn highlight_char(&self, line: &str, pos: usize) -> bool {
+        self.highlighter.highlight_char(line, pos)
+    }
+}
+
+impl rustyline::completion::Completer for RustylineHelper {
+    type Candidate = String;
+}
+
 /// Captures the state of the repl, gathers commands and executes them
 /// one by one
 pub struct Repl {
     /// Rustyline editor for interacting with user on command line
-    rl: Editor<()>,
+    rl: Editor<RustylineHelper>,
 
     /// Current prompt
     prompt: String,
@@ -109,7 +193,8 @@ impl Repl {
         let management_client = influxdb_iox_client::management::Client::new(connection.clone());
         let flight_client = influxdb_iox_client::flight::Client::new(connection.clone());
 
-        let mut rl = Editor::<()>::new();
+        let mut rl = Editor::new();
+        rl.set_helper(Some(RustylineHelper::default()));
         let history_file = history_file();
 
         if let Err(e) = rl.load_history(&history_file) {
@@ -172,37 +257,26 @@ impl Repl {
 
     /// Parss the next command;
     fn next_command(&mut self) -> Result<ReplCommand> {
-        let mut request = "".to_owned();
+        match self.rl.readline(&self.prompt) {
-        loop {
+            Ok(ref line) if is_exit_command(line) => Ok(ReplCommand::Exit),
-            match self.rl.readline(&self.prompt) {
+            Ok(ref line) => {
-                Ok(ref line) if is_exit_command(line) && request.is_empty() => {
+                let request = line.trim_end();
-                    return Ok(ReplCommand::Exit);
+                self.rl.add_history_entry(request.to_owned());
-                }
-                Ok(ref line) if line.trim_end().ends_with(';') => {
-                    request.push_str(line.trim_end());
-                    self.rl.add_history_entry(request.clone());
 
-                    return request
+                request
-                        .try_into()
+                    .try_into()
-                        .map_err(|message| Error::ParsingCommand { message });
+                    .map_err(|message| Error::ParsingCommand { message })
-                }
-                Ok(ref line) => {
-                    request.push_str(line);
-                    request.push(' ');
-                }
-                Err(ReadlineError::Eof) => {
-                    debug!("Received Ctrl-D");
-                    return Ok(ReplCommand::Exit);
-                }
-                Err(ReadlineError::Interrupted) => {
-                    debug!("Received Ctrl-C");
-                    return Ok(ReplCommand::Exit);
-                }
-                // Some sort of real underlying error
-                Err(e) => {
-                    return Err(Error::Readline { source: e });
-                }
             }
+            Err(ReadlineError::Eof) => {
+                debug!("Received Ctrl-D");
+                Ok(ReplCommand::Exit)
+            }
+            Err(ReadlineError::Interrupted) => {
+                debug!("Received Ctrl-C");
+                Ok(ReplCommand::Exit)
+            }
+            // Some sort of real underlying error
+            Err(e) => Err(Error::Readline { source: e }),
         }
     }
 

influxdb_iox/src/commands/sql/repl_command.rs

@@ -1,5 +1,3 @@
-use std::convert::TryInto;
-
 use observability_deps::tracing::{debug, warn};
 
 /// Represents the parsed command from the user (which may be over many lines)
@@ -14,23 +12,31 @@ pub enum ReplCommand {
     Exit,
 }
 
-impl TryInto<ReplCommand> for String {
+impl TryFrom<String> for ReplCommand {
-    type Error = Self;
+    type Error = String;
+
+    fn try_from(input: String) -> Result<Self, Self::Error> {
+        Self::try_from(&input[..])
+    }
+}
+
+impl TryFrom<&str> for ReplCommand {
+    type Error = String;
 
     #[allow(clippy::if_same_then_else)]
-    fn try_into(self) -> Result<ReplCommand, Self::Error> {
+    fn try_from(input: &str) -> Result<Self, Self::Error> {
-        debug!(%self, "tokenizing to ReplCommand");
+        debug!(%input, "tokenizing to ReplCommand");
 
-        if self.trim().is_empty() {
+        if input.trim().is_empty() {
             return Err("No command specified".to_string());
         }
 
         // tokenized commands, normalized whitespace but original case
-        let raw_commands = self
+        let raw_commands = input
             .trim()
             // chop off trailing semicolon
             .strip_suffix(';')
-            .unwrap_or(&self)
+            .unwrap_or(input)
             // tokenize on whitespace
             .split(' ')
             .map(|c| c.trim())
@@ -49,37 +55,37 @@ impl TryInto<ReplCommand> for String {
         let commands = commands.iter().map(|s| s.as_str()).collect::<Vec<_>>();
 
         match commands.as_slice() {
-            ["help"] => Ok(ReplCommand::Help),
+            ["help"] => Ok(Self::Help),
             ["help", ..] => {
                 let extra_content = commands[1..].join(" ");
                 warn!(%extra_content, "ignoring tokens after 'help'");
-                Ok(ReplCommand::Help)
+                Ok(Self::Help)
             }
-            ["observer"] => Ok(ReplCommand::Observer),
+            ["observer"] => Ok(Self::Observer),
-            ["exit"] => Ok(ReplCommand::Exit),
+            ["exit"] => Ok(Self::Exit),
-            ["quit"] => Ok(ReplCommand::Exit),
+            ["quit"] => Ok(Self::Exit),
             ["use", "database"] => {
                 Err("name not specified. Usage: USE DATABASE <name>".to_string())
             } // USE DATABASE
             ["use", "database", _name] => {
                 // USE DATABASE <name>
-                Ok(ReplCommand::UseDatabase {
+                Ok(Self::UseDatabase {
                     db_name: raw_commands[2].to_string(),
                 })
             }
             ["use", _command] => {
                 // USE <name>
-                Ok(ReplCommand::UseDatabase {
+                Ok(Self::UseDatabase {
                     db_name: raw_commands[1].to_string(),
                 })
             }
-            ["show", "databases"] => Ok(ReplCommand::ShowDatabases),
+            ["show", "databases"] => Ok(Self::ShowDatabases),
-            ["set", "format", _format] => Ok(ReplCommand::SetFormat {
+            ["set", "format", _format] => Ok(Self::SetFormat {
                 format: raw_commands[2].to_string(),
             }),
             _ => {
                 // By default, treat the entire string as SQL
-                Ok(ReplCommand::SqlCommand { sql: self })
+                Ok(Self::SqlCommand { sql: input.into() })
             }
         }
     }
@@ -130,15 +136,6 @@ LIMIT 20
     }
 }
 
-#[cfg(test)]
-impl TryInto<ReplCommand> for &str {
-    type Error = String;
-
-    fn try_into(self) -> Result<ReplCommand, Self::Error> {
-        self.to_string().try_into()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;