Fix #631 crossing branch lines have the wrong color #945
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue jonas#631 notes that 'When branch lines are crossing each other, the horizontal line has the color of the branch it is crossing (the one going vertically), instead of being the same color that continues after the crossing. It is especially apparent if the line crosses multiple, differently colored vertical lines.' The mis-coloring occurs because of three factors: 1. A crossover symbol shows vertical and horizontal lines, for example, '─│' for UTF-8 line-graphics. 2. A symbol can have one color assignment. 3. Graphs are rendered with vertical lines taking priority over horizontal lines, ie, the vertical line will have the correct color while the horizontal line will be mis-colored. An obvious fix is to simplify the crossover symbol by removing the mis-colored horizontal component, for example, use ' │' for '─│', but the result is unsatisfying because the path in which the horizontal line was part of becomes too broken. The chosen fix is to remove the vertical component instead, ie, use "──" for '─│', and then use the same color as the right-hand neighbour. Another crossover symbol to fix is "─╭"; here we remove the horizontal component and simplify the symbol to " ╭". The resultant vertical path extend further on the screen, and so to the eye, it does not break as much as the horizontal path. A graph using horizontal crossover symbols is quite satisfying and may actually be preferred. We introduce a new toggle option for `commit-title-graph` to enable the user to switch between the old and the new ways. If the command 'set main-view-commit-title-graph = v2-horizontal-crossover' is used instead of the default 'set main-view-commit-title-graph = v2', then the cross-over symbols will give priority to horizontal paths and as a result there will not be mis-colorings. Code changes are as follows. 1. types.h Add an entry to GRAPH_DISPLAY_ENUM to handle a new selection called 'v2-horizontal-crossover' for the toggle option 'commit-title-graph'. 2. graph.c a. init_graph() Call init_graph_v2() with the value of 'v2-horizontal-crossover' (true or false). 3. graph-v2.c a. init_graph_v2() Modify the API table to select handlers for 'v2-horizontal-crossover' or 'v2'. b. graph_symbol_to_utf8_horiz(), graph_symbol_to_chtype_horiz(), graph_symbol_to_ascii_horiz() Declare handlers for 'v2-horizontal-crossover' for each type of line-graphics option. A handler will simplify a crossover symbol to one that can be correctly colored. For other symbols, the handler will fall back to the default handler. c. graph_foreach_symbol_horiz() Declare a handler for 'v2-horizontal-crossover'. It calls graph_symbol_recolor() before calling the default handler graph_foreach_symbol(). d. graph_symbol_recolor() Declare a new function to recolor each symbol of a graph canvas. The function scans the symbol array in reverse direction in order to extend colors for horizontal paths from right to left. e. graph_generate_symbols() The function generates graph symbols for a commit and appends them to the commit's graph canvas. For each symbol, the function assigns an initial color based on a graph column's ID. The code for doing this is rewritten to simplify four distributed lines to one line, making the logic more comprehensible.
|
Any feedback from maintainers on this? I happened to notice the same bug and then found the issues and now I see a fix has been pending for 5 years with no feedback (at least not visible here) as to why it is or is not able to move forward. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #631 notes that
The mis-coloring occurs because of three factors:
An obvious fix is to simplify the crossover symbol by removing the mis-colored horizontal component, for example, use " │" for"'─│", but the result is unsatisfying because the path in which the horizontal line was part of becomes too broken.
The chosen fix is to remove the vertical component instead, ie, use "──" for "─│", and then use the same color as the right-hand neighbour. Another crossover symbol to fix is "─╭"; here we remove the horizontal component and simplify the symbol to " ╭". The resultant vertical path extend further on the screen, and so to the eye, it does not break as much as the horizontal path. A graph using horizontal crossover symbols is quite satisfying and may actually be preferred.
We introduce a new toggle option for
commit-title-graphto enable the user to switch between the old and the new ways. If the commandset main-view-commit-title-graph = v2-horizontal-crossoveris used instead of the defaultset main-view-commit-title-graph = v2, then the cross-over symbols will give priority to horizontal paths and as a result there will not be mis-colorings.The PR is organized into three commits.