abapGit icon indicating copy to clipboard operation
abapGit copied to clipboard

Suggestion for Improvement in Diff View (Unified and Split)

Open francisco-milan opened this issue 8 months ago • 3 comments

Hi guys,

I noticed something that seems a bit unclear to me when comparing the version of a program in my system with the one in the Git repository. I'm hoping someone could help me understand it better—and perhaps this could also be considered for improvement.

Version of the program in my system:

REPORT zhello_world_v1.

CLASS zcl_hello_world DEFINITION.
  PUBLIC SECTION.
    METHODS: say_hello.
ENDCLASS.

CLASS zcl_hello_world IMPLEMENTATION.
  METHOD say_hello.
    cl_demo_output=>display( 'Hello, World!' ).
  ENDMETHOD.
ENDCLASS.

START-OF-SELECTION.
  DATA(lo_hello) = NEW zcl_hello_world( ).
  lo_hello->say_hello( ).

Version of the program in the Repository:

REPORT zhello_world_v1.

CLASS zcl_hello_world DEFINITION.
  PUBLIC SECTION.
    METHODS: say_hello.
ENDCLASS.

CLASS zcl_hello_world IMPLEMENTATION.
  METHOD say_hello.
    DATA(lv_name) = 'ABAP Developer'.
    cl_demo_output=>display( |Hello, { lv_name }!| ).
  ENDMETHOD.
ENDCLASS.

START-OF-SELECTION.
  DATA(lo_hello) = NEW zcl_hello_world( ).
  lo_hello->say_hello( ).

When I use the Unified view to compare these versions, I see:

In the Header: + 1 (green), - 0 (pink), ~ 1 (yellow) In the Code: 1 pink line with a minus sign ( - ) on the left 2 green lines with plus sign ( + ) on the left

Image

I was expecting something like: Header: + 2 (green), - 1 (pink), and no ~ (yellow)

In the Split view, it looks like this:

In the Header: again + 1 (green), - 0 (pink), ~ 1 (yellow) In the Code : LOCAL side: 1 yellow line with ( ~ ) on the left and 1 green line with ( + ) on the left REMOTE side: 1 yellow line with ( ~ ) on the left

Image

Expectation: I would think this change could be reflected more clearly as 2 added lines (in green) and 1 removed line (in pink), rather than using the yellow color for changes. Header: +2 (green), -1 (pink), and no ~ (yellow)

Would it be possible to improve how these diffs are interpreted and displayed? Or provide some help on how to interpret these colors and header counts correctly?

Also, I think it makes more sense if the version in the system was labeled as "LOCAL", and the version in the Git repository as "REMOTE". This would help make the direction of change more intuitive when reviewing differences.

Best regards, Francisco Milan.

francisco-milan avatar Apr 12 '25 03:04 francisco-milan

In git terms, it's a +2, -1 since there's no "change" in git diff.

You get a yellow "change" in abapGit if there's a + or - on both sides but you only see it in the split view. I think it's a nice feature since you see quickly if where there are slight changes. If there are lines added or removed, diff algorithms have a hard time telling what's changed vs what added (that's why there are many "diffs"). In the example, it could be an added line locally (empty on remote), and a change on the second line. abapGit relies on the diff from SAP which calculates it differently. Nothing we can do about that.

If you want to change the counters in the unified view to just + and -, feel free to create a PR.

It does say LOCAL and REMOTE already. I don't understand what should be changed there.

mbtools avatar Apr 12 '25 09:04 mbtools

Thanks for the explanation, Marc!

I understand now that abapGit uses SAP’s diff algorithm, which behaves a bit differently compared to standard Git. Still, I was expecting the diff functionality in abapGit to behave more like what we usually see in Git tools—where only green and red lines are used to show added and removed lines, and there's no concept of yellow lines to represent "changes."

From a consistency and user experience point of view, I think it would be more intuitive if both the unified and split views followed the standard Git convention of just additions (green) and deletions (pink/red), without introducing the yellow lines. The yellow “change” highlight could be a nice enhancement, but ideally as an optional, configurable feature for users who want that extra layer of detail.

Regarding the header counts, it would also help if they directly matched the actual added (green) and removed (pink) lines in the code. That way, it's easier to get a quick and accurate summary of the changes without needing to interpret extra logic behind the numbers.

Lastly, about the LOCAL and REMOTE labels—I might have missed something, but in the example I shared, the code in my system (what I would call LOCAL) was labeled as REMOTE, and the repository version as LOCAL. That felt a bit inverted to me. I think labeling the system version as LOCAL and the Git repository version as REMOTE would make the direction of changes more intuitive, especially when reviewing and pulling updates.

Thanks again for your time!

Best, Francisco Milan.

francisco-milan avatar Apr 12 '25 16:04 francisco-milan

I would be ok with just red/green as the default, if

  • there's a switch in the view menu to also show yellow (current behaviour)
  • we persist the view setting (actually, it would be good to save all diff view settings)

It can happen that local and remote get switched. There are cases where abapGit is not able to determine which side got changed. For example, a line is removed locally. This could also be seen as an addition remotely. We don't track the history of changes in ABAP so we can't definitely say what came first. abapGit uses checksums to see what changed. It's a known limitation causing issues for example when you switch between origins like branches, tags, PRs, forks and pull from there. Checksums should depend on the origin but that's a big enhancement. Removing this local/remote switch in the diff view certainly requires a deep review as you can see.

mbtools avatar Apr 12 '25 17:04 mbtools