tpope / vim-fugitive

fugitive.vim: A Git wrapper so awesome, it should be illegal
https://www.vim.org/scripts/script.php?script_id=2975
19.83k stars 1.01k forks source link

Gblame: Moving forward in time #543

Closed idbrii closed 5 years ago

idbrii commented 10 years ago

Gblame is useful to find the last change to a line, but I often want to find a specific change to that line and not just any change. Currently, I can use Gblame and P to go back through time, and once the part of the line that I'm looking for disappears I know I've found my desired commit. However, I'm on some parent of that commit so I can't examine the commit that introduced the change I'm interested in. Now I need to reblame at HEAD and go back until just before that commit (which requires remembering how many times I hit P).

Instead, the Gblame window could keep track of blame target commits ("reblame at commit"). I'd only expect this list to last for the duration of the fugitiveblame buffer (once it's closed, I lose my history).

You could map it to u:

Ideally, traversing along this history would pop the last item so you could go all the way back up to HEAD (and not just the last commit you reblamed).


Example: I have this line:

find_tag("Player1", recursive);

I want to see when the string was set to "Player1":

  1. :Gblame
  2. P several times
  3. Say I'm on commit ab123985 and now it looks like:

    find_tag("Player1");

  4. P again and now I have

    find_tag("Player");

The first argument changed, but I'm on commit be234098, but the commit I'm looking for is ab123985. As far as I can tell, there's no way to get this behavior. I'm proposing that u would bring me back to ab123985 (and u several times would take me back to HEAD).

tpope commented 10 years ago

Not inherently opposed to this, but it sounds complicated to implement.

For comparison, here's the workflow I use:

superjamie commented 8 years ago

I also wish this existed. Often I'm jumping from HEAD through earlier commits, find the commit I want, but then I need to start again at HEAD looking for a different commit.

I thought :Gread - would let me get back to HEAD but it doesn't. Is there a way to reblame at HEAD?

Could the Ctrl+i and Ctrl+o jumplist be used for this? That would be ideal.

jeffvandyke commented 6 years ago

I find Ctrl+o and Ctrl+i works in the file window, but it doesn't work so well in Gblame. I wish I knew if there were ways to programmatically modify the jumplist in vim-fugitive to make this easier.

evantravers commented 6 years ago

I also have been wondering this… I've considered adding u or ``` as a macro/bind to switch to:

  1. Switch to code pane
  2. go back to the previous git buffer in the jumplist
  3. re-run :Gblame on this buffer to "refresh" the blame sidebar.

I will try your flow @tpope… maybe there's a piece I'm missing. I do often wind up chasing the line back through merges/moves trying to find the original commit that introduces an idea, and something smoother would be nice.

(edit: also… thank you for this amazing tool. It has saved me so much time and been such a delight to use, I'd hate for our only interaction on it to be a complaint. Thank you!)

tpope commented 6 years ago

I've since introduced a feature where you can hit <CR> in a blob and jump directly to the blamed commit, which should make the whole jump list thing a lot more manageable.

Also, y'all know you can use :Gedit to go back to the original version to start over, right?

bulletmark commented 6 years ago

What I am looking for is pretty much exactly as tig does it now. Run tig blame file and then press , to reblame. Use < to go back. We want the function of < in fugitive. Presently I am using tig for blame navigation but I prefer to navigate the actual code from within my familiar vim so that is why I would like this in fugitive.

idbrii commented 6 years ago

Using Gedit would mean pressing P several times (counting them), :Gedit, P one less than the count. This is what I currently do and it's awkward (did I press it 15 or 16 times?)

A use case for my workflow is finding when a feature was added. People may change the details of the feature (arguments to a function or rename the function), but I want to see when the whole thing disappears. I can keep Ping until it's completely gone.

Using the workflow you described (and I think the CR feature you mention) is slower since I need to open the diff and blame again. Also, I find the mental context switching between blame view and blob view slows me down (even though fugitive awesomely positions my cursor within the diff on the blame line).

idbrii commented 6 years ago

I've given it a go. https://github.com/idbrii/vim-fugitive/tree/add-blame-undo

I store the blame line and commit in the buffer and use those to "undo" our reblame.

@tpope Does this approach seem reasonable? (I have no idea how blame buffers are manipulated and there's some bdelete that I worry might lose our values.)

I'm not deep in a git-backed project right now so it's a bit hard to test real usage. I've possibly messed up the handling of suffixes in the current commit.

@bulletmark @evantravers Can you test it out?

osa1 commented 5 years ago

@idbrii what's the key to undo the reblame? I'm trying to test your branch but I have no idea which key to press or command to run.

idbrii commented 5 years ago

From the help:

                    u     undo reblame and go back to last blamed commit

On Wed, Sep 26, 2018 at 11:45 PM, Ömer Sinan Ağacan < notifications@github.com> wrote:

@idbrii https://github.com/idbrii what's the key binding to undo the reblame?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tpope/vim-fugitive/issues/543#issuecomment-424977107, or mute the thread https://github.com/notifications/unsubscribe-auth/AACqJ-56m-fXIrp7kbEqMDhHAPPLkv-Mks5ufHQFgaJpZM4CdVVR .

shmerl commented 5 years ago

I got stumbled on this too. Is it good to be upstreamed, or there are problems with this solution?

tpope commented 5 years ago

I don't really understand what the code is doing. The variable line is saved, but the only use of that variable in subsequent invocations that I see is ... saving it again.

This is calling my attention to the fact the existing code is pretty dubious as well. They jump to the line number found in the blame, but that is only the correct line when jumping to the commit with the - map, not for jumping to previous commits with the ~ or P maps. Then again, the correct line has by definition been removed, so there's not really an obvious place to jump to instead.

idbrii commented 5 years ago

I don't really understand what the code is doing. The variable line is saved, but the only use of that variable in subsequent invocations that I see is ... saving it again.

Thanks for having a look at the code! line got lost after a merge. Fixed now. Also pushed another commit that tries to keep the window view the same.

@shmerl No one's tested it out and provided feedback, so there hasn't been any progress.

I tried rebasing on latest and hit a snag in #1282.

odnoletkov commented 5 years ago

While I was also previously missing this 'Back to the Future' feature I since learned to appreciate this workflow:

  • Press enter on the line of interest to jump to the commit that added it.
  • Review the diff for a corresponding removed line of interest.
  • If it exists, press enter to jump to it in the history and reblame.
  • Repeat until reaching the original commit.

With addition of \<CR> to blame the blob it is now simple sequence:

Initial file -> :.Gblame -> commit -> \<CR> -> blob -> \<CR> -> commit -> <CR> -> blob -> …

Back CTRL-o and forward CTRL-i work as expected and often you don't even need that final 'go one step back' step because you just find what you are looking for in the diff.

Additional benefit is it works much faster in busy files/projects because it doesn't load the full blame at any point (I believe).

The only small improvement I can see in this flow is to support :.Gblame for diff lines in commits directly. For '+' lines it is a no-op but '-' and context lines would lead to re-blamed commit directly – without the blob step.

tpope commented 5 years ago

I've added a new mode of operation. :%Gblame (or any other range) opens a blame in a simple horizontal split, free of any coupling to the original buffer. Losing the dual window nature means that CTRL-O and CTRL-I work as expected.

If you're attached to the vertical split, one thing you might try is doing is the recently added :Gblame --reverse once you've overshot. If that works well, I am way more amenable to a --reverse map than I am to inventing a new idiom.

Optimistically closing until I hear otherwise.

idbrii commented 3 years ago

Woah. I missed that reply.

I'm not entirely sure I follow what you mean by Gblame --reverse. I guess the workflow would be something like:

  1. Want to know when fugitive#MapJumps was added to the git FileType autocmd.
  2. :Vo p/fugitive.vim
  3. 367G
  4. :Gblame
  5. P twice
  6. fugitive#MapJumps and the whole autocmd FileType git is gone. I want to go back a step.
  7. :Gblame --reverse{parse commit out of bufname()}..HEAD
  8. jump to appropriate line number
  9. look around for fugitive#MapJumps again
  10. - to go to "next" commit (forward in time)
  11. o to view the commit patch.

The :%Gblame version is much simpler since it works pretty much how I expect, but I lose syntax highlighting in that window. I'll probably try mapping %Gblame | wincmd T to my blame key so I get it in a new tab. (And see if I can figure out how to unintrusively jump it to the current line.) Thanks for adding that feature!

FatBoyXPC commented 3 years ago

I also would love it if ctrl o/i worked appropriately with the vertical split. That syntax highlighting is so nice to have.

@idbrii I tried the steps you outlined and it never brought me back to the appropriate version of the file.

tpope commented 3 years ago

I'm just gonna say it. P is bad. It's what everyone thinks they want - heck, I implemented it because I thought I wanted it too - but what almost nobody needs. Let's dig into @idbrii's example to see what I mean.

  1. Want to know when fugitive#MapJumps was added to the git FileType autocmd.
  2. :Vo p/fugitive.vim

You'll need to :Gedit v3.3~100:% here to get to a version of the file contemporary with the comment.

  1. 367G
  2. :Gblame

(:Git blame now.)

  1. P twice

When I pressed P the first time, I was initially confused. The surrounding context changed a bit, but the line itself looked the same. If I'd been paying closer attention, I might have noticed the shift in indent: The commit we jumped over was a whitespace fix.

This is a fundamental problem with P. You're jumping past a change, and then comparing the new line to your memory of the old line to try to deduce what the change actually was. Small changes like whitespace can be easy to miss, but also large changes can make it hard to pull out what didn't change. And for multiline changes, you can sometimes end up on the wrong line entirely. And stacked on top of all of this is that you're not just jumping over one commit, you're jumping over numerous other commits, most of which are unrelated, but any of which has the potential to change the surrounding context.

If you press <CR> rather than P, you get taken to the appropriate line in the diff that makes it extremely obvious this was a simple whitespace change. Move the cursor up one line and press <CR><CR> and you're onto the next diff. This is the commit that moved the autocommand from the autoload file to the plugin. If your question was "when did it get added to the plugin file", you're done. If you're interested in the true origin of the behavior, you've got your next place <CR><CR>.

  1. fugitive#MapJumps and the whole autocmd FileType git is gone. I want to go back a step.
  2. :Gblame --reverse{parse commit out of bufname()}..HEAD
  3. jump to appropriate line number
  4. look around for fugitive#MapJumps again
  5. - to go to "next" commit (forward in time)

You don't need the argument, :Git blame --reverse will fill it in automatically. But in this case (and probably many cases), it's worthless, as the foothold we need to get back is in a different file. Instead of steps 7 thru 10, you could close the blame window, press <C-O>, and blame again.

  1. o to view the commit patch.

This is the same place that <CR><CR> took us, without needing to overshoot and back up.

In conclusion, "fixing" <C-O>, if it were possible (I highly doubt it could be done robustly), would still not make P worth using.

FatBoyXPC commented 3 years ago

So the work flow you described (<CR> on a commit instead of P, then keep hitting enter on the diff to move through time) works great when you've got a small diff. It doesn't work so great if you've got a large diff. I did the "move up a line and hit <CR> and it put me on a different line of where I initially started traversing history. Instead of "move up a line" it's more like "Move to the reciprocal - line in the diff".

How does :Git blame --reverse work in this case? Every time I've tried to use it I can't seem to find the commit that introduced a given line.

I'll make an honest effort to try moving through diffs instead of P - but the ease of P is just so nice.

tpope commented 3 years ago

So the work flow you described (<CR> on a commit instead of P, then keep hitting enter on the diff to move through time) works great when you've got a small diff. It doesn't work so great if you've got a large diff. I did the "move up a line and hit <CR> and it put me on a different line of where I initially started traversing history. Instead of "move up a line" it's more like "Move to the reciprocal - line in the diff".

See :help fugitive_star, it's custom built for this purpose (but I failed to note that in the docs :thinking:).

How does :Git blame --reverse work in this case? Every time I've tried to use it I can't seem to find the commit that introduced a given line.

"Reverse" here means "find the commit that removed the line".

FatBoyXPC commented 3 years ago

See :help fugitive_star, it's custom built for this purpose

:astonished: :astonished: AMAZING! This would have been handy even before trying this workflow!

"Reverse" here means "find the commit that removed the line".

Okay, I finally figured out how that works, took quite a bit of playing, though. This works great when a line is changed. What about when a line is added, though?

tpope commented 3 years ago

"Reverse" here means "find the commit that removed the line".

Okay, I finally figured out how that works, took quite a bit of playing, though. This works great when a line is changed. What about when a line is added, though?

It's worthless, much like how a regular blame is worthless to find when a line was deleted. I overestimated its usefulness when I recommended it.

FatBoyXPC commented 3 years ago

Ah, I see. I don't remember the last time I had to do such a thing, but I definitely often want to know why lines are added quite often.

idbrii commented 3 years ago

Wow! I've only recently discovered using CR to jump from diffs to blobs (mostly from doing it in status and accidentally modifying the staged file). I never thought to do them while viewing a diff or realized I could hit CR to jump back to a commit. That, along with fugitive_star, creates a much better workflow:

I can jump to the line I want, :.Git blame to go directly to the commit touching that line, and navigate through history via the diffs.

Thanks for the thorough explainer tpope!

shmerl commented 8 months ago

I'm still pretty confused how diffs / CR workflow helps with jumping forward.

Let's say I have a very simple case. A repo with 3 commits. Let's call them A, B, C.

How do I get from there back to HEAD (C) or any other smaller forward step (like commit B for example)?

The closest I got to it is:

Jump back to editor pane, press Ctrl+o and do Git blame again to refresh the blame pane.

tpope commented 8 months ago

How do I get from there back to HEAD (C)

To go to the HEAD version of the file, use :Gedit >HEAD or :Gedit >@. But you probably started in the work tree; if that's where you want to return to, :Gedit with no argument will do the trick.

or any other smaller forward step (like commit B for example)?

Much like in Git itself, there's no step forward operation. But :Git blame --reverse will show you when every line was subsequently changed/deleted; those are stops along the way that may be of interest.

shmerl commented 8 months ago

Ah, I see, thanks! Yeah, using :Git blame --reverse can help jump to a future diff.

shmerl commented 8 months ago

One thing is confusing with using about Git blame --reverse though, that doing "reblame at" (pressing -) doesn't always jump to the listed commit unlike when with Git blame.