- Notifications
You must be signed in to change notification settings - Fork707
feat: support fzf extra options#1026
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/cmd/query.rs Outdated
| ]) | ||
| .enable_preview() | ||
| let default_args = config::fzf_default_args(); | ||
| let args =ifletSome(fzf_extra_opts) = config::fzf_extra_opts(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Since this returns values for both branches, it should probably be amatch 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Indeed, udpated too!
src/cmd/query.rs Outdated
| fnget_fzf() ->Result<FzfChild>{ | ||
| letmut fzf =Fzf::new()?; | ||
| ifletSome(fzf_opts) = config::fzf_opts(){ | ||
| ifletSome(mut fzf_opts) = config::fzf_opts(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This could be a match expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's right, updated.
src/cmd/query.rs Outdated
| letmut fzf =Fzf::new()?; | ||
| ifletSome(fzf_opts) = config::fzf_opts(){ | ||
| ifletSome(mut fzf_opts) = config::fzf_opts(){ | ||
| ifletSome(fzf_extra_opts) = config::fzf_extra_opts(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is the right way to use anif let, since there's no other execution path.
azaleacolburn commentedMay 15, 2025
diff --git a/src/cmd/query.rs b/src/cmd/query.rsindex 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() } |
ValJed commentedMay 19, 2025
Thanks for the review@azaleacolburn. |
ValJed commentedJun 4, 2025
@ajeetdsouza Would love to see this merged 🙏🏼 |
azaleacolburn commentedAug 10, 2025
@ajeetdsouza Several issues would be resolved by this feature, which you've already endorsed. I would love to get your review on this PR. |
j8s0n commentedOct 2, 2025
Will this feature make it into zoxide? When I run |
Uh oh!
There was an error while loading.Please reload this page.
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 inthis issue.
Usage
Simply set
_ZO_FZF_EXTRA_OPTSinstead of_ZO_FZF_OPTSto append flags to the default ones.