GitGraph: line routing and colouring for branches above main

guypursey commented 1 year ago

Description

Branches render unexpectedly differently when they're positioned above main (as compared with their default position underneath main).

This was found by @ijmitch during proposed fix for #4912.

Steps to reproduce

Open Mermaid Live

Replace the code with the following

gitGraph
  commit
  branch release-1
  commit

Observe that arrow between first commit and second has colour of new branch and comes straight out of main before joining release-1 with a curve.
Either a. Add following to config option on Config tab: "gitGraph": { "mainBranchOrder": 2}, or b Paste following line above graph code: %%{init: {'theme': 'base', 'gitGraph': {'mainBranchOrder': 2}} }%%
Observe that arrow between first commit and second now has colour of main branch instead and curves out of main before joining release-1 straight on.

Screenshots

Here are examples with simplest possible graph for demonstration.

Example, standard with main as top branch:

Branch arrow goes straight out and curves into branch commit:
Branch arrow takes on colour of new branch

gitGraph
  commit
  branch release-1
  commit

Example, standard with main as bottom branch:

Branch arrow *curves out* and goes straight into** branch commit
Branch arrow retains colour of main branch

%%{init: {'theme': 'base', 'gitGraph': {'mainBranchOrder': 2}} }%%
gitGraph
  commit
  branch release-1
  commit

Setup

Mermaid version: 10.5.0
Browser and Version: Firefox 118.0.1

Additional Context

Assumption: that branch curves should only apply to branches that aren't main.

ijmitch commented 1 year ago

+1 @guypursey

for context, I was trying to write some guidance which is largely aligned to https://learn.microsoft.com/en-us/azure/devops/repos/git/git-branching-guidance?view=azure-devops#use-release-branches and other schemes which advocate release branches and depict them above main (and obviously never merging into it), whereas feature branches are depicted below main and generally do merge into it.

guypursey commented 1 year ago

@ijmitch Thanks for the extra context. With this in mind, do you think the point I've made about the curves still makes sense?

Guess it's a design decision about whether curves and straight lines are a visual shorthand for anything semantically. I haven't found any guidance or documentation on this in the Mermaid repo so far.

ijmitch commented 1 year ago

It would be ideal in my opinion if branching 'upwards' was a mirror-image of the normal 'downwards' rendering. So, yes, I mentioned the colour and was a bit lazy not to mention the curves used as well.

mathbraga commented 1 year ago

Hi @guypursey, are you working on this?

guypursey commented 1 year ago

Hi @mathbraga, yes, I started work on this, as I was looking at those parts of the codebase already and drafting documentation. But I'm waiting to see if #4927 will be accepted and merged as I was building on top of the work in that PR.

mathbraga commented 1 year ago

Yeah I noticed your involvement with #4927, that's why I wanted to know before diving too much into this.

I did dig into this for a bit though, so I'll share some of what I found out that might help.

I believe that one of the ways to solving this would be to identify when a branch is above or below main.

getBranchesAsObjArray in /packages/mermaid/src/diagrams/git/gitGraphAst.js returns only the name of each branch. You can modify this to return also the branch order value.

The second .map inside getBranchesAsObjArray could be changed from:

.map(({ name }) => ({ name }))

to something like

.map(({ name, order }) => ({ name, order }))

Then inside draw in /packages/mermaid/src/diagrams/git/gitGraphRenderer.js you can have access to branches with ordering. Then you can modify drawArrows to also accept branches as input and work from there.

I didn't really give much thought to this so there could be better solutions, just felt like leaving this here as it could help.

mermaid-js / mermaid