pnedev / comparePlus

Compare plugin for Notepad++
GNU General Public License v3.0
1.01k stars 139 forks source link

Navigation: make sure the beginning of line is visible #322

Closed Yaron10 closed 2 years ago

Yaron10 commented 2 years ago

Hello Pavel,

Could you please compare these files?

Place the caret at line 84 in prefsNew.js and make sure the beginning of line 69 is not visible.

תמונה

Click Prev.

Result: The caret correctly jumps to line 69, but the beginning of that line is still not visible.

NOTE: if you add bookmarks to lines 84 and 69, place the caret at line 84 and use "Go to Previous Bookmark" - the beginning of line 69 is visible.

https://github.com/notepad-plus-plus/notepad-plus-plus/blob/dfa6c19d8766a8c3cf66fdb6632deddbd3b6a8e9/PowerEditor/src/Notepad_plus.cpp#L4520

Thank you.

pnedev commented 2 years ago

Hello Yaron,

I'll check that, thank you.

BR

Yaron10 commented 2 years ago

Hello Pavel,

👍 Great. Thank you. I haven't tested it with an older commit, but I think it was introduced recently.

BR

pnedev commented 2 years ago

Hello Yaron,

The behavior is changed accordingly here. Please have in mind that if you try the same STR but in the other view will lead to placing the caret at line 73 and displaying diff arrow mark towards the blank section. Thank you.

BR

Yaron10 commented 2 years ago

Hello Pavel,

Excellent! 👍 Thank you for yet another improvement.

BR

Yaron10 commented 2 years ago

Hello Pavel,

Thank you for https://github.com/pnedev/comparePlus/commit/c1c61aa2d4c267a2a2cef570752a579bf1083c6c.
I haven't tested it properly, but It seems like a good idea. 👍

If you place the caret at line 74 in prefsOld.js and press Prev, you jump to line 72. If you place the caret at line 73 in prefsOld.js and press Prev, you jump to line 60.

Now that line 73 is no longer a "place holder" (or an "arrow-line") for line 72, shouldn't you jump to line 72? What's the difference now between line 74 and line 73?

Would it require a major refactoring and re-testing? If it would, I'm wondering if the previous behavior might not be better ans safer.

BR

pnedev commented 2 years ago

Hello Yaron,

Thank you for catching that, good work! :+1:

This wrinkle is ironed here.

BR

Yaron10 commented 2 years ago

Hello Pavel,

👍 It's been smoothly and perfectly ironed. Thank you maestro. :)

BR

Yaron10 commented 2 years ago

Hello Pavel,

Could you please compare these files? Place the caret at line 1527 in CompareOld.cpp. Prev.

Result: You do not jump to Prev.

Whenever you have time. Thank you.

BR

pnedev commented 2 years ago

Hello Yaron,

Strange, it works for me. Placing the caret at line 1527 and using Prev I get to line 1515, just below the blank and I also see the arrow mark properly.

Any specific details on your side? Thank you.

BR

Yaron10 commented 2 years ago

Hello Pavel,

I get your result with "Ignore Spaces" checked. Could you please try it with that setting OFF?

תמונה

Thank you. BR

pnedev commented 2 years ago

Hello Yaron,

Correct, now it reproduces just fine :+1: Thank you.

BR

pnedev commented 2 years ago

Hello Yaron,

The problem case should be solved here. Thank you once again for testing and reporting.

BR

Yaron10 commented 2 years ago

Hello Pavel,

👍 Great! Thank you for the quick fix. I appreciate it.

BR