mcdiff color weirdnesses

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/3759
Reporter	egmont (@egmontkob)

Please see the attached screenshot which was taken using a modified version of the gray skins, namely:

[diffviewer]
    added = ;green
    removed = ;red
    changedline = ;magenta
    changednew = ;yellow
    changed = ;cyan

Note that if a hunk contains solely addition or solely removal of lines then the first two colors (green and red) are used; if a hunk contains both addition and removal then the last three colors are used. I couldn't find any other possibility, but maybe I'm missing something.

My problems are, in no particular order:

• I don't see a valid reason why the red and cyan lines are treated differently (different color, as well as different symbol: '-' vs '*'). They both represent the very same thing: a line that does not exist in the given file, but was visually inserted to stretch the file purely for the purpose of keeping the lines in sync with the other file. I recommend a simplification so that the same color, and the same '-' sign is used for both...

• ... although I'm wondering whether something else, maybe just nothing instead of the '-' sign would be even better. In unified diffs the minus sign represents lines that are removed, but are followed by those actual lines. This two-panel view is a completely different representation, here the lines are empty, and no, such empty lines were not removed from those files. So the minus sign can actually be misleading.

• The name "changed" (cyan on the screenshot) is a terrrrrrribly bad name for this purpose. If this name is used at all, it should be used for what is "changednew" now.

• In fact, the name "changednew" is weird too, I mean it's a bit redundant. I do recommend to rename it to "changed"... but wait...

• I don't see any reason why "changednew" (yellow on the screenshot) is allowed to be different from "added" (green). Both just show added content, either parts of a line, or an entire line, but why treat them differently? Actually line 12 on the left panel is yellow, not green, so even if we wanted to display entire new lines differently than parts of a line, it's currently done incorrectly anyways. So my proposal is to merge the two and use the color "added" for both.

• Lines 10 and 11 on the left panel go in purple color beyond the end of the text, line 12 goes in yellow. This looks weird. Of course lines 10-11 would look stupid if they switched to yellow at the end of the data, so instead line 12 should be changed to switch to purple. As per the previous bullet point's unification, I guess this same thing should apply for lines 2-3 (newly added ones) too. Which will have the advantage that trailing spaces become visible.

• Getting back to naming, "removed" is pretty bad too. It does not mark lines that are removed because removed lines are never displayed in that panel. They are only displayed in the opposite panel as "added" ones. I think "gap" or something similar would be a better name.

What do you guys think?

Please note that I've actually never used mcdiff (other than as necessary for creating skins) and I'm not familiar with visual diff tools (tkdiff, meld, kdiff...) either, so I'm not aware of common practices and such. Let me know if my proposal goes against them.

Note that the currently work-in-progress Seasons skins (#3724) will in the next version demonstrate the proposed merging of colors, that is, will use only 3 distinct colors instead of 5 (in addition to the standard background color): one for added content, one for highlighting lines that contain added content (so that they're prominent even if scrolled out horizontally), and one for the gap.

Note

Original attachments:

• mcdiff-3759-colors.png (raw) by egmont (@egmontkob) on Jan 21, 2017 at 0:14 UTC
• a.txt (raw) by egmont (@egmontkob) on Jan 21, 2017 at 0:15 UTC
• b.txt (raw) by egmont (@egmontkob) on Jan 21, 2017 at 0:15 UTC

[mc-butler]

Changed by egmont (@egmontkob) on Jan 21, 2017 at 0:14 UTC

• Attachment mcdiff-3759-colors.png (raw) added

Screenshot

[mc-butler]

Changed by egmont (@egmontkob) on Jan 21, 2017 at 0:15 UTC

• Attachment a.txt (raw) added

[mc-butler]

Changed by egmont (@egmontkob) on Jan 21, 2017 at 0:15 UTC

• Attachment b.txt (raw) added

[mc-butler]

Changed by egmont (@egmontkob) on Jan 21, 2017 at 10:10 UTC (comment 1)

Another possible direction is to make more distinctions, e.g. introduce new colors for:

• spaces past EOL for left panel lines 2-3 (currently green on the screenshot),
• spaces past EOL for lines 10-11 (currently purple),
• spaces past EOL for line 12 (currently yellow),
• the actual line 12 (currently yellow) (a newly added line within a hook that contains changed lines as well).

This way at least we could modify the current skins in a way that does not result in user visible change. Currently merging colors would result in a different look. Plus, skin authors would have absolutely full control over how to highlight diffs.

I still don't understand though why the following two are handled conceptually totally differently, and are marked with different symbols ('+' and '-' versus '*') and different colors:

• 10 lines added,
• 10 lines added and 1 removed.

In my opinion the 10 added lines should not be marked differently in these two cases. But maybe someone who regularly uses this tool has a good explanation.

Note that this ticket is IMO of very low priority. At least much less important than another skin-related issue in #3160.