Add FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS

[ccoVeille]

Check list

• [X] I have performed a self-review of my code
• [ ] I have commented my code in hard-to-understand areas
• [X] I have made corresponding changes to the documentation

Description

FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS variable can be used to tune the rendering of a stash preview.

The following example allows listing the content of the stash before opening it.

FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS="--patch-with-stat --stat-count=10"

Type of change

• [ ] Bug fix
• [X] New feature
• [ ] Refactor
• [ ] Breaking change
• [ ] Documentation change

Test environment

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

[ccoVeille]

Thanks for your contribution @ccoVeille! Just some minor changes to do here. The code itself works fine for me and I find the ability of having the diff stat on top of the preview very neat. The example you provided will go straight to my dotfiles once this is merged 😉

Do you think, l I should add it to the documentation the same way FORGIT_CHECKOUT_BRANCH_BRANCH_GIT_OPTS is explained ?

FORGIT_STASH_SHOW_PREVIEW_GIT_OPTS="--patch-with-stat --stat-count=10"

otherwise you can merge as this

[sandr01d]

Do you think, l I should add it to the documentation the same way FORGIT_CHECKOUT_BRANCH_BRANCH_GIT_OPTS is explained ?

Yes, that sounds useful :+1:

[ccoVeille]

@sandr01d pushed back with the changes in README.md

[ccoVeille]

Please relaunch the CI as it looks like a false positive due to apt

[carlfriedrich]

I would suggest to wait for #391 before merging this (see my comment there).

[carlfriedrich]

@sandr01d @ccoVeille For your information (because that seems to be similar to what you want to achieve here): you can override preview functions by defining your own and then include it in the according FORGIT_*_FZF_OPTS variable.

I am using this by defining a custom preview function for forgit logs and including that in my FORGIT_STASH_FZF_OPTS variable. It includes the diff stat in a dimmed representation, see a screenshot here:

This works already without introducing new variables, so might this eventually be an alternative way to go for you?

[ccoVeille]

My issue is that I cannot achieve what I want with the _FZF variables.

I don't want to affect the git command that is launched of the left, but the git command that are displayed on the right, the preview.

Right now, there is no way to do it.

Edited: I will have a look

[ccoVeille]

@sandr01d @ccoVeille For your information (because that seems to be similar to what you want to achieve here): you can override preview functions by defining your own and then include it in the according FORGIT_*_FZF_OPTS variable.

I am using this by defining a custom preview function for forgit logs and including that in my FORGIT_STASH_FZF_OPTS variable. It includes the diff stat in a dimmed representation, see a screenshot here:

This works already without introducing new variables, so might this eventually be an alternative way to go for you?

I find your solution too complex. I mean you reimplemented somehow the internals of forgit.

It's OK for an advanced shell user, but for a random dev who want to add a parameter to a command. Your code works, someone could copy paste it, but I don't think they would be able to maintain it.

[carlfriedrich]

I don't want to affect the git command that is launched of the left, but the git command that are displayed on the right, the preview.

@ccoVeille Yes, I got that. Take a look at the links in my previous comment, those point to my dotfiles where I configure forgit similarly to what (I think) you want to achieve here. It works with the current forgit code base.

The point to understand is: instead of configuring the preview function, I replace it with my own (using fzf's --preview option). This gives you full freedom over any forgit preview, without having to add more variables to the forgit code.

[carlfriedrich]

@ccoVeille Ah sorry, our comments overlapped. I agree that it's not a simple solution for everyone. However, maybe we could add an examples section to the README for use cases like this? Might be better than introducing more variables for things that already can be configured. Not sure about that myself, yet, though.

Curious to hear opinions from @wfxr @sandr01d @cjappl.

[sandr01d]

Thanks for the insights @carlfriedrich. While the solution you provided is certainly an interesting way to solve this, I think replacing preview functions with injected code goes beyond configuration since it requires familiarity with the inner workings of forgit – at least to a certain extend. I think adding environment variables to handle this would simplify the process of customizing the preview quite a bit (while of course adding complexity to the project). I lean more towards adding such variables if we can manage to reduce boiler plate to a minimum. I agree that the changes from this PR and those proposed in #391 should go into a single PR.

[ccoVeille]

OK for me.

[ccoVeille]

For records, I don't think that I'll be able to work on it in the next days.

[ccoVeille]

For records, I don't think that I'll be able to work on it in the next days.

For records, I'm still alive, but we got a new kid, so well, I'm pretty busy :smile: for now