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.

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

Fix #631 crossing branch lines have the wrong color #945

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stevenyvr987
Copy link
Contributor

Issue #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.

The PR is organized into three commits.

  1. The main commit; code changes are summarized in the commit message.
  2. Update the relevant test suite.
  3. Update the tigrc man page.

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.
@alerque
Copy link

alerque commented Jan 16, 2024

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants