Merge pull request #367 from lotabout/ticket-365

fix #365: new option `--show-cmd-error`

man/man1/sk.1

@@ -217,6 +217,13 @@ This is not default however because similar usecases for \fBgrep\fR and
 \fBrg\fR had already been optimized where empty result of a query do mean
 "empty" and previous results should be cleared.
 
+.TP
+.BI "--show-cmd-error"
+If the command fails, send the error messages and show them as items. This
+option was intended to help debugging interactive commands. It's not enabled
+by default because the command often fails before we complete the "cmd-query"
+and error messages would be annoying.
+
 .SS Display
 .TP
 .B "--ansi"

src/bin/main.rs

@@ -55,6 +55,7 @@ Usage: sk [options]
     --keep-right         Keep the right end of the line visible on overflow
     --skip-to-pattern    Line starts with the start of matched pattern
     --no-clear-if-empty  Do not clear previous items if command returns empty result
+    --show-cmd-error     Send command error message if command fails
 
   Layout
     --layout=LAYOUT      Choose layout: [default|reverse|reverse-list]
@@ -238,6 +239,7 @@ fn real_main() -> Result<i32, std::io::Error> {
         .arg(Arg::with_name("pre-select-items").long("pre-select-items").multiple(true).takes_value(true))
         .arg(Arg::with_name("pre-select-file").long("pre-select-file").multiple(true).takes_value(true).default_value(""))
         .arg(Arg::with_name("no-clear-if-empty").long("no-clear-if-empty").multiple(true))
+        .arg(Arg::with_name("show-cmd-error").long("show-cmd-error").multiple(true))
         .get_matches_from(args);
 
     if opts.is_present("help") {
@@ -264,6 +266,7 @@ fn real_main() -> Result<i32, std::io::Error> {
         .with_nth(opts.values_of("with-nth").and_then(|vals| vals.last()).unwrap_or(""))
         .nth(opts.values_of("nth").and_then(|vals| vals.last()).unwrap_or(""))
         .read0(opts.is_present("read0"))
+        .show_error(opts.is_present("show-cmd-error"))
         .build();
 
     let cmd_collector = Rc::new(RefCell::new(SkimItemReader::new(item_reader_option)));

src/helper/item_reader.rs

@@ -1,10 +1,4 @@
 /// helper for turn a BufRead into a skim stream
-use crate::field::FieldRange;
-use crate::helper::item::DefaultSkimItem;
-use crate::reader::CommandCollector;
-use crate::{SkimItem, SkimItemReceiver, SkimItemSender};
-use crossbeam::channel::{bounded, Receiver, Sender};
-use regex::Regex;
 use std::env;
 use std::error::Error;
 use std::io::{BufRead, BufReader};
@@ -13,6 +7,14 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::Arc;
 use std::thread;
 
+use crossbeam::channel::{bounded, Receiver, Sender};
+use regex::Regex;
+
+use crate::field::FieldRange;
+use crate::helper::item::DefaultSkimItem;
+use crate::reader::CommandCollector;
+use crate::{SkimItem, SkimItemReceiver, SkimItemSender};
+
 const CMD_CHANNEL_SIZE: usize = 1024;
 const ITEM_CHANNEL_SIZE: usize = 10240;
 const DELIMITER_STR: &str = r"[\t\n ]+";
@@ -31,6 +33,7 @@ pub struct SkimItemReaderOption {
     matching_fields: Vec<FieldRange>,
     delimiter: Regex,
     line_ending: u8,
+    show_error: bool,
 }
 
 impl Default for SkimItemReaderOption {
@@ -42,6 +45,7 @@ impl Default for SkimItemReaderOption {
             transform_fields: Vec::new(),
             matching_fields: Vec::new(),
             delimiter: Regex::new(DELIMITER_STR).unwrap(),
+            show_error: false,
         }
     }
 }
@@ -108,6 +112,11 @@ impl SkimItemReaderOption {
         self
     }
 
+    pub fn show_error(mut self, show_error: bool) -> Self {
+        self.show_error = show_error;
+        self
+    }
+
     pub fn build(self) -> Self {
         self
     }
@@ -205,17 +214,31 @@ impl SkimItemReader {
         let started = Arc::new(AtomicBool::new(false));
         let started_clone = started.clone();
         let components_to_stop_clone = components_to_stop.clone();
+        let tx_item_clone = tx_item.clone();
+        let send_error = self.option.show_error;
         // listening to close signal and kill command if needed
         thread::spawn(move || {
             debug!("collector: command killer start");
             components_to_stop_clone.fetch_add(1, Ordering::SeqCst);
             started_clone.store(true, Ordering::SeqCst); // notify parent that it is started
 
             let _ = rx_interrupt.recv(); // block waiting
-                                         // clean up resources
             if let Some(mut x) = command {
-                let _ = x.kill();
-                let _ = x.wait();
+                let success = match x.try_wait() {
+                    Ok(Some(status)) if status.success() => true,
+                    _ => false,
+                };
+
+                // clean up resources
+                if success {
+                    let _ = x.kill();
+                    let _ = x.wait();
+                } else if send_error {
+                    let output = x.wait_with_output().expect("could not retrieve error message");
+                    for line in String::from_utf8_lossy(&output.stderr).lines() {
+                        let _ = tx_item_clone.send(Arc::new(line.to_string()));
+                    }
+                }
             }
 
             components_to_stop_clone.fetch_sub(1, Ordering::SeqCst);
@@ -295,13 +318,14 @@ impl CommandCollector for SkimItemReader {
 }
 
 type CommandOutput = (Option<Child>, Box<dyn BufRead + Send>);
+
 fn get_command_output(cmd: &str) -> Result<CommandOutput, Box<dyn Error>> {
     let shell = env::var("SHELL").unwrap_or_else(|_| "sh".to_string());
     let mut command: Child = Command::new(shell)
         .arg("-c")
         .arg(cmd)
         .stdout(Stdio::piped())
-        .stderr(Stdio::null())
+        .stderr(Stdio::piped())
         .spawn()?;
 
     let stdout = command

test/test_skim.py

@@ -895,7 +895,7 @@ def test_query_history(self):
     def test_cmd_history(self):
         """query history should work"""
 
-        history_file = f'{self.tempname()}.history'
+        history_file = f'{self.tempname()}.cmd-history'
 
         self.tmux.send_keys(f"echo -e 'a\nb\nc' > {history_file}", Key('Enter'))
         self.tmux.send_keys(f"""{self.sk("-i -c 'echo {}'", '--cmd-history', history_file)}""", Key('Enter'))
@@ -1031,7 +1031,7 @@ def test_pre_select_file(self):
 
     def test_no_clear_if_empty(self):
         text_file = f'{self.tempname()}.txt'
-        self.tmux.send_keys(f"echo -e 'b\nc' > {text_file}", Key('Enter'))
+        self.tmux.send_keys(f"echo -e 'b\\nc' > {text_file}", Key('Enter'))
 
         args = "-c 'cat {}'" + f''' -i --cmd-query='{text_file}' --no-clear-if-empty'''
         self.tmux.send_keys(f"""{self.sk(args)}""", Key('Enter'))