From c19337c8bfa6bbc8fb791642fb425ea3c8460b42 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 21 Jun 2018 16:26:50 -0700 Subject: [PATCH 1/3] [cli] Part 1: Bump linefeed; use linefeed::Interface; add "--no-tty" argument. I don't really understand why we were using `linefeed::Reader` directly, but reading is not the full set of linefeed features we want to access. I think the `linefeed::Interface` should be owned by the `Repl`, not the `InputReader`, but it's a little awkward to share access with that configuration, so I'm not going to lift the ownership until I have a reason to. I think the "--no-tty" argument might be useful for running inside Emacs. Along the way, I made read_stdin() strip the trailing newline, which agrees with InputReader::read_line(). --- tools/cli/Cargo.toml | 2 +- tools/cli/src/mentat_cli/input.rs | 55 +++++++++++++++++++------------ tools/cli/src/mentat_cli/lib.rs | 15 +++++---- tools/cli/src/mentat_cli/repl.rs | 24 ++++++++++---- 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/tools/cli/Cargo.toml b/tools/cli/Cargo.toml index 6a6eb088..dc377e13 100644 --- a/tools/cli/Cargo.toml +++ b/tools/cli/Cargo.toml @@ -24,7 +24,7 @@ failure = "0.1.1" failure_derive = "0.1.1" getopts = "0.2" lazy_static = "0.2" -linefeed = "0.4" +linefeed = "0.5" log = "0.4" tabwriter = "1" tempfile = "1.1" diff --git a/tools/cli/src/mentat_cli/input.rs b/tools/cli/src/mentat_cli/input.rs index 5956ef91..c190aaab 100644 --- a/tools/cli/src/mentat_cli/input.rs +++ b/tools/cli/src/mentat_cli/input.rs @@ -8,11 +8,15 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -use std::io::stdin; +use std::io::{ + stdin, + stdout, + Write, +}; use linefeed::{ DefaultTerminal, - Reader, + Interface, ReadResult, Signal, }; @@ -52,7 +56,7 @@ pub enum InputResult { /// Reads input from `stdin` pub struct InputReader { buffer: String, - reader: Option>, + interface: Option>, in_process_cmd: Option, } @@ -71,27 +75,24 @@ enum UserAction { impl InputReader { /// Constructs a new `InputReader` reading from `stdin`. - pub fn new() -> InputReader { - let r = match Reader::new("mentat") { - Ok(mut r) => { - // Handle SIGINT (Ctrl-C) - r.set_report_signal(Signal::Interrupt, true); - r.set_word_break_chars(" \t\n!\"#$%&'(){}*+,-./:;<=>?@[\\]^`"); - Some(r) - }, - Err(_) => None, - }; + pub fn new(interface: Option>) -> InputReader { + if let Some(ref interface) = interface { + let mut r = interface.lock_reader(); + // Handle SIGINT (Ctrl-C) + r.set_report_signal(Signal::Interrupt, true); + r.set_word_break_chars(" \t\n!\"#$%&'(){}*+,-./:;<=>?@[\\]^`"); + } InputReader{ buffer: String::new(), - reader: r, + interface, in_process_cmd: None, } } /// Returns whether the `InputReader` is reading from a TTY. pub fn is_tty(&self) -> bool { - self.reader.is_some() + self.interface.is_some() } /// Reads a single command, item, or statement from `stdin`. @@ -179,7 +180,7 @@ impl InputReader { } fn read_line(&mut self, prompt: &str) -> UserAction { - match self.reader { + match self.interface { Some(ref mut r) => { r.set_prompt(prompt); r.read_line().ok().map_or(UserAction::Quit, |line| @@ -191,7 +192,13 @@ impl InputReader { }) }, - None => self.read_stdin() + None => { + print!("{}", prompt); + if stdout().flush().is_err() { + return UserAction::Quit; + } + self.read_stdin() + }, } } @@ -200,13 +207,19 @@ impl InputReader { match stdin().read_line(&mut s) { Ok(0) | Err(_) => UserAction::Quit, - Ok(_) => UserAction::TextInput(s) + Ok(_) => { + if s.ends_with("\n") { + let len = s.len() - 1; + s.truncate(len); + } + UserAction::TextInput(s) + }, } } - fn add_history(&mut self, line: String) { - if let Some(ref mut r) = self.reader { - r.add_history(line); + fn add_history(&self, line: String) { + if let Some(ref interface) = self.interface { + interface.add_history(line); } } } diff --git a/tools/cli/src/mentat_cli/lib.rs b/tools/cli/src/mentat_cli/lib.rs index bb637e19..b8df8caf 100644 --- a/tools/cli/src/mentat_cli/lib.rs +++ b/tools/cli/src/mentat_cli/lib.rs @@ -64,6 +64,7 @@ pub fn run() -> i32 { opts.optmulti("t", "transact", "Execute a transact on startup. Transacts are executed before queries.", "TRANSACT"); opts.optmulti("i", "import", "Execute an import on startup. Imports are executed before queries.", "PATH"); opts.optflag("v", "version", "Print version and exit"); + opts.optflag("", "no-tty", "Don't try to use a TTY for readline-like input processing"); let matches = match opts.parse(&args[1..]) { Ok(m) => m, @@ -121,13 +122,15 @@ pub fn run() -> i32 { } }).collect(); - let repl = repl::Repl::new(); - if repl.is_ok() { - repl.unwrap().run(Some(cmds)); + let mut repl = match repl::Repl::new(!matches.opt_present("no-tty")) { + Ok(repl) => repl, + Err(e) => { + println!("{}", e); + return 1 + } + }; - } else { - println!("{}", repl.err().unwrap()); - } + repl.run(Some(cmds)); 0 } diff --git a/tools/cli/src/mentat_cli/repl.rs b/tools/cli/src/mentat_cli/repl.rs index b4f66197..d68b3d7a 100644 --- a/tools/cli/src/mentat_cli/repl.rs +++ b/tools/cli/src/mentat_cli/repl.rs @@ -16,6 +16,10 @@ use failure::{ Error, }; +use linefeed::{ + Interface, +}; + use tabwriter::TabWriter; use termion::{ @@ -185,6 +189,7 @@ fn format_time(duration: Duration) { /// Executes input and maintains state of persistent items. pub struct Repl { + input_reader: InputReader, path: String, store: Store, timer_on: bool, @@ -200,19 +205,26 @@ impl Repl { } /// Constructs a new `Repl`. - pub fn new() -> Result { + pub fn new(tty: bool) -> Result { + let interface = if tty { + Some(Interface::new("mentat").map_err(|_| "failed to create tty interface; try --no-tty")?) + } else { + None + }; + + let input_reader = InputReader::new(interface); + let store = Store::open("").map_err(|e| e.to_string())?; Ok(Repl { + input_reader, path: "".to_string(), - store: store, + store, timer_on: false, }) } /// Runs the REPL interactively. pub fn run(&mut self, startup_commands: Option>) { - let mut input = InputReader::new(); - if let Some(cmds) = startup_commands { for command in cmds.iter() { println!("{}", command.output()); @@ -221,7 +233,7 @@ impl Repl { } loop { - let res = input.read_input(); + let res = self.input_reader.read_input(); match res { Ok(MetaCommand(cmd)) => { @@ -231,7 +243,7 @@ impl Repl { Ok(Empty) | Ok(More) => (), Ok(Eof) => { - if input.is_tty() { + if self.input_reader.is_tty() { println!(); } break; From c41d728d1d837fc345d6d896c1a7c51d82acd8a4 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Fri, 22 Jun 2018 11:28:02 -0700 Subject: [PATCH 2/3] [cli] Part 2: Don't use exit() to terminate the CLI. It's not possible to do meaningful clean-up (such as saving history) if we use exit() to quit. Instead, each handled command returns a boolean requesting exit. I elected not to allow ".exit" when processing commands from the command line; it might be useful to handle accept that. In general, though, REPLs that accept "-c 'commands'" on the command line exit after processing those commands, so I'd rather think more deeply about that model than build in ".exit" with our existing system. --- tools/cli/src/mentat_cli/repl.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/cli/src/mentat_cli/repl.rs b/tools/cli/src/mentat_cli/repl.rs index d68b3d7a..266e0c30 100644 --- a/tools/cli/src/mentat_cli/repl.rs +++ b/tools/cli/src/mentat_cli/repl.rs @@ -9,7 +9,6 @@ // specific language governing permissions and limitations under the License. use std::io::Write; -use std::process; use failure::{ err_msg, @@ -238,7 +237,9 @@ impl Repl { match res { Ok(MetaCommand(cmd)) => { debug!("read command: {:?}", cmd); - self.handle_command(cmd); + if !self.handle_command(cmd) { + break; + } }, Ok(Empty) | Ok(More) => (), @@ -265,7 +266,7 @@ impl Repl { } /// Runs a single command input. - fn handle_command(&mut self, cmd: Command) { + fn handle_command(&mut self, cmd: Command) -> bool { let should_print_times = self.timer_on && cmd.is_timed(); let mut start = PreciseTime::now(); @@ -279,9 +280,8 @@ impl Repl { self.close(); }, Command::Exit => { - self.close(); eprintln!("Exiting…"); - process::exit(0); + return false; }, Command::Help(args) => { self.help_command(args); @@ -378,6 +378,8 @@ impl Repl { eprint!(": "); format_time(start.to(end)); } + + return true; } fn execute_import(&mut self, path: T) From 4ea9c78c50984786a7683675926b342ec88a4095 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Fri, 22 Jun 2018 11:26:52 -0700 Subject: [PATCH 3/3] [cli] Part 3: {load,save}_history as appropriate. It's possible that we should be saving more aggressively -- perhaps after each entered command -- but we can add that later. --- tools/cli/src/mentat_cli/input.rs | 15 +++++++++++++++ tools/cli/src/mentat_cli/lib.rs | 15 +++++++++++++++ tools/cli/src/mentat_cli/repl.rs | 2 ++ 3 files changed, 32 insertions(+) diff --git a/tools/cli/src/mentat_cli/input.rs b/tools/cli/src/mentat_cli/input.rs index c190aaab..27139af8 100644 --- a/tools/cli/src/mentat_cli/input.rs +++ b/tools/cli/src/mentat_cli/input.rs @@ -77,6 +77,11 @@ impl InputReader { /// Constructs a new `InputReader` reading from `stdin`. pub fn new(interface: Option>) -> InputReader { if let Some(ref interface) = interface { + // It's fine to fail to load history. + let p = ::history_file_path(); + let loaded = interface.load_history(&p); + debug!("history read from {}: {}", p.display(), loaded.is_ok()); + let mut r = interface.lock_reader(); // Handle SIGINT (Ctrl-C) r.set_report_signal(Signal::Interrupt, true); @@ -221,5 +226,15 @@ impl InputReader { if let Some(ref interface) = self.interface { interface.add_history(line); } + self.save_history(); + } + + pub fn save_history(&self) -> () { + if let Some(ref interface) = self.interface { + let p = ::history_file_path(); + // It's okay to fail to save history. + let saved = interface.save_history(&p); + debug!("history saved to {}: {}", p.display(), saved.is_ok()); + } } } diff --git a/tools/cli/src/mentat_cli/lib.rs b/tools/cli/src/mentat_cli/lib.rs index b8df8caf..0a47272b 100644 --- a/tools/cli/src/mentat_cli/lib.rs +++ b/tools/cli/src/mentat_cli/lib.rs @@ -10,6 +10,10 @@ #![crate_name = "mentat_cli"] +use std::path::{ + PathBuf, +}; + #[macro_use] extern crate failure_derive; #[macro_use] extern crate log; #[macro_use] extern crate lazy_static; @@ -36,6 +40,17 @@ use termion::{ color, }; +static HISTORY_FILE_PATH: &str = ".mentat_history"; + +/// The Mentat CLI stores input history in a readline-compatible file like "~/.mentat_history". +/// This accords with main other tools which prefix with "." and suffix with "_history": lein, +/// node_repl, python, and sqlite, at least. +pub(crate) fn history_file_path() -> PathBuf { + let mut p = ::std::env::home_dir().unwrap_or_default(); + p.push(::HISTORY_FILE_PATH); + p +} + static BLUE: color::Rgb = color::Rgb(0x99, 0xaa, 0xFF); static GREEN: color::Rgb = color::Rgb(0x77, 0xFF, 0x99); diff --git a/tools/cli/src/mentat_cli/repl.rs b/tools/cli/src/mentat_cli/repl.rs index 266e0c30..ce08286a 100644 --- a/tools/cli/src/mentat_cli/repl.rs +++ b/tools/cli/src/mentat_cli/repl.rs @@ -252,6 +252,8 @@ impl Repl { Err(e) => eprintln!("{}", e.to_string()), } } + + self.input_reader.save_history(); } fn cache(&mut self, attr: String, direction: CacheDirection) {