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

Preview command cannot be customized in command-specific options #185

Closed
5 of 10 tasks
carlfriedrich opened this issue Feb 27, 2022 · 3 comments · Fixed by #186 or carlfriedrich/forgit#1
Closed
5 of 10 tasks
Assignees

Comments

@carlfriedrich
Copy link
Collaborator

Check list

  • I have read through the README
  • I have the latest version of forgit
  • I have searched through the existing issues

Environment info

  • OS
    • Linux
    • Mac OS X
    • Windows
    • Others:
  • Shell
    • bash
    • zsh
    • fish

Problem / Steps to reproduce

I would like to customize the preview command that it used in glo, so I thought I could easily add it to FORGIT_LOG_FZF_OPTS like described in the README. This does not work, however, because the --preview option is not included in FZF_DEFAULT_OPTS but a hard-coded argument to fzf:

FZF_DEFAULT_OPTS="$opts" fzf --preview="$cmd"

Is there a specific reason for this, or would it be possible to move the --preview="$cmd" argument into the opts between line 27 and 28?

forgit/forgit.plugin.zsh

Lines 23 to 33 in e0d3552

opts="
$FORGIT_FZF_DEFAULT_OPTS
+s +m --tiebreak=index
--bind=\"enter:execute($cmd | LESS='-r' less)\"
--bind=\"ctrl-y:execute-silent(echo {} |grep -Eo '[a-f0-9]+' | head -1 | tr -d '[:space:]' |${FORGIT_COPY_CMD:-pbcopy})\"
$FORGIT_LOG_FZF_OPTS
"
graph=--graph
[[ $FORGIT_LOG_GRAPH_ENABLE == false ]] && graph=
eval "git log $graph --color=always --format='$forgit_log_format' $* $forgit_emojify" |
FZF_DEFAULT_OPTS="$opts" fzf --preview="$cmd"

That way I would be able to override the preview command in FORGIT_LOG_FZF_OPTS.

This also affects most other commands with their corresponding variables:

  • FORGIT_DIFF_FZF_OPTS
  • FORGIT_ADD_FZF_OPTS
  • FORGIT_RESET_HEAD_FZF_OPTS
  • FORGIT_STASH_FZF_OPTS
  • FORGIT_REBASE_FZF_OPTS
  • FORGIT_FIXUP_FZF_OPTS
  • FORGIT_CHECKOUT_FILE_FZF_OPTS
  • FORGIT_CHECKOUT_BRANCH_FZF_OPTS
  • FORGIT_CHECKOUT_TAG_FZF_OPTS
  • FORGIT_COMMIT_FZF_OPTS
  • FORGIT_IGNORE_FZF_OPTS

(BTW why doesn't forgit::cherry::pick have a corresponding variable?)

If nothing speaks against that, I could provide a pull request for it.

carlfriedrich added a commit to carlfriedrich/dotfiles that referenced this issue Feb 27, 2022
The preview command is currently hardcoded in forgit, see
wfxr/forgit#185

Patched this in order to be able to override it.
carlfriedrich added a commit to carlfriedrich/forgit that referenced this issue Mar 7, 2022
Arguments to fzf are passed via the FZF_DEFAULT_OPTS environment
variable, which can be overridden by a dedicated FORGIT_<cmd>_FZF_OPTS
variable for each command separately. The '--preview' argument, however,
was excluded from this mechanism and hard-coded to the fzf call, so that
there was no chance overriding it.
This patch moves the passing of this argument to the FZF_DEFAULT_OPTS
variable for all commands, so that it can be overridden as well.

Fixes wfxr#185
@carlfriedrich
Copy link
Collaborator Author

Since I don't see any drawbacks in making the proposed change, I just filed a PR #186 to fix this. Would love to see this merged.

@carlfriedrich
Copy link
Collaborator Author

Sorry, I accidentally closed this when I merged the change into my own fork. Reopen it to merge it here as well.
@wfxr Let me know if you accept the change.

@wfxr
Copy link
Owner

wfxr commented Mar 21, 2022

@carlfriedrich Nice improvement, thanks for your contribution!

carlfriedrich added a commit to carlfriedrich/forgit that referenced this issue Mar 21, 2022
Arguments to fzf are passed via the FZF_DEFAULT_OPTS environment
variable, which can be overridden by a dedicated FORGIT_<cmd>_FZF_OPTS
variable for each command separately. The '--preview' argument, however,
was excluded from this mechanism and hard-coded to the fzf call, so that
there was no chance overriding it.
This patch moves the passing of this argument to the FZF_DEFAULT_OPTS
variable for all commands, so that it can be overridden as well.

Fixes wfxr#185
@wfxr wfxr closed this as completed in #186 Mar 21, 2022
@cjappl cjappl mentioned this issue Mar 21, 2022
15 tasks
cjappl added a commit that referenced this issue Mar 21, 2022
Checked in a bunch of things that were only for bash #189 #186 #185 #190 #188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants