Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default commit order breaks if timestamps out of order #238

Closed
BenBergman opened this issue Feb 2, 2014 · 4 comments
Closed

Default commit order breaks if timestamps out of order #238

BenBergman opened this issue Feb 2, 2014 · 4 comments

Comments

@BenBergman
Copy link
Contributor

While working on the new graphing code (#144), I noticed that there are some situations in which the commits are not listed in the order I would expect. Specifically, if the timestamp of a parent commit is later than a child commit (which could easily happen if the two commits were done on different machines with different clock settings), then the parent commit could be displayed above the child commit. This breaks a fundamental expectation in the graph code logic.

I made a simple example repository to highlight this condition: https://github.com/BenBergman/tig-commit-order-edge-case

Using the current tip of my fix-graph branch (BenBergman/tig@dd7c263)

git_dates 0 * % git log --pretty=raw --parents | ~/git/tig/tools/test-graph
●─╮ Merge branch 'feature_branch'
● │ More master
│ ● More featuresA
│ ● Add feature
●─│─╮ Merge branch 'feature_branch'
● │ │ Add to master
◎─╯ │ init

Looking at the man page for git log, it seems the --date-order flag fixes the issue.

git_dates 0 * % git log --date-order --pretty=raw --parents | ~/git/tig/tools/test-graph
●─╮ Merge branch 'feature_branch'
● │ More master
│ ● More featuresA
●─│─╮ Merge branch 'feature_branch'
│ ●─╯ Add feature
● │ Add to master
◎─╯ init

I tried adding --date-arder to every instance of "log" I could find in tig.c, but this didn't seem to help.

I've not rebased onto master in a while, so I will try that soon.

@jonas
Copy link
Owner

jonas commented Aug 13, 2014

Looks like this is what was reported as Debian Bug #757692

The easy fix is to just assume (or force) --topo-order and optionally also accept --date-order when graph rendering is enabled. The --topo-order flag was removed by default because it slows down rendering on large repos.

@jonas
Copy link
Owner

jonas commented Aug 13, 2014

Note that git-log --graph implies the --topo-order option by default, but the --date-order option may also be specified.

@BenBergman
Copy link
Contributor Author

I think that the debian bug is more of a reference to the way branches and merges are drawn. We discussed this a bit during the rewrite, around here: #144 (comment)

Regarding this ticket, setting topo-order does indeed fix it. If that is what git-log does by default (and I'm fairly sure other tools like gitk do as well) then perhaps it makes sense for tig to do the same.

@BenBergman
Copy link
Contributor Author

This seems to be related to #300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants