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

always use »git show« instead of »git diff« #78

Closed
wants to merge 1 commit into from

Conversation

AlterDepp
Copy link

The mapping <CR> uses git show, in the new window I can open a vimdiff by pressing <CR> on the keyword diff.
The mappings <dd>, <dv>, ... use git diff and I can not open a vimdiff (at least I don’t know how).
This commit fixes the task for me.

@rbong
Copy link
Owner

rbong commented Jun 25, 2022

This is a great partial fix to a longtime issue with Flog/Fugitive. I still have to reject it for a few reasons:

  1. It doesn't work with all bindings.

It doesn't work with bindings that require a path, or with staged/unstaged/untracked changes. It should be consistent.

  1. The diff subcommand is the more appropriate one to use.

I have documentation that uses git diff, I don't want to educate people on the subtle differences.

  1. This should be fixed in Fugitive.

The binding works in git diff buffers so there's an argument to be made that it should work there. If it's not supposed to work there, it should be disabled in these buffers, and there is still an argument for a feature request. If Fugitive rejects this feature there's little we can hope to do - we can't fight against the plugin this is built on top of.

There's likely been discussion on this in the Fugitive issue tracker already. It will probably be hard, but if you want to really fix this you should make a Fugitive PR, if there isn't one already.

We shouldn't start hardcoding workarounds to broken/missing features in Fugitive.


As a workaround, see this user command.

This is better as a workaround because it's opt-in, fixing all the problems I detailed above.

I have created an issue to track this until Fugitive fixes this: #82

@rbong rbong closed this Jun 25, 2022
@rbong
Copy link
Owner

rbong commented Jul 11, 2022

Actually this doesn't work as a workaround either, it doesn't actually diff two commits, just shows each diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants