On Wed, May 28, 2008 at 02:24:05PM +0300, Teemu Likonen wrote: > Sometimes "log --graph --pretty=oneline" prints a sort of broken graph > line. In the git repository try this: > > $ git log --graph --pretty=oneline --abbrev-commit -4 8366b7b > > [current graph output removed] > > Extra spaces there. I don't mind that myself but to some users it may > look like a bug. Maybe one would expect output like this: > > > M 8366b7b... Merge branch 'maint' > |\ > | M a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint > | |\ > | | \ > | M \ 93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint > | |\ \ > | | \ \ > | M \ | 6abf189... Merge branch 'maint-1.5.4' into maint > | |\ | | > > It requires more lines though. Hmm. Yes, the output could definitely be improved. This problem occurs for merges with 2 parents (not octopus merges), and only when the branch line to the right of the merge was moving right at the end of the previous commit (i.e., it was displayed with '\' instead of '|' or '/'). Note that this doesn't require extra lines to fix: M 8366b7b... Merge branch 'maint' |\ | M a2f5be5... Merge branch 'jk/maint-send-email-compose' into maint | |\ | M \ 93c7b9c... Merge branch 'hb/maint-send-email-quote-recipients' into maint | |\ \ | M \ \ 6abf189... Merge branch 'maint-1.5.4' into maint | |\ \ \ This can easily be implemented by changing the output for 2-way merges from something like this: | M \ | |\ | to this: | M \ | |\ \ However, I find this makes the graph slightly uglier when the incoming branch to the right of the merge wasn't '\' on the previous line. The following change seems to look better when the branch line was '|' or '/' on the previous line: | M | | |\ \ For comparison, here's a comparison of several scenarios of how the output looks now, and how it would look with these fixes. Current behavior Option 1 Option 2 | |\ | |\ | |\ | M \ | M \ | M | | |\ | | |\ \ | |\ \ | | | | | | | | | | M \ | M \ | M | | |\ | | |\ \ | |\ \ | | / | | / | | / | M \ | M \ | M | | |\ | | |\ \ | |\ \ | | |/ | | |/ | | |/ | M \ | M \ | M | | |\ | | |\ \ | |\ \ Note that in the case of octopus merges, the current code already produces output like that of option 1. However, I find that both options 1 and 2 look a little uglier than the current behavior when the branch lines need to be collapsed again after the merge. The graph has more pointy angles, but it is still readable in all cases: Current behavior Option 1 Option 2 | * | | | * | | | * | | | M \ \ | M \ \ | M | | | |\ | | | |\ \ \ | |\ \ \ |/ / / / |/ / / / |/ / / / For sections of the graph where there are several merge commits in a row, I think option 1 looks the best: Current behavior Option 1 Option 2 * | * | * | M \ M \ M | |\ | |\ \ |\ \ M \ \ M \ \ M | | |\ | | |\ \ \ |\ \ \ M \ \ \ M \ \ \ M | | | |\ | | | |\ \ \ \ |\ \ \ \ What is everyone's preference between the 3 options? Personally, I'm leaning towards Option 2. I'll send out some informal patches of both option 1 and option 2, for comparison. In the future, the code could even be improved to dynamically choose between these three options based on the output printed for the previous commit. Currently it doesn't store enough information from the previous commit to do this. -- Adam Simpkins adam@xxxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
