Use top bottom and middle commit symbols #125
Conversation
|
I tried this style of commit character while I was writing v2, but for me, it's way too hard to visually identify the horizontal position of commits on branches quickly in large graphs, and I find coloring these characters white doesn't help with legibility much and is quite ugly personally. (The choice to use the circle character for commits also lead to the decision to put a space between disconnected commits - I just wanted to make you aware of this, but you can choose to include spaces or not with these changes.) Because this is a change to the graph drawing function and I don't have a lot of interest in this myself, I have to ask for a couple things before I approve this PR:
Since the graph drawing function is optimized and isn't really designed for easy maintenance, it can be hard to make changes to just one spot without impacting other things. Tests are important for all new graph drawing functionality to make sure everything works and continues to work when future changes are made. I would like to see these test cases:
(Actually, your changes as they exist right now will break tests I believe. Not sure why pipelines aren't catching that.)
Measure performance with: Please use a reasonably large repository with complex branches (I think I use either the Linux or Git repo typically) and give your recorded time before/after the changes with the setting off (you can test with it on if you'd like, but my primary concern is the default behaviour). I have no preference if you use VIM or nvim, as long as you use LuaJIT. The graph drawing function is hyper-optimized - if any changes are made to the function it needs to remain hyper-optimized, so performance needs to be checked. Sorry, I know tests and performance comparisons are annoying, but this is the price for an optimized custom graph drawing function. I should warn you as well... after you make these changes, I can't guarantee I won't have more feedback, although I hope I don't. |
|
Great to see such dedication to keeping flog high-performance and fault-free. I think these things will take me a long while, but I don't mind that. Thank you for including a lot of specifics so I know where to start. |
I like to use ┌ ├ └ for the commits, because it gives
me lines to follow visually. With flog v1 this works:
Plug 'rbong/vim-flog', { 'branch': 'v1' }
Plug 'TamaMcGlinn/flog-forest', { 'branch': 'v1' }
But with v2 the ability to customize the forest used in the background
was lost, so I have re-added this by checking if the commit has child
commits already drawn, and whether it has children that will later be
drawn, and still falling back to the • if neither (disconnected
commits).
The change to the MergeStart line just makes it a little more strict,
as it must be followed by a non-space character such as ──╮.
Otherwise, it thinks each middle-commit line is actually a
mergestart line and those commit lines become all-white.
TamaMcGlinn
force-pushed
the
feature/top_bottom_middle_commit_symbols
branch
from
3e8484a
to
c0dca86
Compare
I like to use ┌ ├ └ for the commits, because it gives me lines to follow visually. With flog v1 this works:
Plug 'rbong/vim-flog', { 'branch': 'v1' }
Plug 'TamaMcGlinn/flog-forest', { 'branch': 'v1' }
But with v2 the ability to customize the forest used in the background was lost, so I have re-added this by checking if the commit has child commits already drawn, and whether it has children that will later be drawn, and still falling back to the • if neither (disconnected commits).
The change to the MergeStart line just makes it a little more strict, as it must be followed by a non-space character such as ──╮. Otherwise, it thinks each middle-commit line is actually a mergestart line and those commit lines become all-white.
Here's a video of me switching between this commit and its parent. I have checked and this works without any merge conflicts on current rbong/master. I have also been using this since the commit date one year ago, across a wide variety of repos, including very large ones, with no issues whatsoever.
flog_top_bottom_middle.webm