Compare to last Save: always place original file at bottom

[Yaron10]

Hello Pavel,

The file at top is considered the old version and is compared to the new one at bottom (i.e. the "Line deleted" marks are always at top, and the "Line added" marks are at bottom).

Obviously the correct arrangement is up to the user and we can't control it.

However, in "Compare to last Save" we know that the original file is the new version.
I think, therefor, that if we start in One-View mode we should make sure that the original file is always placed at bottom.

That means checking if "currentView == 1" and make the appropriate adjustments.

Thank you.

Best regards.

[pnedev]

Hello Yaron,

I really don't know what you are talking about at the moment :) but I'll get to that part of the code soon and all will become clear. Thanks.

BR

[Yaron10]

Hello Pavel,

When you get to that part please ping me and I'll try to make myself clearer. :)
Thank you.

BR

[Yaron10]

Hello Pavel,

After the comment in bcfcea5, this is somewhat irrelevant.
Still, let's wait. :)

Thank you.

BR

[pnedev]

Hello Yaron,

I'm working on a complex rework now that will affect this behavior so this may become irrelevant.
This concerns bcfcea5 also.

PS. I'm not having much free time this week so it's going to take a while.

BR

[Yaron10]

Hello Pavel,

Best of luck. I'll be looking forward to it.
And many thanks again.

Best regards.

[pnedev]

Hello again Yaron,

I've made a preliminary commit to master that is the re-implemented compared files handling.
There are still things to fix/implement (compare to last save/git/svn are not working, compared file save will be a mess, so don't do it!) - I'm currently working on those.
Could you meanwhile re-test the issues you reported so far (at least the relevant ones) and update those (close the ones that are not seen anymore, etc.)?

Thanks.

BR

[Yaron10]

Hello Pavel,

I've had a busy day and I downloaded the updated repo a short while ago.
I highly appreciate it.
I'll thank and compliment you properly once I have learned and tested this amazing work more thoroughly. :)
I suppose I'll be able to do that on Friday. My sincere apologies.

Some short comments after a quick test:

1. Issue Compare results are cleared when a THIRD file is closed #18 seems to be solved and I'll close it. 👍
2. I often compare the same files again (without Clear Compare) , and I think it's important the user should be able to that without a prompt.
3. If the user is trying to compare an already compared file with a third non-compared file - how about ...Would you like to clear the current Compare?
4. IMO it's highly important to do handle "current-file-at-top" as consistently as possible (i.e. if we Compare in a Single-View mode and the view is 1, the current file is placed at bottom).

Best regards.

[pnedev]

Hello Yaron,

I suppose I'll be able to do that on Friday. My sincere apologies.

No problem, whenever you have spare time, we are not in a hurry :)
I have closed the issues that fail to reproduce.

I often compare the same files again (without Clear Compare) , and I think it's important the user should be able to that without a prompt.

That sounds reasonable. Anyway, I'll prompt (with default to 'yes' - re-compare) if the re-compare was deliberate just in case you triggered it by mistake (some files may need much time to compare).

I'll think about suggestion 3) - at first it seems a bit heavy to implement but if it is not so I'll do it.

About 4) - it doesn't seem that important to me, but I'll consider it anyway.

Thanks for the valuable feedback.

BR

[pnedev]

One update:
4) is implemented in the latest master branch commit. Enjoy! ;)

Can you explain further the idea behind the current issue when you have time?
Thanks.

BR

[Yaron10]

Hello Pavel,

When viewing a commit in a split mode on GitHub, the deletions are in the left pane and the additions are in the right one.
Why?
The left pane is the old version compared to the FOCAL new version in the right pane.
"Line deleted" in the OTHER pane. "Line added" in THIS pane.

In principle this is true in Compare Plugin as well: the file containing the "Line added" (interestingly the sub-view) marks is the FOCAL one.
I don't know if the original developer thought about all this. :) I suppose that most users are not aware of it; they just compare two files.

Anyway, "current-at-top/left" is also current gets the "Line deleted" marks and the other gets the "Line added" marks (as in GitHub).

In "Compare to last Save" the current original file is certainly the NEW version compared to the OLD version before the recent changes were made.
So it should be in the right (bottom) view.

If we ignore the "Rotate to Right/Left" issue, it might still be relevant.
But there some more important tasks. :)

I'll open new issues for the other topics discussed here.

Thank you.

Best regards.

[Yaron10]

Hello again,

I'll think about suggestion 3) - at first it seems a bit heavy to implement but if it is not so I'll do it.

If it's too complex gust forget it. :)

One update:
4) is implemented in the latest master branch commit. Enjoy! ;)

Thank you. I appreciate it.

Best regards.

[pnedev]

Hello Yaron,

Thank you for the clarifications. I'll have in mind that when implementing the other compare modes.
Now when you mention it, the '+' and '-' marks are a bit strange to me. I think that the 'focal' file as you call it shouldn't have '-' marks. For example if I want to compare file A against file B the file A would be the 'focal' one or put in other words - the origin for the compare. So when I look at the results and I see lines with '-' marks I kind of interpret it as "those lines are deleted compared to the other file" which is obviously wrong. Those should be '+' marks saying "those lines are added compared to the other file".
What do you think?

BR

[Yaron10]

Hello Pavel,

Let me describe the "evolution of my thoughts" about this issue.
Please read carefully. :)

When I first started analyzing it I thought the best approach would be treating both files as equal in terms of "importance". That is: no primary vs. secondary (or focal vs. sub) but just two files.

Practically this would mean the following:
The two compared files can have both the "+" and the "-" marks.
If a line appears in File A and is missing in File B, mark it with "+" in File A and add a red blank line marked with "-" in File B.
If a different line appears in File B and is missing in File A, mark it with "+" in File B and with "-" in File A.
(No more gray blank lines).

And the interpretation is simple.
"+": line existing in this file, missing in the other.
"-": line missing in this file, existing in the other.

But then I thought that people were used to comparing primary vs. secondary (if we take the split view on GitHub as an example).
And if so, it's actually not primary vs. secondary but rather secondary vs. primary or old in the left vs. new in the right (think about it as an ascended order: left == old, right == new).
Now you might argue: the current file is primary / focal and therefore should be placed in the right view (again: right == new == primary).
Well, in a normal workflow in the real world this wouldn't always be the case. The user might open his new (primary) file first, then the old (secondary) file and press Compare.
(In "Compare to last Save" we know the current file is new (primary), and therefore I suggested to place it in the bottom view).
You can go on and ask: why not assume the current file is new (primary)?
Forget for a moment the previous analysis and what you know about MAIN_VIEW and SUB_VIEW. If you asked the average user which pane is primary, they would surely answer "left" - which means the current file should be placed in that view.

Conclusion.

You can change the current behavior in three ways:
Place the current file in the sub-view (with the "+" marks) and ignore the average user's expectation to see it in the main-view. (Personally I wouldn't like that).
Slightly deviate from the GitHub pattern, and change the code so that the main-view would have the "+" marks. (Not bad IMO).
Completely deviate from the GitHub pattern, and implement the "two equal files" concept. (Personally I would find that most convenient).

What do you think?

Thank you.

Best regards.

[pnedev]

Hello Yaron,

I'm currently unable to read that carefully ;) so it will wait for a while.
I'll surely come back to this soon, thanks for the detailed explanation.

As a matter of fact my first user experience with Compare plugin was not quite good exactly because of the marks. Those didn't seem intuitive to me at all - I just didn't "feel" them well after being using BeyondCompare for quite some time now.

BR

[Yaron10]

Hello Pavel,

As a matter of fact my first user experience with Compare plugin was not quite good exactly because of the marks.

I'm really glad you feel that way.
I think this is the most important issue In the Compare Plugin.
It would be great if you don't find a major downside in the "two equal files" concept. It seems so "right".

Thank you very much.

Best regards.

[pnedev]

Hello Yaron,

I think this is the most important issue In the Compare Plugin.

I couldn't agree more. It's a compare program after all (or should I say "before everything else").
Will review this issue soon.

BR

[Yaron10]

Hello Pavel,

Great. Thank you very much.
Obviously this issue might affect many other parts/features.

BR

[Yaron10]

Hello,

An illustration to the "PlusMinus in BothFiles" concept (formerly "Two Equal Files" :) ).

Current imlementation:

PlusMinus in BothFiles:

The order of the marks still depends on current vs. second.
Seeing the "implementation", it requires some more thinking.

Best regards.

[pnedev]

Hello Yaron,

Thanks for the visualization.
+/- in both files seems a bit too much to me. A lot of marks and colorization that seems confusing to me. Besides it does not remove the blank lines actually. It just adds +/- mark to them and changes the color to green/red instead of grey.

I would go for the case of having only '+' signs in the first selected file and '-' in 'compared-against' one.
It is the case of all commercial compare programs out there (I think).
It is your second suggestion I think.
I hope you agree, thanks again.

BR

[xylographe]

Besides it does not remove the blank lines actually.

Precisely. These lines are alignment gaps due to enabled "Align Matches". They should be grey.
Also, the + in front of line 2 on the left is confusing, because this line was replaced with line 3 on the right. In diff -u format:

  Pavel
- xylographe
+ Compare
  Yaron

BTW Is there an easy way to change the grey colour (a little less dark would suit me better).

[pnedev]

Hello @xylographe ,

Can you suggest RGB values that suit you? I'll try to see how those look. Thanks.

BR

[xylographe]

@pnedev
Sorry, Pavel, I did not intend to make a request to change the colour for everyone.
I only want a slight adjustment for myself (in a local 'custom' branch) but couldn't find the appropriate source file.

[pnedev]

Perhaps I could add an option for the blank lines color as well (together with the other color settings). Do you think it is justified?
Meanwhile you can look into NPPHelpers.cpp, line 173 - Settings.ColorSettings.blank = r | (g << 8) | (b << 16);

BR

[xylographe]

Thank you**!!**
I've been _grep_ping for "gap", "empty", etc., but, of course, it is called "blank". Why didn't I think of that? :)

Adding the blank line colour to "Options > Color settings" would be very nice. AFAICT it's the only colour missing. On the other hand, ColorSettings.blank is now a calculated value depending on editor background colour. I think, many users might prefer it that way. Hence, this would require another option: dynamic or fixed blank line colour (a check box to the left of the colour chooser perhaps).

[Yaron10]

Hello Pavel and xylographe,

+/- in both files seems a bit too much to me. A lot of marks and colorization that seems confusing to me.

Indeed. When I saw the result I thought so too.

Besides it does not remove the blank lines actually.

That's correct.

These lines are alignment gaps due to enabled "Align Matches".
Also, the + in front of line 2 on the left is confusing, because this line was replaced with line 3 on the right. In diff -u format:

If this is implemented "Align Matches" should be the only option.
I was not aware of the diff -u format.

It is the case of all commercial compare programs out there (I think).

That's also a good point.

Some good Cons so far.
But let's think of a major Pro: you get effective/correct results regardless of old/new, original/changed, primary/ secondary etc..
IOW, you don't need to "calculate" which file to activate when you press Compare.
Also, the user might want to compare two versions (of a short story for example) without knowing himself which was created first.

I'm not sure about the conclusion myself. But I do think it's worth considering.

---

Pavel,

If the current concept of old vs. new prevails, allow me to change my mind regarding the position of the files. xylographe has converted me (thanks). :)

When we discussed "current vs. previous" or "current vs. next" my argument was the workflow: you have some files open, you then open two new files and Compare.
It's true there's no notation as to which is old and which is new, but in a normal workflow it's more likely that you'd open the old file first.
(When a new Firefox version comes out and I want to compare some source files, I tend to open the folder containing the old version first).

So left == old, right == new.
And it would be consistent with "Set First" and "Compare to last Save" (and GitHub :) ).

Sorry for the confusion.
What do you think?

---

There used to be a setting for Blank Lines background color.
UFO removed it at some point.

Thank you very much.

Best regards.

[Yaron10]

Hello again,

Anther reason why xylographe is right: we compare current to previous; so before Compare current was on the right and the other on the left (unless first tab is active etc.).

I usually use horizontal split (top/bottom) so it's not that obvious.

Regarding the focus: I think we should leave it with the current file on the right.

Thank you.

BR

[pnedev]

Hello @Yaron10 and @xylographe ,

I fixed that in master branch. Please read #45 latest post for details.

BR