renderdag: box: option to pad between nodes from separate branches

[thomasa88]

Makes it easier to visually scan the branches and find the heads quickly.

Before:

│ │ │ o  F some text message
│ ├───╯  that continues on the next line
│ │ │ o  E some text message
│ ├───╯  that continues on the next line
│ │ o  D some text message
│ │ │  that continues on the next line
│ │ o  C some text message
│ │ │  that continues on the next line
│ o │  B some text message
│ ├─╯  that continues on the next line
│ o  A some text message
├─╯  that continues on the next line

After:

│ │ │ o  F some text message
│ ├───╯  that continues on the next line
│ │ │
│ │ │ o  E some text message
│ ├───╯  that continues on the next line
│ │ │
│ │ o  D some text message
│ │ │  that continues on the next line
│ │ o  C some text message
│ │ │  that continues on the next line
│ │ │
│ o │  B some text message
│ ├─╯  that continues on the next line
│ o  A some text message
├─╯  that continues on the next line

[facebook-github-bot]

@thomasa88 has updated the pull request. You must reimport the pull request before landing.

[meta-codesync]

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D89118021. (Because this pull request was imported automatically, there will not be any future comments.)

[quark-zju]

Review automatically exported from Phabricator review in Meta.

[quark-zju]

Sorry for the slow review. In your example it seems inconsistent that there are no spaces between (A, B), or (D, C) but there are spaces between (E, F), (B, C), and (D, E).

If you want more spaces, have you tried appending new lines \n to "that continues on the next line", or use the with_min_row_height(3) API?

[quark-zju]

Review automatically exported from Phabricator review in Meta.

[thomasa88]

Sorry for the slow review. In your example it seems inconsistent that there are no spaces between (A, B), or (D, C) but there are spaces between (E, F), (B, C), and (D, E).

If you want more spaces, have you tried appending new lines \n to "that continues on the next line", or use the with_min_row_height(3) API?

Hi, DC is in fact correct as it is. It is more apparent if there are more nodes. All nodes belonging to the same branch get grouped and spacing is added between nodes from different branches. If you look at the nodes (o) you see that they are on the same branch.

Looking closer, I realize that AB should likely have the spacing, since even though they are on the same vertical line, the branch between A B should likely make them be considered belonging to different branches. 🤔 . I will have to dig in to the code a bit and see if I can fix it.

[quark-zju]

All nodes belonging to the same branch

I see. I'm not sure renderdag is the best place to implement this. Could you check from the call-site and pass an extra newline as commit message as needed? e.g. if a commit's parent has multiple children, in your example, F, E, C, B, then just make their commit message longer by appending a new line.

Also note there is a "shorten graph" mode inherited from https://repo.mercurial-scm.org/hg/rev/ac30adb260ea for compact output. We control this mode via with_min_row_height. So unconditionally adding spaces will break users who want the compact output.

[thomasa88]

I think I ended up adding the implementation into renderdag instead of the call-site (Jujutsu), as the direct caller only has information on each node's parent edges and not information on which branch a node belongs to or the number of children, as it does not, from what I remember, build the graph. Breaking other render options does not sound good 😅 .

I will look to see if I can pass more information to the caller in some good way.

[quark-zju]

Got it. If you change renderdag could you make it customizable? This way we can keep current behaviors unchanged.