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.

Sign up for GitHub

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

Jump to bottom

Merge commit loses graphical "angle" if another branch starts from it #419

Closed
minaguib opened this issue Jun 12, 2015 · 4 comments
Closed

Merge commit loses graphical "angle" if another branch starts from it #419

minaguib opened this issue Jun 12, 2015 · 4 comments

Comments

@minaguib
Copy link
Contributor

Currently looks like this:

2015-05-28 13:08 Mina Naguib             │ ● commit2 feat2
2015-05-28 10:47 Mina Naguib             │ ● commit1 feat2
2015-05-26 13:32 Mina Naguib             ●─╯ Merge branch 'feat1' into production
2015-05-25 16:56 Mina Naguib             │ ● [feat1] commit2 feat1
2015-05-25 16:52 Mina Naguib             │ ● commit1 feat1

I'd expect logically something like this:

2015-05-28 13:08 Mina Naguib             │ ● commit2 feat2
2015-05-28 10:47 Mina Naguib             │ ● commit1 feat2
2015-05-26 13:32 Mina Naguib             ●═╡ Merge branch 'feat1' into production
2015-05-25 16:56 Mina Naguib             │ ● [feat1] commit2 feat1
2015-05-25 16:52 Mina Naguib             │ ● commit1 feat1
@minaguib minaguib changed the title Merge commit loses graphical "angle" if another feature starts from it Merge commit loses graphical "angle" if another branch starts from it Jun 13, 2015
@SteveLTN
Copy link

SteveLTN commented Sep 8, 2015

I also noticed this problem. For me, it looks like

 M─┐
 │ o
 M─┘
 │ o
 M─┘
 │ o
 M─┘

@quinox
Copy link

quinox commented Aug 25, 2016

This one bit me today, I was about to scold a developer about not merging his branch :)

@halfgaar
Copy link

I have the same issue. I was very confused by the graph, until I opened gitk.

zevv added a commit to zevv/tig that referenced this issue Jun 19, 2017
@zevv
Trying to fix graphs where the same commit is hit by a merge and a new
984031c 
branch, but the graph gives the impression the commit is not merged.

The old case rendered this result. Not that the bottom branch looks
like it terminates without being merged:

 ● Update NEWS with post 2.1.1 changes
 ●─╮ Merge pull request jonas#457 from vivien/text-variable
 │ ● add a %(text) variable
 ●─╯ Merge pull request jonas#439 from peff/fix-blame-args
 │ ● blame: allow user to specify rev arguments to blame
 ●─╯ Update OSX make config to find brew installed ncurses
 ● Remove unneeded calls to {def,reset}_prog_mode

With fix I get this:

 ● Update NEWS with post 2.1.1 changes
 ●─╮ Merge pull request jonas#457 from vivien/text-variable
 │ ● add a %(text) variable
 ●─┤ Merge pull request jonas#439 from peff/fix-blame-args
 │ ● blame: allow user to specify rev arguments to blame
 ●─╯ Update OSX make config to find brew installed ncurses
 ● Remove unneeded calls to {def,reset}_prog_mode

Related to jonas#419

My current solution is not nice: it depends on the order of evaluation in the
graph_symbol_to_XXX() functions so that the new case of graph_symbol_merge_branch()
is hit before graph_symbol_turn_left() or graph_symbol_merge() do.

The graph-generating code is complex, and I have not spent enough time in it to
know for sure my fix does not mess up other situations.
@zevv
Copy link

zevv commented Jun 19, 2017

I too scolded a colleague for not merging, the end result was not pretty.

To make up I'm trying to fix this issue, any feedback appreciated:

zevv@984031c

@zevv zevv mentioned this issue Jun 19, 2017
Closed
@jonas jonas mentioned this issue Jul 4, 2017
Merged
@jonas jonas closed this as completed in 440d03d Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@minaguib @halfgaar @SteveLTN @quinox @zevv