From 0bd7941a187588299f4141d7f6e47fff67da4643 Mon Sep 17 00:00:00 2001 From: Markus Westerlind <marwes91@gmail.com> Date: Thu, 3 Feb 2022 18:11:01 +0100 Subject: [PATCH] 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 | 1 + influxdb_iox/Cargo.toml | 1 + influxdb_iox/src/commands/sql/repl.rs | 138 ++++++++++++++---- influxdb_iox/src/commands/sql/repl_command.rs | 53 ++++--- 4 files changed, 133 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49b790bb20..c65afd84a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1752,6 +1752,7 @@ dependencies = [ name = "influxdb_iox" version = "0.1.0" dependencies = [ + "ansi_term", "arrow", "arrow-flight", "arrow_util", diff --git a/influxdb_iox/Cargo.toml b/influxdb_iox/Cargo.toml index 17367c79e5..5dbfbff2ef 100644 --- a/influxdb_iox/Cargo.toml +++ b/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" diff --git a/influxdb_iox/src/commands/sql/repl.rs b/influxdb_iox/src/commands/sql/repl.rs index 831a8e4522..12208592dc 100644 --- a/influxdb_iox/src/commands/sql/repl.rs +++ b/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(); - loop { - match self.rl.readline(&self.prompt) { - Ok(ref line) if is_exit_command(line) && request.is_empty() => { - return Ok(ReplCommand::Exit); - } - Ok(ref line) if line.trim_end().ends_with(';') => { - request.push_str(line.trim_end()); - self.rl.add_history_entry(request.clone()); + match self.rl.readline(&self.prompt) { + Ok(ref line) if is_exit_command(line) => Ok(ReplCommand::Exit), + Ok(ref line) => { + let request = line.trim_end(); + self.rl.add_history_entry(request.to_owned()); - return request - .try_into() - .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 }); - } + request + .try_into() + .map_err(|message| Error::ParsingCommand { message }) } + 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 }), } } diff --git a/influxdb_iox/src/commands/sql/repl_command.rs b/influxdb_iox/src/commands/sql/repl_command.rs index db347934df..ae7939bd08 100644 --- a/influxdb_iox/src/commands/sql/repl_command.rs +++ b/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 { - type Error = Self; +impl TryFrom<String> for ReplCommand { + 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> { - debug!(%self, "tokenizing to ReplCommand"); + fn try_from(input: &str) -> Result<Self, Self::Error> { + 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), - ["exit"] => Ok(ReplCommand::Exit), - ["quit"] => Ok(ReplCommand::Exit), + ["observer"] => Ok(Self::Observer), + ["exit"] => Ok(Self::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), - ["set", "format", _format] => Ok(ReplCommand::SetFormat { + ["show", "databases"] => Ok(Self::ShowDatabases), + ["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::*;