comparePlus icon indicating copy to clipboard operation
comparePlus copied to clipboard

Compare Selections

Open Yaron10 opened this issue 4 years ago • 10 comments

Hello Pavel,

Tested with the CSS files (from https://github.com/pnedev/compare-plugin/issues/220#issue-549153709).

STR: Show Only Compared Selections: OFF. Select line 7 in userChrome Old.css and lines 1-9 in userChrome New.css. Compare Selections.

Result: No marks in one file (or Compare Selections Block Start). Caret in the wrong position. First does not work.


STR: Show Only Compared Selections: ON. Select line 7 in userChrome Old.css and lines 1-9 in userChrome New.css. Compare Selections.

Result: Caret is invisible. Issues with navigation commands.


STR: Show Only Compared Selections: ON. Select lines 1-9 in both files. Compare Selections. Press Last.

Result: Caret is invisible.


STR: Select lines 1-9 in both files. Compare Selections. Select line 7 in one file. Re-Compare Selections.

Result: Line 7 vs. lines 1-9. I'm not sure this is the expected behavior.


STR: Show Only Compared Selections: should be tested when ON/FF. Select lines 1-9 in both files. Compare Selections. Select line 9 in one file. Paste some text.

Result: Line 9 is not in the Selection Block. Re-Compare is triggered even if Auto Re-Compare is OFF.


I suppose there are more issues. IMO they can be marked as known-issues for the time being and should not postpone the release of ComparePlus v1.

Thank you. BR

Yaron10 avatar Mar 05 '20 20:03 Yaron10

Hello Yaron,

I'll have a look at all these cases when I get home. As a remark - there is currently a known issue with navigation commands that I'm about to address next. Just to let you know about it.

Thank you for filling these STRs, much appreciated.

BR

pnedev avatar Mar 06 '20 08:03 pnedev

Hello Pavel,

Thank you. I appreciate it.

Enjoy your long weekend.

BR

Yaron10 avatar Mar 06 '20 12:03 Yaron10

Hello Pavel,

I don't really remember our discussions regarding "Compare Selections".

Trying to figure out what the user wants on re-Compare if currently there are no selections, should be very complex. If you allow me, I'd suggest to make it as basic and simple as possible.

Thank you.

BR

Yaron10 avatar Jul 06 '22 22:07 Yaron10

@Yaron10 , Hello my friend,

Now we are both back to Compare, I appreciate and enjoy that! :) This reminds me of the "old days" and the discussions we had back then ;)

I can't remember if we had ever discussed "Selection compare" as this was a new feature introduced in ComparePlus and I stopped the development for a very long time. I agree with you that selections re-compare should be "as simple as practically possible" whatever that kind-of political statement might mean. But joking aside what I mean is that in my practice it proved to be handy to remember internally the selections currently compared so if you do some editing and then do automatic or manual re-compare you don't have to bother selecting again and so on. What I also added is a way to "sub-select" which means that if you do selections compare and then do sub-selection in one of the files followed by new selections compare the other file selection is used against the new sub-selection in the other. That again proved to be useful for my use-cases and spares me from the need to re-select the portion in the other file. And the last thing is that if you haven't closed the comparison you can do manual full compare and go back to selection compare without the need to re-select the files portions (the selections are internally remembered as long as the compare is not cleared). That again proved to be handy at least in my day-work. So these behaviors are implemented at the moment and are deliberate.

Nice to have you back :+1:

BR

pnedev avatar Jul 07 '22 10:07 pnedev

Hello my friend Pavel,

Back in the old days. When we were young and beautiful. :) I'm glad you're back and enjoying it. So am I. Thank you.

Yes, we have discussed "Compare Selections". Sadly, I don't remember the details.

Trying to figure out what the user wants on re-Compare if currently there are no selections, should be very complex.

Briefly testing that now, it seems to be implemented very well. 👍 Some of the issues listed here seem to be still valid. I'll test "Compare Selections" more thoroughly once you let me know it should be ready.

What I also added is a way to "sub-select"... ... And the last thing is that if you haven't closed the comparison you can do manual full compare and go back to selection...

Brilliant! 👍

Thanks again for your great work.

BR


I'm sure the new RegEx feature would be very useful to many users. Unfortunately I don't know anything about that and can't try it.

Yaron10 avatar Jul 07 '22 17:07 Yaron10

Hello Yaron,

Some of the issues listed here seem to be still valid. I'll test "Compare Selections" more thoroughly once you let me know it should be ready.

Yes, I haven't come to these issues yet. I'll let you know when I debug them, thanks.

And thank you once again for all the feedback.

Unfortunately I don't know anything about that and can't try it.

Me neither :) I've tested it briefly but people that will make extensive use of that feature will report if things are not as expected.

But what comes with that feature is also a better Unicode handling and in that regard could I ask you to try comparing some texts in Hebrew including detection of char diffs and case ignoring and let me know if that works OK. If and when you find the time of course it is neither urgent nor mandatory. I appreciate it, thanks.

BR

pnedev avatar Jul 08 '22 08:07 pnedev

Hello Pavel,

Yes, I haven't come to these issues yet. I'll let you know when I debug them, thanks.

👍

But what comes with that feature is also a better Unicode handling and in that regard could I ask you to try comparing some texts in Hebrew including detection of char diffs and case ignoring and let me know if that works OK.

Sure, I'd be glad to try that. There's no "Case" in Hebrew.

Thanks again for your time and work. 👍

BR

Yaron10 avatar Jul 08 '22 10:07 Yaron10

Hello again,

But what comes with that feature is also a better Unicode handling and in that regard could I ask you to try comparing some texts in Hebrew including detection of char diffs and case ignoring and let me know if that works OK.

Thank you for pointing it out. Interesting. :)

https://github.com/pnedev/compare-plugin/issues/299.

BR

Yaron10 avatar Jul 08 '22 15:07 Yaron10

Hello Yaron,

Thanks for trying that and for the new report :+1:

BR

pnedev avatar Jul 12 '22 13:07 pnedev

Hello Pavel,

👍 Thank you for looking into it. I appreciate that. Best of luck. Whenever you have time.

Yaron10 avatar Jul 12 '22 22:07 Yaron10

Hello Yaron,

Most of the issues described here should be fixed with build https://ci.appveyor.com/project/pnedev/compareplus/builds/44533437.

Two remain:

  1. This part of the first STR:

No marks in one file (or Compare Selections Block Start).

I will check it tomorrow.

  1. Last STR:

Select line 9 in one file. Paste some text.

Result: Line 9 is not in the Selection Block. Re-Compare is triggered even if Auto Re-Compare is OFF.

Actually if Auto-Re-compare is ON the behavior is as expected. If it is OFF then re-compare does not happen but the change and selections update logic triggers that has difficulties dealing with corner cases (replacement of selections edges) and the result is what you have observed. You can even try undo-ing after the replacement to see the effect. I'm not sure I can do something about that and if it is worth the effort so I'll leave it as it is for the moment.

Could you please verify the other ones?

Thank You very much!

BR

pnedev avatar Aug 21 '22 21:08 pnedev

Hello Pavel,

Thank you very much for these fixes as well. Great work. 👍

  1. Last STR:

    Select line 9 in one file. Paste some text.

    Result: Line 9 is not in the Selection Block.

Actually if Auto-Re-compare is ON the behavior is as expected.

I don't understand why. Shouldn't any changes within the Compare Selections Block be considered as the "new selections"?

If it is OFF then re-compare does not happen but the change and selections update logic triggers ... I'm not sure I can do something about that and if it is worth the effort so I'll leave it as it is for the moment.

👍

BR

Yaron10 avatar Aug 25 '22 11:08 Yaron10

Hello Yaron,

I don't understand why....

Yes, you are right of course. I didn't make it clear - selection compare logic works with Scintilla notifications and such corner cases (replacement of the last line in the selection) cannot be easily avoided. That is because the so-called 'replacement' of the last line is actually composed of two steps - line deletion (triggers Scintilla notification and the selection is adjusted by removing the last line) followed by line insertion (again notification is triggered but this time the selection is not updated to include the new line).

I haven't thoroughly checked that but I will probably do it some time after the first release. AFAIR including a line just after the selection was a bit tricky because the behavior was not very intuitive when you position the caret in the beginning of the next line (after the selection) and insert a new line there (press Enter for instance) and if you do the same but position the caret at the end of the last selection line.

BR

pnedev avatar Aug 28 '22 22:08 pnedev

Hello Yaron,

All the issues (except the one with the selection corner line replacement) should be fixed now. Could you please check this build ? If it is not too much I would ask you to verify again that the Notepad++ tab in the task bar is flashing on "Wrap around diffs". Thank you, much appreciated.

Oh, forgot to mention that I have changed the name of the external libraries folder from ComparePlus to libs. Please change it manually in your plugin install folder.

BR

pnedev avatar Aug 30 '22 00:08 pnedev

Hello Pavel,

selection compare logic works with Scintilla notifications and such corner cases (replacement of the last line in the selection) cannot be easily avoided.

Thank you for the explanation. I should have thought about it.

AFAIR including a line just after the selection was a bit tricky because the behavior was not very intuitive when you position the caret in the beginning of the next line (after the selection) and insert a new line there (press Enter for instance) and if you do the same but position the caret at the end of the last selection line.

If and when you return to this issue, apparently there should indeed be a difference between the cases.

All the issues (except the one with the selection corner line replacement) should be fixed now.

Great! 👍 Thank you very much. Highly appreciated as always.

I have a feeling that "Compare Selections" should be further tested. Should I close it now?


The window properly flashes on "Wrap around diffs". 👍


I have changed the name of the external libraries folder from ComparePlus to libs.

👍

BR

Yaron10 avatar Aug 30 '22 11:08 Yaron10

Hello Yaron,

Thank You very much.

If and when you return to this issue, apparently there should indeed be a difference between the cases.

I think the behavior is intuitive now for the cases pointed by me, correct me if I am wrong. If I fix the corner replacement issue the behavior in the described cases is not intuitive anymore. I might think of a workaround sometime in the future so both the replacement and insertion behaviors are OK.

I have a feeling that "Compare Selections" should be further tested.

It seems we feel the same way ;) :+1:

For example if we do selections compare and show only selections if you move the caret below the last line using the keyboard arrow keys the caret is not visible. That seems to be a Scintilla issue as I do not interfere with caret movement.

I will close this issue now and make the final preparations for the first release.

If (or should I say when :) ) you find other issues please file them here and I will address them for future ComparePlus updates.

Thank You once again for all the help!

BR

pnedev avatar Aug 30 '22 11:08 pnedev

Hello Pavel,

I think the behavior is intuitive now for the cases pointed by me, correct me if I am wrong.

On adding a line the current behavior is intuitive. I misunderstood some part of your comment.

I might think of a workaround sometime in the future so both the replacement and insertion behaviors are OK.

Great. 👍 Apparently whatever is done within the Compare Selections Block range should be part of the new selections.

For example if we do selections compare and show only selections if you move the caret below the last line using the keyboard arrow keys the caret is not visible. That seems to be a Scintilla issue as I do not interfere with caret movement.

I've noticed that on clicking after the Compare Selections Block. Does Scintilla "know" the last selection line should be the last line in the file? :)

I thought of another fine-tuning point: always display Compare Selections - Start Block in both files. I think it would be more clear. Well, for the next release.

Good luck with the final preparations for the first release. The long journey has yielded a brilliant masterpiece! 👍 👍 👍 Thank you so much.

BR

Yaron10 avatar Aug 30 '22 13:08 Yaron10

Hello Yaron,

Apparently whatever is done within the Compare Selections Block range should be part of the new selections.

I totally agree :+1:

Does Scintilla "know" the last selection line should be the last line in the file?

It should know that the next lines are hidden and the caret shouldn't be moved there on user interaction. At the moment the caret seems to be positioned exactly in the beginning of the following line (the first hidden line) just after the last visible line end. IDK if this is actually Scintilla or Notepad++.

BR

pnedev avatar Aug 30 '22 14:08 pnedev

Hello Pavel,

Thanks for the explanation.

Looking forward to the grand announcement.

BR

Yaron10 avatar Aug 30 '22 15:08 Yaron10