-
Notifications
You must be signed in to change notification settings - Fork 369
Implement possibility of choosing git graph rendering algorithm #325
Comments
Hey @marekyggdrasil 👋 I'm impressed by the quality of your research and very detailed issue. I just created an issue to explain explicitly that I won't have time to deal with this, but I'm looking for volunteer to maintain the library: #328 Let me know if that's something you'd be interested in 😉 |
Hi, If you need my code to be under MIT license, I can change the license. And if you need more information concerning the algorithm, I can help. Best, |
Thanks guys! @nicoespeon that is perfectly understandable, I think all I would need from you is basic code review and merging, I'll take care of implementation and make sure tests are passing etc. Regarding your need of help with maintenance, I could occasionally dedicate some time to help you out. @pvigier so glad you joined the discussion! let me just play with your algo a bit and if I have questions I'll surely reach you, thanks for that! Regarding the licence, unfortunately I'm not an expert so I can't comment on that, perhaps Nicolas (or anyone else following this discussion) could give some comments. |
@pvigier @marekyggdrasil regarding the license, we use the MIT license too. It's all good 👍 I'd like we credit Pierre's work in the README if we use his algorithm, so it's fair recognition 😉 |
Glad the licence problem is resolved, and regarding the recognition I agree and I suggest the following:
|
Some analysis. The position of each commit depends on gitgraph.js/packages/gitgraph-core/src/gitgraph.ts Lines 376 to 381 in a4a26a2
For each commit gitgraph.js/packages/gitgraph-core/src/gitgraph.ts Lines 369 to 372 in a4a26a2
And objects that provide them ( gitgraph.js/packages/gitgraph-core/src/gitgraph.ts Lines 364 to 368 in a4a26a2
I plan to replace it with a single object called The entire layout is computed when gitgraph.js/packages/gitgraph-core/src/gitgraph.ts Lines 224 to 229 in a4a26a2
I will replace this with computing the ( no need to comment on that, although comments are always welcome, I'm just used to use issues as notepad, it might be somehow useful for future contributors ) |
Hi, is this still being worked on? Or is there already a way to create graphs the way as they are discribed in the issue? Greetings |
@TheMightyMic the issue is still relevant, nothing as changed in this direction yet. ping @marekyggdrasil > would you be able / motivated to move on this topic? |
Hi, I'm also interested in such a feature, not a configurable graph-rendering algorithm as suggested by @marekyggdrasil , but at least a fix that addresses the one-column-can-have-only-one-branch problem. Best regards |
Sorry for being silent, guys @TheMightyMic, @nicoespeon and @joliebig, seeing growing interest in this issue is really motivating, let's get back to work and have it done. The biggest challenge currently is including the algorithm designed by @pvigier into the library. The reason why it is challenging is both algorithms operate on graph but differ in data structures and representation. The easiest way to do it would be to rewrite the rendering from The solution I'm proposing is to abstract out the rendering, so that algorithm designed by Pierre could be included in the library without removing the default one, this would not break the backwards compatibility and allow to add customised rendering heuristics in the future. Since there is interest in having it done, I will undust my notes and think about it during next few days, anyone willing to contribute please don't hesitate to reach me. |
Currently
Now I will try to translate Gitamine algo, this is the part that computes positions and include it here, this is where it is supposed to be computed |
Sorry it was just the rows... But today I made an update, both rows and columns for each individual commit are computer by a single function now This function can be implemented for different layout rending algorithms, I'll try to translate gitamine algo into that |
For those of you who are looking forward for this feature to be implemented and want to keep track of the progress, feel free to check marekyggdrasil#1 |
Is your feature request related to a problem? Please describe.
I was analysing the problem mentioned in feature request #290 and I realised this is bigger change and demands more discussion and proper planning and design so I decided to open a proper feature proposal issue.
Briefly for those unfamiliar with #290, every time we branch a new column gets assigned to new branch even if there is available space. For projects with many short-lived branches layout grows really large pretty quickly. I prepared an example that demonstrates it pretty clearly:
Describe the solution you'd like
I came across the article commit graph drawing algorithms by @pvigier the developer of Gitamine who written a master thesis about this subject.
It appears like the process of rendering git graph consists of three steps
x-coordinate
of each commits)y-coordinates
)I propose to isolate those steps and implement separate functions for them so that user can select which algorithm would like to use
that gives full customisation regarding the output layout, for less advanced users who would want to replicate the behaviour of their favourite software we could prepare pre-defined layout, each consisting of sorting and column order algorithm and eventual optimisation, such as
would replicate the output as similar to SourceTree as we can reproduce
the disadvantage is, there would be a lot of code to take into account all those algorithms and possibly not enough contributors to implement all of those, so I also suggest and alternative solution
Describe alternatives you've considered
We could also adapt some code from Gitamine which is GPL-3.0 licenced, I briefly researched it and it looks like it is possible to include GPL-3.0 code in MIT licence project as stated here.
My alternative proposal is to still abstract out part of the code that assigns order to columns and keep the
layout
option proposed earlier aswould use the Gitamine-based algorithm and
would render everything exactly as it is now, also
regular
would be the default value so update would not break backwards compatibility and in the future we could consider using different layout as default if community demands it.Additional context
I attach some figures from article by @pvigier, they demonstrate quite nicely the possible variety of layouts.
Also, I'd like to mention that I already forked and briefly researched the source and if there would be positive feedback on proposal I'll be happy to discuss techy details based on my notes and I can take care of the implementation.
I'd like to mention that the adapted algorithm by @pvigier I would commit separately and set him as the author of that commit.
The text was updated successfully, but these errors were encountered: