feat: support fzf extra options#1026
Conversation
| ]) | ||
| .enable_preview() | ||
| let default_args = config::fzf_default_args(); | ||
| let args = if let Some(fzf_extra_opts) = config::fzf_extra_opts() { |
There was a problem hiding this comment.
Since this returns values for both branches, it should probably be a match expression. if let expressions are generally used when there's some code that ought to be conditionally executed, rather than being assigned to a variable in the parent scope.
| fn get_fzf() -> Result<FzfChild> { | ||
| let mut fzf = Fzf::new()?; | ||
| if let Some(fzf_opts) = config::fzf_opts() { | ||
| if let Some(mut fzf_opts) = config::fzf_opts() { |
There was a problem hiding this comment.
This could be a match expression
| let mut fzf = Fzf::new()?; | ||
| if let Some(fzf_opts) = config::fzf_opts() { | ||
| if let Some(mut fzf_opts) = config::fzf_opts() { | ||
| if let Some(fzf_extra_opts) = config::fzf_extra_opts() { |
There was a problem hiding this comment.
This is the right way to use an if let, since there's no other execution path.
diff --git a/src/cmd/query.rs b/src/cmd/query.rs
index adaeff7..652e624 100644
--- a/src/cmd/query.rs
+++ b/src/cmd/query.rs
@@ -3,7 +3,7 @@ use std::io::{self, Write};
use anyhow::{Context, Result};
use crate::cmd::{Query, Run};
-use crate::config;
+use crate::config::{self};
use crate::db::{Database, Epoch, Stream, StreamOptions};
use crate::error::BrokenPipeHandler;
use crate::util::{self, Fzf, FzfChild};
@@ -91,29 +91,33 @@ impl Query {
fn get_fzf() -> Result<FzfChild> {
let mut fzf = Fzf::new()?;
- if let Some(mut fzf_opts) = config::fzf_opts() {
- if let Some(fzf_extra_opts) = config::fzf_extra_opts() {
- fzf_opts.push(" ");
- fzf_opts.push(fzf_extra_opts);
+ match config::fzf_opts() {
+ Some(mut fzf_opts) => {
+ if let Some(fzf_extra_opts) = config::fzf_extra_opts() {
+ fzf_opts.push(" ");
+ fzf_opts.push(fzf_extra_opts);
+ }
+
+ fzf.env("FZF_DEFAULT_OPTS", fzf_opts)
}
+ None => {
+ let default_args = config::fzf_default_args();
+ let args = match config::fzf_extra_opts() {
+ Some(fzf_extra_opts) => {
+ let extra_fzf_args_list: Vec<String> = fzf_extra_opts
+ .to_str()
+ .unwrap_or_default()
+ .split_whitespace()
+ .map(|arg| String::from(arg))
+ .collect();
+
+ vec![default_args, extra_fzf_args_list].concat()
+ }
+ None => default_args,
+ };
- fzf.env("FZF_DEFAULT_OPTS", fzf_opts)
- } else {
- let default_args = config::fzf_default_args();
- let args = if let Some(fzf_extra_opts) = config::fzf_extra_opts() {
- let extra_fzf_args_list: Vec<String> = fzf_extra_opts
- .to_str()
- .unwrap_or_default()
- .split_whitespace()
- .map(|arg| String::from(arg))
- .collect();
-
- vec![default_args, extra_fzf_args_list].concat()
- } else {
- default_args
- };
-
- fzf.args(args).enable_preview()
+ fzf.args(args).enable_preview()
+ }
}
.spawn()
} |
|
Thanks for the review @azaleacolburn. |
|
@ajeetdsouza Would love to see this merged 🙏🏼 |
|
@ajeetdsouza Several issues would be resolved by this feature, which you've already endorsed. I would love to get your review on this PR. |
|
Will this feature make it into zoxide? When I run |
|
@azaleacolburn @ajeetdsouza is there a problem with this PR it's been waiting for a review for almost a year now. Does something need to be done to move it along? |
Description
Add support for a new env variable
_ZO_FZF_EXTRA_OPTSto allow adding flags to the default ones, instead of erasing them.This has been asked in this issue.
Usage
Simply set
_ZO_FZF_EXTRA_OPTSinstead of_ZO_FZF_OPTSto append flags to the default ones.