Added ability to navigate parent/child commits

[TamaMcGlinn]

Added functions to autoload/flog.vim to navigate to children and parents. This is the same as GitExtensions' Cntrl-P and Cntrl-N shortcuts. To bind, do something like:

augroup flog
  autocmd FileType floggraph nno <buffer> <silent> ]c :<C-U>call flog#jump_to_parent()<CR>
  autocmd FileType floggraph nno <buffer> <silent> [c :<C-U>call flog#jump_to_child()<CR>
augroup END

It does this in a stable way by recording the previous inputs and outputs, so that going up X times and then down X times, or vice-verse, you always end up on the same commit. So you navigate down through parents from your feature branch back onto develop (before you had started your branch), navigating back up to children will go back to your feature branch. It is also stable in that it always ends up on the symbol of the commit graph, so that you could use this in a macro to e.g. get the commit messages of a branch into a buffer.

Navigating to child commits also works if they are only available in the reflog, ~assuming that vim-flog is set to display such commits (use gr in the graph to toggle this).~ and will enable the reflog if necessary while navigating.

• Unless you hit the top or bottom of the git graph, of course; then such navigation is a no-op.

[TamaMcGlinn]

This fixes #45

[TamaMcGlinn]

~Unfortunately, it's not currently repeatable, so 5]c doesn't work. I don't know how to solve that. (does mentioning Tim Pope in a PR summon him to come and help you fix some vimscript?)~

5]c now also works

[TamaMcGlinn]

I think there must be a reason that fugitive executes git commands different to what I have done, but I can't think what it is. For example, instead of doing system('git log') directly, fugitive#Prepare('log') gives git -C /home/tama/code/vimplugins/vim-flog --literal-pathspecs log. I think I need to apply this change at least to all my changes in the PR, but possibly also to the rest of flog? @rbong have you been using fugitive#Prepare everywhere? Maybe this becomes relevant when we are in a submodule?

[rbong]

Sorry for not being very responsive lately, I have been pretty busy with work. I have taken a quick look and left all of my initial thoughts so you're not left for weeks not knowing what to do, but I still have to do a deep dive.

I believe your question about git commands is answered by my one response.

Also, do you plan to add default bindings as part of this MR?

[TamaMcGlinn]

Thanks very much; it would great if you could have a look later on and establish if I can reuse more existing code; I have been reading and re-reading vim-flog, but with all the caching and passing around initially empty local cache variables, it is nigh impossible for me to figure out how to use it.

I have mapped these to hjkl myself as follows. I don't understand how to add default bindings but it would be a great idea:

augroup flog
  autocmd FileType floggraph nno <buffer> <silent> h :<C-U>call flog#jump_to_parent()<CR>
  autocmd FileType floggraph nno <buffer> <silent> j :<C-U>call flog#down()<CR>
  autocmd FileType floggraph nno <buffer> <silent> k :<C-U>call flog#up()<CR>
  autocmd FileType floggraph nno <buffer> <silent> l :<C-U>call flog#jump_to_child()<CR>

  autocmd FileType floggraph nno <buffer> <silent> ]h :<C-U>call flog#jump_to_next_head()<CR>
  autocmd FileType floggraph nno <buffer> <silent> [h :<C-U>call flog#jump_to_previous_head()<CR>
augroup END

Note the hackery in flog#up/down to allow a single j/k to skip to the next commit (i.e. skip 'graph-only' lines), while using the relative line numbers with e.g. 7j will still jump to the line you expect.

[rbong]

Apologies for the difficulties reading Flog... code readability and self-documentation was a big initial goal of this project so I'm sad to hear that. Let me know if you have any ideas for increasing its readability.

Mappings can be added in ftplugin/floggraph.vim. It's not a huge deal for me to add mappings/documentation when this gets merged in but any help is appreciated.

[TamaMcGlinn]

It could be a lot worse. In fact, my own vimscript project will probably be a lot worse, since I am just quickly learning barely enough vimscript to get by. For my work I am in a hurry to get a reasonable menu-based UI on top of flog, so I can stop issuing a myriad of commands on the commandline. Professionally, I used GitExtensions for the past 5 years but have now switched to linux, so I'm looking to port my past experience and flog together with fugitive has already satisfied the first 50% I would say. Thank you.

The main problem I have is with finding out how to call functions; like what should the type of each parameter be - I have just been looking up all the references to the function to do that, which works.

[TamaMcGlinn]

Oh, there were two vint errors that I have nothing to do with; I'm not sure if you can suppress them or just don't care:

syntax/floggraph.vim:27:80: E492: Not an editor command: )\)/ (see vim-jp/vim-vimlparser)
t/flog.vim:6:1: E492: Not an editor command: describe ':Flog' (see vim-jp/vim-vimlparser)

[TamaMcGlinn]

I'm a bit confused about function! flog#get_ref_types(commit) abort which didn't exist when I started this PR, but I needed it so I made the very similar flog#parse_ref_name_list(commit) in commit d680a56, and then in the main branch you created it as well, so I could switch to it, but then you deleted it again in 1555cdb3.

As it stands, this PR will re-add the function, ~but it will no longer work, giving an index out of range in at least one case: when HEAD, master, origin/master and a tag are on the same line and you call this (I haven't tested other cases).~

[rbong]

As it stands, this PR will re-add the function, but it will no longer work, giving an index out of range in at least one case: when HEAD, master, origin/master and a tag are on the same line and you call this (I haven't tested other cases).

I think this function was only meant to be used in very specific cases, and hence why it was removed, and it might have been allowed to fail in that specific case. I'm not exactly sure what you mean by it giving an index out of range, will this be an issue for this feature? Are you waiting on me to fix it in the original function?

At this point pretty much everything looks good except for minor issues like naming and organization, which I can fix on merge along with that one issue. I just need you to confirm if this MR is stable enough to merge, and after that I just need to find the time.

[TamaMcGlinn]

Okay, tested and this works just fine after merging to master. I was asking because, in another plugin, I copied over your parsing code, and I just found an edge case in which it doesn't work anymore. But I won't bother you with that, it has nothing to do with flog. I was wondering if the same edge case might also affect flog, but I can't find any way to trigger the error.

I never answered your question regarding default keybindings; of course, I love my own keybindings, perhaps try them yourself and see how you feel:

  autocmd FileType floggraph nno <buffer> <silent> h :<C-U>call flog#jump_to_parent()<CR>
  autocmd FileType floggraph nno <buffer> <silent> j :<C-U>call flog#down()<CR>
  autocmd FileType floggraph nno <buffer> <silent> k :<C-U>call flog#up()<CR>
  autocmd FileType floggraph nno <buffer> <silent> l :<C-U>call flog#jump_to_child()<CR>

Since your fingers rest on j and k, I wanted those to be the quick navigation. For linear git histories they are of course all that you need; but when you find yourself on a commit whose parent is way down / off the screen, conceptually I think of myself going a page to the left / right, hence the mappings. I got used to these on day one.

[rbong]

I'm probably going to be removing the commit parsing function again, this was removed because the values should cached now and not recalculated.

[TamaMcGlinn]

~Ah, but wait; you just added the ability to sort in reverse; we will need an extra layer of abstraction which inverts the directions when that is the case, otherwise the jk mappings will be very unintuitive. Not sure about h and l, though.~

That works automagically, just tested and jk work, while h and l do the same thing regardless of sorting (which is fine in my opinion)

[TamaMcGlinn]

So the TODO is:

• Map hjkl if there is no user mapping - this means adding commands :FlogUp, :FlogChild [n] and plug mappings I guess
• Remove the commit parsing function (optional, cleaner, but could be done later and really has nothing to do with this PR)
• ~Make everything work for multiple repositories by storing stuff per repo (in my opinion, not necessary)~ DONE

[TamaMcGlinn]

Could you, pretty please, add the default hjkl bindings to this, and then merge? I'm sorry for pressing you on this, it's just that I'd really like to move forward with my next PR's, and it is rather difficult to keep rebasing these on each other so that I can use all my unmerged features.

Teaser; my airline is configured to show a quick status summary so that I know if I am good to switch branches; that will be the next PR.

[TamaMcGlinn]

Any update on this?

[rbong]

Sorry, I haven't had any time to work on my side projects lately. Thing have been crazy at work.

I still want to test this, there's a couple things I want to try out. Also, I haven't decided yet if I'm comfortable with binding hjkl by default to this navigation. I think that might be a little too opinionated.

[TamaMcGlinn]

Then add let g:flog_no_default_mappings = 'no thank you' to your vimrc, as the EXAMPLES says. I should cherry-pick that change over to this PR, assuming you are going to merge this eventually.

[TamaMcGlinn]

What are the things you still want to try out?

[rbong]

I wanted to test out performance on large repos and with different commands. I also wanted to test different merge types and other edge cases.

I apologize again for my inactivity on this issue. Now that I have a little more mental bandwidth and have had time to think it over, I have more feedback, and I additionally apologize for burdening you with more of my opinions on this issue. Let's get to it; unfortunately I have decided to categorically reject these changes for reasons I outline below.

I considered making it up to you by trying to find a way to change Flog to accommodate this kind of feature, but this requires even more mental bandwidth. This has contributed in part to my silence, and I have decided now that it's best just to bite the bullet and give you my opinion and let you do with it what you want.

You are free, of course, to disregard my opinion and continue maintaining your own branch or an extension plugin if you wish.

The reason I feel I have to reject these changes at this time:

In a "real" visual Git client, where we're building the graph, we already know the structure of parents and children. Flog is not a real visual Git client. It relies on git log or git-forest or similar to parse and output the branch structure for us.

One of the issues is that it's expensive to get the commit graph structure in large repos, especially in vimscript, but the main issue is that it's the way this is implemented now, it's disconnected from the visual structure of the commit graph. There will be a visual disconnect between the child that we're jumping to and how it appears on screen based on how commits are sorted.

Even if the system of how we build the internal structure of parent and child commits was more robust - do we really want this? Flog then becomes a plugin that both relies on external plugins to build the commit graph, AND it builds its own internal commit graph that it has to keep in sync.

I am worried that these changes will balloon far out of control, both from a perspective of future maintenance and precedent.

Because of all these factors, I have made the difficult decision that, unless if Flog changes to a plugin that builds its own commit graph, I have to categorically reject all changes that rely on finding the parent/child commit graph structure.

I do think in the future, eventually there should be a "real" graphical Git client for vim/neovim. I'm still not sure if that's going to be Flog. I am leaving this open on the chance that someday Flog is ready for these changes, though they will probably be re-implemented.

I know that you probably don't want to hear that your commit is getting rejected for opinionated theoretical reasons after all this time, but I hope that there is some relief in finally having some kind of closure on this PR.

[TamaMcGlinn]

Okay; thank you for writing that. I think I will split this PR into two, because it seems all your objections are on the side of finding parent/child commits - and if I split that aspect out into a separate plugin then it will be much easier to stay in-sync. The other side of this current PR is that if you press j/k then you move to the next/previous commit in the buffer, thus skipping over lines that only contain symbols. However, when you specify a count then it falls back to the normal behaviour, so that commands based on the relative line numbers still work as expected.