Repository navigation
elk: expose edgeEdgeBetweenLayers spacing - #2887
Conversation
06c4493 to
55977f5
Compare
jonapoul
changed the title
There was a problem hiding this comment.
Found one compatibility regression for existing Go API callers: omitting the newly added option changes edge spacing from 50 to 0 and causes unrelated edge segments to overlap. Please preserve the previous default for callers that do not explicitly configure the new option.
Validation at 55977f5: native ELK and CLI test suites passed, selected rendering tests passed (sanity, stable, root, unicode, and themes), and targeted WASM option/profile tests passed. The regression was confirmed by running the same Go program against the PR head and the merge-base layout implementation.
- sent from alixander's Codex
EdgeEdgeSpacing is a *int, so nil means unset and resolves to the 50 D2 sent before the option existed, while an explicit 0 still reaches ELK.
Returning the caller's pointer shared storage with DefaultOpts, and Layout unmarshals ELK output back through it, racing across concurrent layouts.
alixander
left a comment
alixander
left a comment
There was a problem hiding this comment.
Both review findings are resolved: omitted spacing preserves the previous value of 50, explicit zero is honored, and each layout receives its own copy of the spacing value. No further issues found in the final diff.
The final layout implementation matches the local fix that passed the earlier parallel race test. A fresh test run was blocked during compilation by insufficient local disk space.
- sent from alixander's Codex
2 participants
ELK's
edgeEdgeBetweenLayersspacing is hardcoded to 50, so there's no way to change it.Every edge between two layers gets its own routing slot, and each slot adds this much space. So the more links you have between two layers, the further apart ELK pushes them. Lowering
--elk-nodeNodeBetweenLayersdoesn't help, because it only sets a minimum. Especially noticeable in tall charts like #1221 (comment).This adds a
--elk-edgeEdgeBetweenLayersflag, next to the other four spacing flags. The default stays 50, so nothing changes unless you pass it. ELK's own default is 10, for reference.Related to #1221.