Closed TamaMcGlinn closed 1 year ago
I'm sorry, an early version of Flog did use this style of commits, but ultimately I just find the current style much easier to visually parse, especially in large graphs.
I don't like shutting this down, because one of the original reasons Flog switched to drawing its own graph is for customizability, but this is one area that's aggressively optimized and I have to make a choice one way or another.
For future reference, if you do modify the commit graph drawing function, don't add new functions, just add to the existing one big function. Yes, it's a huge mess, but it's an aggressively optimized mess. Adding a function call is too much overhead for what it's doing generally. This is most important for something that's done every branch, but something that's done on every line is kind of important as well. We're competing with command line Git, where you can see results immediately even if you're looking at 10s of thousands of lines at once. Good job modifying the function other than that though.
We can make this customizable by grabbing the string desired for each from a global, optional setting. Something like this, where your current one is the default and then I could define a separate plugin that just defines these three globals:
local top_commit_str = vim.g['flog_custom_top_commit_str'] or '• '
local middle_commit_str = vim.g['flog_custom_middle_commit_str'] or '• '
local bottom_commit_str = vim.g['flog_custom_bottom_commit_str'] or '• '
In open-source, there is no 'shutting this down' - only a choice between being graceful or obtuse, and by it choosing for either good collaboration and a better end product, or forking and time spent on other workarounds. Your feedback suggests you like the input in principle, but then closing immediately suggests that there is nothing I can change on this PR that will make it acceptable. I'm pretty sure that is the normal way of communicating the change to be irredeemable, is that what you meant to say, or would an aggressively optimized alternative to the function call make this mergeable?
I can understand that it's hard to convey meaning over text so perhaps I'm misreading but if you're implying that I'm being graceless or obtuse I don't appreciate that.
I do appreciate that I did not communicate a clear path to get similar changes in and I was quick to close so I will remedy that, I would ideally like to see:
1) Speed tests before and after with a large repo (ex. the Linux repo with 10k comits) 2) Aggressive optimization (no function calls, this requires stack manipulation which adds additional overhead) 3) This change to be optional (as you've already suggested) 4) Unit tests so that this change doesn't inevitably get mangled, because I'm not going to see it every day so it will be hard to catch bugs
Please understand I have been both sick and busy lately and I can't make any guarantees about my response time. I also can't think of alternative solutions to rejected changes every time right away. Though I'll try not to just close PRs right away in the future. I guarantee I'm not intentionally being obtuse. In this case, I simply thought it would be better for you to have your own fork than go through all this.
If you are willing to put the time in still to implement this change, understanding that it could be hard, that I still have to consider the speed impact, and that I might not be the most responsive, please feel free to re-submit. I can't re-open because the branch was force pushed. I would maybe focus on points 1 and 2 first and maybe 3 and see what the speed impact is like.
Ok. When I get some time I will look into those changes.
I much prefer to stay in-line so that I get fixes and updates (I really liked gP for first-parent - I used that a lot in a previous git GUI), even if it means adding a whole new plugin to override some function. I think this is also an option, regardless of points 1, 2 and 3 - but point 4 is only possible if I integrate into flog, and it would guarantee me stability in future
I'm sorry for hurting your feelings; but you were correctly reading that I find it obtuse to close PRs so quickly with a comment saying you were 'taking this down', because I took it to imply the PR was categorically rejected. I'm glad to hear that my understanding was incorrect. I hope you don't feel pushed to respond quickly to anything - that was never my intent as I have no problem at all waiting weeks for replies. Even though we sometimes get a little emotional over our pet projects, let us remember this is all just the digital world; personal life comes first. With that in mind, I will be praying for your good health.
Keeping in mind personal health is important, I don't believe my mental health would survive all of my decisions on my open source projects being characterized so strongly as to be called graceless or obtuse. In this case, saying you disagree with a decision and offering alternatives is enough to respond to the disagreement with without characterizing it so strongly, and it absolutely was not required for a response you wanted to hear. In fact, it made it hard to respond impartially. I reserve the right to temporarily suspend anyone for similar behaviour on my open source projects, not just towards me, but towards anyone, yourself included. I'm sorry if my original decision made you feel strongly, but I never strongly characterized your changes one way or another, and I won't even venture to think of what I might have said. I'm not sure this is what happened, but if you do feel that closing an issue or PR is an overreaction please remember it's not personal, for example it is not meant as a personal attack or an insult or a strong characterization of your submission even when it's mistakenly done in haste. Sorry if this seems sensitive but this is just the type of open source project I would like to run. I hope that makes sense.
Sure, it makes sense. Sorry again.
As you know, I like to use ┌ ├ └ for the commits in the style of a certain git-forest fork, 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' }
Also, I really like your change where disconnected commits get a blank line. It should also allow me to paragraph-jump with <shift-]> but for some reason this does not work. I also tried and failed to find the source of the single space on that line, as I have a global setting somewhere I cannot find that visualizes that as a little grey dot.
And if commits are disconnected on both sides, I use the circle you have currently in v2:
Now, performance-wise, I have added a map that has each commit in it, and looking up in that map once for every line. I think the effect is negligible, but will see if I can measure that with a few large repo's and let you know.