Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support external subcommands: rg, diff, git-show (etc.) #1769

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::ffi::OsString;
use std::path::{Path, PathBuf};

use bat::assets::HighlightingAssets;
use clap::error::Error;
use clap::{ArgMatches, ColorChoice, CommandFactory, FromArgMatches, Parser, ValueEnum, ValueHint};
use clap_complete::Shell;
use console::Term;
Expand All @@ -16,6 +17,7 @@ use crate::config::delta_unreachable;
use crate::env::DeltaEnv;
use crate::git_config::GitConfig;
use crate::options;
use crate::subcommands;
use crate::utils;
use crate::utils::bat::output::PagingMode;

Expand Down Expand Up @@ -1217,23 +1219,12 @@ pub enum DetectDarkLight {
#[derive(Debug)]
pub enum Call<T> {
Delta(T),
DeltaDiff(T, PathBuf, PathBuf),
SubCommand(T, subcommands::SubCommand),
Help(String),
Version(String),
}

// Custom conversion because a) generic TryFrom<A,B> is not possible and
// b) the Delta(T) variant can't be converted.
impl<A> Call<A> {
fn try_convert<B>(self) -> Option<Call<B>> {
use Call::*;
match self {
Delta(_) => None,
Help(help) => Some(Help(help)),
Version(ver) => Some(Version(ver)),
}
}
}

impl Opt {
fn handle_help_and_version(args: &[OsString]) -> Call<ArgMatches> {
match Self::command().try_get_matches_from(args) {
Expand Down Expand Up @@ -1283,31 +1274,55 @@ impl Opt {
Call::Help(help)
}
Err(e) => {
e.exit();
// Calls `e.exit()` if error persists.
let (matches, subcmd) = subcommands::extract(args, e);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clever...

Call::SubCommand(matches, subcmd)
}
Ok(matches) => {
// subcommands take precedence over diffs
let minus_file = matches.get_one::<PathBuf>("minus_file").map(PathBuf::from);
Copy link
Owner

@dandavison dandavison Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is our support for delta file_a file_b going to cause trouble when introducing standard clap subcommands?

if let Some(subcmd) = &minus_file {
if let Some(arg) = subcmd.to_str() {
Comment on lines +1284 to +1285
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically just code golf, done while trying to get into the substance of the PR. Feel free to ignore.

Suggested change
if let Some(subcmd) = &minus_file {
if let Some(arg) = subcmd.to_str() {
if let Some(arg) = minus_file.as_ref().and_then(|p| p.to_str()) {

if subcommands::SUBCOMMANDS.contains(&arg) {
let unreachable_error =
Error::new(clap::error::ErrorKind::InvalidSubcommand);
let (matches, subcmd) = subcommands::extract(args, unreachable_error);
return Call::SubCommand(matches, subcmd);
}
}
}

match (
minus_file,
matches.get_one::<PathBuf>("plus_file").map(PathBuf::from),
) {
(Some(minus_file), Some(plus_file)) => {
Call::DeltaDiff(matches, minus_file, plus_file)
}
_ => Call::Delta(matches),
}
}
Ok(matches) => Call::Delta(matches),
}
}

pub fn from_args_and_git_config(
args: Vec<OsString>,
env: &DeltaEnv,
assets: HighlightingAssets,
) -> Call<Self> {
) -> (Call<()>, Option<Opt>) {
#[cfg(test)]
// Set argv[0] when called in tests:
let args = {
let mut args = args;
args.insert(0, OsString::from("delta"));
args
};
let matches = match Self::handle_help_and_version(&args) {
Call::Delta(t) => t,
msg => {
return msg
.try_convert()
.unwrap_or_else(|| panic!("Call<_> conversion failed"))
}
let (matches, call) = match Self::handle_help_and_version(&args) {
Call::Delta(t) => (t, Call::Delta(())),
Call::DeltaDiff(t, a, b) => (t, Call::DeltaDiff((), a, b)),
Call::SubCommand(t, cmd) => (t, Call::SubCommand((), cmd)),
Call::Help(help) => return (Call::Help(help), None),
Call::Version(ver) => return (Call::Version(ver), None),
};

let mut final_config = if *matches.get_one::<bool>("no_gitconfig").unwrap_or(&false) {
Expand All @@ -1323,12 +1338,8 @@ impl Opt {
}
}

Call::Delta(Self::from_clap_and_git_config(
env,
matches,
final_config,
assets,
))
let opt = Self::from_clap_and_git_config(env, matches, final_config, assets);
(call, Some(opt))
}

pub fn from_iter_and_git_config<I>(
Expand Down Expand Up @@ -1399,9 +1410,3 @@ lazy_static! {
.into_iter()
.collect();
}

// Call::Help(format!(
// "foo\nbar\nbatz\n{}\n{}",
// help.replace("Options:", "well well\n\nOptions:"),
// h2
// ))
186 changes: 151 additions & 35 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ mod subcommands;

mod tests;

use std::ffi::OsString;
use std::io::{self, Cursor, ErrorKind, IsTerminal, Write};
use std::process;
use std::ffi::{OsStr, OsString};
use std::io::{self, BufRead, Cursor, ErrorKind, IsTerminal, Write};
use std::process::{self, Command, Stdio};

use bytelines::ByteLinesReader;

use crate::cli::Call;
use crate::config::delta_unreachable;
use crate::delta::delta;
use crate::subcommands::{SubCmdKind, SubCommand};
use crate::utils::bat::assets::list_languages;
use crate::utils::bat::output::{OutputType, PagingMode};

Expand Down Expand Up @@ -67,7 +69,7 @@ fn main() -> std::io::Result<()> {
ctrlc::set_handler(|| {})
.unwrap_or_else(|err| eprintln!("Failed to set ctrl-c handler: {err}"));
let exit_code = run_app(std::env::args_os().collect::<Vec<_>>(), None)?;
// when you call process::exit, no destructors are called, so we want to do it only once, here
// when you call process::exit, no drop impls are called, so we want to do it only once, here
process::exit(exit_code);
}

Expand All @@ -81,19 +83,25 @@ pub fn run_app(
) -> std::io::Result<i32> {
let env = env::DeltaEnv::init();
let assets = utils::bat::assets::load_highlighting_assets();
let opt = cli::Opt::from_args_and_git_config(args, &env, assets);
let (call, opt) = cli::Opt::from_args_and_git_config(args, &env, assets);

let opt = match opt {
Call::Version(msg) => {
writeln!(std::io::stdout(), "{}", msg.trim_end())?;
return Ok(0);
}
Call::Help(msg) => {
OutputType::oneshot_write(msg)?;
return Ok(0);
}
Call::Delta(opt) => opt,
};
if let Call::Version(msg) = call {
writeln!(std::io::stdout(), "{}", msg.trim_end())?;
return Ok(0);
} else if let Call::Help(msg) = call {
OutputType::oneshot_write(msg)?;
return Ok(0);
} else if let Call::SubCommand(_, cmd) = &call {
// Set before creating the Config, which already asks for the calling process
// (not required for Call::DeltaDiff)
utils::process::set_calling_process(
&cmd.args
.iter()
.map(|arg| OsStr::to_string_lossy(arg).to_string())
.collect::<Vec<_>>(),
);
}
let opt = opt.expect("Opt is set");

let subcommand_result = if let Some(shell) = opt.generate_completion {
Some(subcommands::generate_completion::generate_completion_file(
Expand Down Expand Up @@ -153,26 +161,134 @@ pub fn run_app(
output_type.handle().unwrap()
};

if let (Some(minus_file), Some(plus_file)) = (&config.minus_file, &config.plus_file) {
let exit_code = subcommands::diff::diff(minus_file, plus_file, &config, &mut writer);
return Ok(exit_code);
}
let subcmd = match call {
Call::DeltaDiff(_, minus, plus) => {
match subcommands::diff::build_diff_cmd(&minus, &plus, &config) {
Err(code) => return Ok(code),
Ok(val) => val,
}
}
Call::SubCommand(_, subcmd) => subcmd,
Call::Delta(_) => SubCommand::none(),
Call::Help(_) | Call::Version(_) => delta_unreachable("help/version handled earlier"),
};

if io::stdin().is_terminal() {
eprintln!(
"\
The main way to use delta is to configure it as the pager for git: \
see https://github.com/dandavison/delta#get-started. \
You can also use delta to diff two files: `delta file_A file_B`."
);
return Ok(config.error_exit_code);
}
if subcmd.is_none() {
// Default delta run: read input from stdin, write to stdout or pager (pager started already^).
if io::stdin().is_terminal() {
eprintln!(
"\
The main way to use delta is to configure it as the pager for git: \
see https://github.com/dandavison/delta#get-started. \
You can also use delta to diff two files: `delta file_A file_B`."
);
return Ok(config.error_exit_code);
}

if let Err(error) = delta(io::stdin().lock().byte_lines(), &mut writer, &config) {
match error.kind() {
ErrorKind::BrokenPipe => return Ok(0),
_ => eprintln!("{error}"),
let res = delta(io::stdin().lock().byte_lines(), &mut writer, &config);

if let Err(error) = res {
match error.kind() {
ErrorKind::BrokenPipe => return Ok(0),
_ => {
eprintln!("{error}");
return Ok(config.error_exit_code);
}
}
}
};
Ok(0)

Ok(0)
} else {
// First start a subcommand, and process input from it to delta(). Also handle
// subcommand stderr and exit codes, e.g. for git and diff logic.

let (subcmd_bin, subcmd_args) = subcmd.args.split_first().unwrap();
let subcmd_kind = subcmd.kind; // for easier {} formatting

let subcmd_bin_path = match grep_cli::resolve_binary(std::path::PathBuf::from(subcmd_bin)) {
Ok(path) => path,
Err(err) => {
eprintln!("Failed to resolve command {subcmd_bin:?}: {err}");
return Ok(config.error_exit_code);
}
};

let cmd = Command::new(subcmd_bin)
.args(subcmd_args.iter())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn();

if let Err(err) = cmd {
eprintln!("Failed to execute the command {subcmd_bin:?}: {err}");
return Ok(config.error_exit_code);
}
let mut cmd = cmd.unwrap();

let cmd_stdout = cmd.stdout.as_mut().expect("Failed to open stdout");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a so-far-upheld tradition of using unwrap_or_else(|| panic(...)) because of my personal belief that expect() is confusing as reader and author, but I'd certainly understand if you wished to discontinue that tradition.

let cmd_stdout_buf = io::BufReader::new(cmd_stdout);

let res = delta(cmd_stdout_buf.byte_lines(), &mut writer, &config);

if let Err(error) = res {
match error.kind() {
ErrorKind::BrokenPipe => return Ok(0),
_ => {
eprintln!("{error}");
return Ok(config.error_exit_code);
}
}
};

let subcmd_status = cmd
.wait()
.unwrap_or_else(|_| {
delta_unreachable(&format!("{subcmd_kind} process not running."));
})
.code()
.unwrap_or_else(|| {
eprintln!("{subcmd_kind} process terminated without exit status.");
config.error_exit_code
});

let mut stderr_lines =
io::BufReader::new(cmd.stderr.expect("Failed to open stderr")).lines();
if let Some(line1) = stderr_lines.next() {
// prefix the first error line prefixed with the called subcommand
eprintln!(
"{}: {}",
subcmd_kind,
line1.unwrap_or("<delta: could not parse line>".into())
);
}

// On `git diff` unknown option: print first line (which is an error message) but not
// the remainder (which is the entire --help text).
if !(subcmd_status == 129
&& matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Git(_)))
{
for line in stderr_lines {
eprintln!("{}", line.unwrap_or("<delta: could not parse line>".into()));
}
}

if matches!(subcmd_kind, SubCmdKind::GitDiff | SubCmdKind::Diff) && subcmd_status >= 2 {
eprintln!(
"{subcmd_kind} process failed with exit status {subcmd_status}. Command was: {}",
format_args!(
"{} {}",
subcmd_bin_path.display(),
shell_words::join(
subcmd_args
.iter()
.map(|arg0: &OsString| std::ffi::OsStr::to_string_lossy(arg0))
),
)
);
}

Ok(subcmd_status)
}

// `output_type` drop impl runs here
}
Loading
Loading