Closed pnedev closed 6 years ago
Hello Yaron,
This is a continuation of our discussion from https://github.com/pnedev/compare-plugin/issues/89#issuecomment-277395388.
I thought Compare SL should resemble coping the selections in files A and B, pasting them in New1 and New2 respectively and comparing. Shouldn't this be the approach?
I don't want to do that. It makes things messier, more difficult and if you would like to change the files while in compare mode you won't be able to.
Why should the diff status in line 30 (new) be removed? Why a blank line 30 in the old file?
Diff in line 30 new is removed because the old diff status of the whole previous diff block is removed. This is because in many cases it will be wrong. The blank in line 30 old aligns all lines below line 30 - makes the diffs appear side by side (in top-bottom views layout).
Line 30 in the old file is empty/blank.
I know, that's a big problem and quite difficult to solve too (at least in some cases).
And even regardless of this issue, it seems that allowing re-Compare SL with blank lines might lead to a total mess (think about multiple re-Compare SL with blank lines).
Absolutely true, I agree. That's why I said
As I said before this is an advanced usage and the user needs to pay attention and be aware of what he is doing in order not to screw his results...
Therefore - IMO it would be much better to have this great feature "Compare SL" in a simple and less flexible form but also less prone to unexpected results.
I agree with you. It is not possible to implement the feature the way I thought without leaving the 'doors open' for so many problems / unexpected and strange results. I'll implement it the safer way.
P.S.
Just curiosity: Why did you change #include "icon_sub_16.h" to 17?
That was for illustrating the use case I wanted to show you - this was a random change needed to get the desired result and make my point clearer.
Thanks for the discussion, Yaron.
BR
Hello Yaron,
Re-comparing now always clears all previous diffs - https://github.com/pnedev/compare-plugin/commit/4883ac9a86c1b2beb038e8adc4bd43e31684fbe3.
Thanks.
BR
Hello Pavel,
Thank you very much for the explanation and your work. This is a great feature and I was really impressed by your implementation. π
Would you allow me another suggestion? I've often found myself not sure whether I've pressed Compare or Compere SL. How about adding " SL" to line-selected compared files? (Would " Last Save - Outdated ** SL" be too much?).
An interesting behavior. STR: Have NppHelpers_new.cpp in the main-view and NppHelpers_old.cpp in the sub-view. Select line 19 in NppHelpers_old.cpp and line 20 in NppHelpers_new.cpp. Compare SL.
Result: Line 19 in NppHelpers_new.cpp is highlighted.
It is not the line-added color nor the selection's. NOTE: NPP (or rather Scintilla) has other issues with one-line-selection. Could you please have a look?
Just curiosity: Why did you change #include "icon_sub_16.h" to 17? ... That was for illustrating the use case I wanted to show you - this was a random change needed to get the desired result and make my point clearer.
Well, that was really smart of me! :) I realized my mistake some time after posting those lines. And on a (slightly) more serious note: those images are 14x14 px.
Thanks again. BR
Hello Yaron,
I've often found myself not sure whether I've pressed Compare or Compere SL. How about adding " SL" to line-selected compared files? (Would " Last Save - Outdated ** SL" be too much?).
That will make things not very clear, besides this info affects not the compared file itself but the compared pair as a whole. But your suggestion is reasonable just the implementation should be different IMO. I think here comes the need for the Compare status bar - the compare type should be shown there. I imagine for example a status bar that is shown together with the NavBar - just below the NavBar window. It may initially show only the compare type and later be used to display other useful info. What do you think?
I have problems re-creating the STR. Is it steadily reproducible on your side? I'm rarely testing on native Windows and this might have some implications.
Thanks. BR
Hello Pavel,
besides this info affects not the compared file itself but the compared pair as a whole.
I don't quite understand it but, obviously, I trust you.
BTW, I happened to see yesterday cmpPair
. I loved it. :)
That will make things not very clear
Do you mean it's too messy or "SL" isn't clear? If the latter, you could use "Sel" (displayed in NPP status bar).
I think here comes the need for the Compare status bar - the compare type should be shown there.
That's very good. π
I imagine for example a status bar that is shown together with the NavBar - just below the NavBar window.
Do you mean it should be linked to the NavBar? Personally I hardly use the NavBar.
I have problems re-creating the STR. Is it steadily reproducible on your side?
I've just discovered that it happens with regular Compare as well (so it's not related to Compare SL at all). And now it gets even stranger: It's consistent with lines 19/20 (and some other pairs of lines) but doesn't occur with some other lines. Should I open a new issue for that? - I have a hunch that it's a NPP issue but I may be wrong.
Compare SL is a brilliant useful addition. Thank you for your work. Highly appreciated.
BR
Hello Yaron,
I don't quite understand it but, obviously, I trust you.
I mean that the place for this info is not in the file's name (its N++ tab text) because it does not affect just that file but both compared files. IOW it is not per file info but per compare info.
Do you mean it's too messy or "SL" isn't clear?
It is too messy and for the reasons stated above its logical place is not in the file tab name but somewhere else - the CP Status Bar for example that gives info per compare (just as NavBar gives info per compare).
Do you mean it should be linked to the NavBar?
Well, the CP N++ dockable window is the place for such info IMO. This is the NavBar now but it should contain all needed data per compare. I think if the dockable window contains the NavBar above and below it - the Status Bar with additional data that would be good layout. Do you have other suggestions? Where do you think the status bar should reside?
I've just discovered that it happens with regular Compare as well ...
That's bad. Could you please open another issue and write (if possible) detailed steps to reproduce that (+ the files you are comparing and the CP settings you have). Thank you.
BR
Hello Pavel,
STR: Have NppHelpers_new.cpp in the main-view and NppHelpers_old.cpp in the sub-view. Select line 19 in NppHelpers_old.cpp and line 20 in NppHelpers_new.cpp. Compare SL.
Result: Line 19 in NppHelpers_new.cpp is highlighted.
Good news. It is a NPP issue.
I'm still using NPP v7.1 and the problem happened with that version. I downloaded a portable NPP v7.3.1 and could not reproduce it with the default settings. After some investigation and comparing the old and new config.xml files, here are the STR the bug in NPP v7.3.1 and how it affects CP.
Preferences -> Highlighting -> Smart Highlighting. Check "Enable". Uncheck the the three following options and check "Highlight another view". Uncheck "Enable".
Close NPP.
(This is the relevant line in config.xml: <GUIConfig name="SmartHighLight" matchCase="no" wholeWordOnly="no" useFindSettings="no" onAnotherView="yes">no</GUIConfig>
).
Open NppHelpers_old.cpp and NppHelpers_new.cpp.
Move NppHelpers_old to the other view.
Select the line #include <windows.h>
in that file.
Close NPP.
Restart NPP.
Result: (The session is restored). The line in NppHelpers_new is highlighted (it's "sticky": you can only remove it by closing the file).
And now to CP. When the files are in that state - Compare. Select line 21 in NppHelpers_old (result: line 21 in NppHelpers_new is highlighted). Select another line and, again, its counterpart is highlighted.
NOTE: with CP v1.5.7, the lines in the other files are selected only after scrolling.
Sorry for the confusion and thank you for looking into it.
I mean that the place for this info is not in the file's name (its N++ tab text) because it does not affect just that file but both compared files. IOW it is not per file info but per compare info.
Thanks for the clarification. Sometimes I'm not sure to what extent I should allow myself to argue and express my view. :) If your position is set, let's regard this point as closed.
The more I've used Compare SL, the more positive I've become of the importance of a visual indication differentiating it from full Compare.
Whatever shape and form you implement the toolbar, the user still should have an option not to display it (personally I'd probably prefer to hide it most of the time).
I agree that this is a per-compare info, but both files are in Compare SL mode; so why would it be wrong to display that in both tabs?
Do you have other suggestions? Where do you think the status bar should reside?
Well, mine is a user-perspective not taking into account the implementation and its complexity. :) IMO it would be better to separate the status bar and the NavBar (as one might want to display one and not the other). As for its position: full horizontal bar above NPP status bar.
And now please allow me some comments/questions regarding the Compare Engine.
Could you please compare ? (Left/Right layout; configMew.xml in the right view; Word wrap: OFF).
This is with CP 2:
And this is before the last engine refactoring:
Personally I find the old results more convenient and easier to quickly grasp the differences between the files.
You normally look at line X in both files at the same time. In the current implementation you may have "+" in line X in one file and "-" (or "moved") in the same line in the other. This means that you have to interpret two different texts at the same time.
I think that "Align Matches" in the broader sense means: Align Matches and Do not align non-matches but insert blank lines instead (changed lines are a match for this matter).
IOW: if "Align Matches" is on, we don't just put line X next to line X but rather organize the lines in a way which should be easier to quickly "get the picture". And we don't insert blank lines only to align matches but also not to have non-matches aligned.
What do you think? This is major IMO.
**
Comparing the config files I've noticed that (in the current implementation) the line <GUIConfig name="TagsMatchHighLight" TagAttrHighLight="no" HighLightNonHtmlZone="no">yes</GUIConfig>
is marked as "+" and "-" instead of "moved".
Could you please have a look?
Well, that was quite long. :) Thank you for your great work and patience. Highly appreciated.
BR
Hello Yaron,
I downloaded a portable NPP v7.3.1 and could not reproduce it with the default settings.
So I'll consider this STR irrelevant. Thank you.
I agree that this is a per-compare info, but both files are in Compare SL mode; so why would it be wrong to display that in both tabs?
Because it is not something related to the file. The file tab name is informative to the N++ file itself and should remain intact. A compared file is simply a file for N++ that is no different than the other open files. It is just represented differently by CP. And thus adding "SL" to the name is not very appropriate - the file hasn't been modified in any way. There is a difference when selecting the first file - it shows temporary selection of that file for potential compare. Or with the temp files - those are "fake" files and thus can be freely marked to describe their purpose. The "real" files shouldn't be stamped in any way. Especially both of them for the same purpose (unnecessary redundancy). SL is not their attribute.
IMO it would be better to separate the status bar and the NavBar (as one might want to display one and not the other). As for its position: full horizontal bar above NPP status bar.
Perhaps that would be best, I'll think about that, thanks. It will be something for the near (or not that near :)) future.
And now please allow me some comments/questions regarding the Compare Engine......
Well, the current implementation (v2) is the GitHub representation - please look at any commit diffs, for example https://github.com/pnedev/compare-plugin/commit/4883ac9a86c1b2beb038e8adc4bd43e31684fbe3.
That makes it clearer what portion of the text is replaced by what other portion (side by side) it keeps the real line numbers, reduces the blank lines usage (they have great impact on the speed and the resources needed and are also the reason for breaking the undo - if no blanks are inserted (that is, the files have equal number of lines and changed lines) then the undo history is kept) and make things more compact. As to Align Matches - this is a thing from the past. And also + and - represent kind of replace operation on the text - it is again kind of "match" meaning that the corresponding section of the file is totally changed, but this is the same section - it is not missing from one of the files - it is just different.
Comparing the config files I've noticed that (in the current implementation) the line ...
Hm, that's interesting indeed. I'll have a look some time soon. Thank you for all the comments and suggestions Yaron.
BR
Hello Pavel,
Because it is not something related to the file. ...
Thanks for the explanation. Let's agree not to agree on this point. :)
Perhaps that would be best, I'll think about that, thanks. It will be something for the near (or not that near :)) future.
Sure, whenever you find the time. Thank you.
BTW, NPP Incremental Search Bar is basically a dialog if I understand it correctly.
Well, the current implementation (v2) is the GitHub representation ...
Your arguments are indeed good and powerful.
Clarity
I use Firefox with my own theme. When a new Firefox version comes out, I need to compare the (relatively large) old and new source files and adjust the theme. After the last engine refactoring I found the new method helpful in some cases and less convenient in others. Overall, I thought it was a matter of getting used to it.
Yesterday, testing the sticky-highlighted-line issue with CP 1.5.7 too, I decided to compare and analyze the pros and cons of each method.
I think it's a personal preference which might also depend on the contents of the compared files. Having added and removed lines aligned can be very convenient if you compare small files. In some other cases it may be highly confusing for three reasons:
Performance and Undo You're right but it can be even better without using blank lines at all (uncheck "Align Matches" in old CP).
Other users may not analyze it as I have, but I'm positive that many of them would find the old method more convenient and efficient for their workflow (at least in some cases).
What do you think? Would a preference for that be too complex?
Thank you so much.
**
I've tried WinMerge. CP is much better! :) π There seems to be some inconsistency in WM implementation.
vs.
In CP the last lines are aligned as well.
Hm, that's interesting indeed. I'll have a look some time soon.
Thank you for that too. Highly appreciated as always.
BR
Hello Yaron,
Let's agree not to agree on this point. :)
Agree :)
I couldn't quite grasp those two:
You're right but it can be even better without using blank lines at all (uncheck "Align Matches" in old CP). Other users may not analyze it as I have, but I'm positive that many of them would find the old method more convenient and efficient for their workflow (at least in some cases).
Do you refer to getting back Align Matches or to getting back the old results representation with blanks against the + and - lines?
What do you think? Would a preference for that be too complex?
Using a preference for that is possible, yes. If you really think it is justified and will add-up value. Could you open a new feature request in the later case please? Thanks.
In CP the last lines are aligned as well.
Sorry, this is also not quite clear to me.
BR
Hello Pavel,
You're right but it can be even better without using blank lines at all (uncheck "Align Matches" in old CP).
I meant that the Performance and Undo arguments are good but clarity is more important. Otherwise, you could have the no-blank-lines-at-all method as the only option.
Do you refer to getting back Align Matches or to getting back the old results representation with blanks against the + and - lines?
I was referring to restoring the old results representation with blanks against the + and - lines.
Using a preference for that is possible, yes. If you really think it is justified and will add-up value.
I'm sure it is justified and I'm glad you agree (to agree :) ). Thank you very much.
Continued in https://github.com/pnedev/compare-plugin/issues/109.
There seems to be some inconsistency in WM implementation. In CP the last lines are aligned as well.
I meant that CP is more consistent as in both cases the moved-line is aligned with the removed-line. It was just a footnote.
BR
Hello Yaron,
I meant that the Performance and Undo arguments are good but clarity is more important.
Yes, that's true.
I was referring to restoring the old results representation with blanks against the + and - lines.
OK, I thought that also just wanted to be sure I understand you correctly.
It was just a footnote.
I see, OK. Thanks, Yaron.
BR
Hello Pavel,
Seeing that you're still passionate and full of energy (Dynamic Compare, Status Bar and more), I'll allow myself to return to the Proper alignment in Wrap mode issue.
This is a raw idea. Would it be possible to dynamically align the visible lines?
Can a line be placed in an arbitrary position? That is: position line 2 in coordinate Y some mm below the end of line 1.
If it can be done, the "dynamically aligning the visible lines" idea would still require a lot of work but it isn't far-fetched.
In the compare-process insert blank lines regardless of the counterpart's length. When the results are displayed, check the position of each visible line and align them. (And even if the user moves the splitter (i.e. 70% to one view and 30% to the other), the lines would still be aligned).
If it can not be done, how about an option to copy the extra text in one line and insert it (invisible-transparent) into the counterpart line?
With all your amazing improvements, solving this issue would surely be one of the most important achievements. :)
Another idea for distinguishing Compare SL: change the color of the Compare-Margine (could be customizable).
What do you think?
Thank you very much. BR
Hello Yaron,
I'm not sure I understand everything (and understand correctly) but OK, let me comment things one by one.
Can a line be placed in an arbitrary position? That is: position line 2 in coordinate Y some mm below the end of line 1.
I don't think that's possible in Scintilla. That's why I bother inserting / removing blank lines in the first place.
If it can not be done, how about an option to copy the extra text in one line and insert it (invisible-transparent) into the counterpart line?
If I remember correctly we discussed that before (or something similar). The problem is that if you insert the extra text it WILL BE actually there (even though it is invisible) which makes things VERY prone to errors (when saving or closing the file).
Another idea for distinguishing Compare SL: change the color of the Compare-Margine (could be customizable).
That is not a bad idea actually but doesn't really solve the whole problem. For example it will show you if the compare was full or SL but it will not show you the Ignore Spaces, Ignore Case, Detect Moves states for that compare. Which again shows the need for compare status bar. It is better to put our efforts in the right direction.
Thank you for the ideas and the discussion.
BR
Hello Yaron,
This is a rough StatusBar implementation: https://github.com/pnedev/compare-plugin/commit/bf47e92f8eeb98dc1df1fe6231a1b0060d4a0216.
x86 binary: https://ci.appveyor.com/api/buildjobs/6n88by482o9sgnbe/artifacts/ComparePlugin.dll
I haven't decided yet on the format of the displayed info and what exactly to show there but I'm sure you already have an idea and you'll help with that :)
Currently the status info update conflicts with the Session Manager plugin (that is when switching between the files Session Manager overwrites the info) but this is an illustration of my idea about the StatusBar.
BR
Hello Pavel,
It was a raw idea and you seem to have gotten it as I have.
Can a line be placed in an arbitrary position? That is: position line 2 in coordinate Y some mm below the end of line 1. ... I don't think that's possible in Scintilla. That's why I bother inserting / removing blank lines in the first place.
OK. That could have been very helpful.
If it can not be done, how about an option to copy the extra text in one line and insert it (invisible-transparent) into the counterpart line? ... If I remember correctly we discussed that before (or something similar). The problem is that if you insert the extra text it WILL BE actually there (even though it is invisible) which makes things VERY prone to errors (when saving or closing the file).
Yes, we've discussed it before. I brought it up again and as a user-preference due to the importance of the issue. I understand the potential problems even as a user-choice.
Another idea for distinguishing Compare SL: change the color of the Compare-Margine (could be customizable). ... That is not a bad idea actually but doesn't really solve the whole problem...
I had in mind a toggled CP Status Bar and thought the SL type was (more) important and should always be indicated. Your brilliant idea of using NPP Status Bar is certainly a better solution for this too.
And I'll elaborate on this excellent idea of yours in https://github.com/pnedev/compare-plugin/issues/16.
Thanks for the explanation and your work.
Should I open new issues for the "moved-marked-as-added" and the buttons state? Or are they well remembered and safe on your To-Do list? :)
BR
Hello Yaron,
OK. That could have been very helpful.
Couldn't agree more :(
Yes, we've discussed it before. I brought it up again and as a user-preference due to the importance of the issue. I understand the potential problems even as a user-choice.
This is really a recipe for disaster. Let's not open the Pandora's box.
Should I open new issues for the "moved-marked-as-added" and the buttons state?
I remember those two but yes, it is better to have them written somewhere for the future generations ;) Thanks.
BR
Hello Pavel,
it is better to have them written somewhere for the future generations ;)
Long live the current generation! I'm afraid the future generations won't be as good and clever as this one. :)
Thank you very much "current generation". BR
Hello Yaron,
Long live the current generation!
Cheers! :)
BR
Hello all,
FYI:
A user sent me recently an e-mail with request to make the "Files match. Close (Y/N)" dialog optional. He preferred to have instead a simple "Files match. OK" info message box. Unfortunately I deleted the e-mail by mistake and I don't remember the user name or his contacts but I think it is a reasonable request and I'll implement a setting for that (I personally also prefer a simple info message).
BR
Hello Pavel,
When you're done with the Moved-Lines improvements, could you please have a look at this minor issue/question?
https://github.com/pnedev/compare-plugin/commit/3332eac30add45a567439a7e729f0adbc9911ea8 has improved the caret position handling on switching between compared files.
Before that commit: Switching from a "real" line in file A to a real new line in file B (i.e. the caret was not in that line in file B the last time), the caret would be positioned at the beginning of the line.
After that commit:
The caret is positioned at the same column as well.
If the line in file B is shorter, the caret is positioned at the end of the line.
So far so good. Actually excellent. π
**
But when switching from a blank line to a real line (or vice versa), the behavior is different.
Before that commit: The the caret would be positioned at the beginning of the line.
After that commit: The position seems random.
Thank you very much. BR
Hello Yaron,
I'll have a look, thanks for reporting.
BR
Hello Pavel,
Thank you. I appreciate it.
I think that some form of help would be necessary for the next version. How about adding "Help - Overview..." opening this page? Any comments would be appreciated.
I'd like to add a Moved-Multiple screenshot. Currently it doesn't work with the lines I want to use. I'll wait for your improvements.
May I ask your help with "SVN/Git Diff" and "Compact Navigation Bar"? This is, obviously, not urgent. Whenever you find some free time.
Thank you and have a nice weekend. BR
Hello Yaron,
Thank you for writing such concise but clear and complete CP guide / overview. Excellent job! :+1:
I think that some form of help would be necessary for the next version. How about adding "Help - Overview..." opening this page?
I agree and that would really be helpful. Perhaps in the future it would be better to put the guide you've written in the README.md file?
Currently it doesn't work with the lines I want to use. I'll wait for your improvements.
OK, sure.
May I ask your help with "SVN/Git Diff" and "Compact Navigation Bar"?
Yes, I'll take care of those, thank you.
Could you reference the help page in issue https://github.com/pnedev/compare-plugin/issues/43 and mark that as resolved or you'd like the help to be implemented in other way and prefer to leave that open?
BR
Hello Yaron,
This commit adds the "Help - Overview..." link to the About dialog as you suggested: https://github.com/pnedev/compare-plugin/commit/d07149e9cc0e13ba728bf0a40a0e0b8d80467738. Thanks again.
BR
Hello Pavel,
Thank you for writing such concise but clear and complete CP guide / overview.
Thank you. I'm glad you like it. No comments? :)
Changed: Most of the line is identical in both files. Some changes have been made.
Is that accurate? What's the precise condition for marking a line as "Changed"?
Perhaps in the future it would be better to put the guide you've written in the README.md file?
That would be indeed better. Thanks. That page contains other info sections. Can the link open the page in the Help section? And why "in the future"? :)
May I ask your help with "SVN/Git Diff" and "Compact Navigation Bar"? ... Yes, I'll take care of those, thank you.
Thank you.
Could you reference the help page in issue #43 and mark that as resolved or you'd like the help to be implemented in other way and prefer to leave that open?
I'll close https://github.com/pnedev/compare-plugin/issues/43.
This commit adds the "Help - Overview..." link to the About dialog as you suggested: d07149e.
Thank you. I appreciate it.
Wouldn't it be better to add it to the menu (between "Settings..." and "About..." as the convention)? Sorry for not being clear enough.
Should the info displayed in the Plugin Manager be updated by PM?
Do you think that "Prompt" in "Prompt to close files on match" is the right word? How about "Suggest to close files on match" or "Show "Close files?" on match"?
Thank you. BR
Hello Yaron,
No comments? :)
Surely I have some :) Here they are:
My major remark is regarding this:
Added "+": The line only exists in this new file. IOW: the line has been added to the new file, and does not exist in the old one. Removed "-": The line does not exist in the other new file. IOW: the line has been removed in the new file, and only exists in this old file.
Those statements are not true. The actual meaning is that the line is added/removed to/from that location of the file and (if Detect Moves is enabled) it is not moved somewhere else, IOW - a line is added/deleted in/from the new file (as opposed to the old file).
Moved: The line appears once in the other file and in a different location.
Your picture in https://github.com/pnedev/compare-plugin/issues/116 looks perfectly OK (apart from the text being wrong: There is a typo - "great" instead of "graet" and the moved line statement is not true as explained above.). Do you think it is wrong for some other reason?
Wrap around diffs: Determines whether the "Next" command should be enabled on reaching the last diff and go to the first diff.
And also whether the "Prev" command should go to the last diff if we were positioned on the first diff (reverse-wrap-around).
Re-Compare active on Save: Check to automatically re-Compare the active Compare on saving its files.
"Check to automatically re-Compare the active Compare on saving one of the files (or both)." looks more accurate I think.
Requests and Issues-Report
"Requests and Issue reporting" sounds better to me, what do you think?
Also all the images show N++'s horizontal/vertical sync toolbar icons enabled while those are disabled in the latest CP development versions.
Moving on to your other remarks.
Is that accurate? What's the precise condition for marking a line as "Changed"?
Yes, that's accurate. The actual matching threshold to decide a line is changed and not completely new is if at least 1/3 of its size (excl. spaces) matches with the old version.
That page contains other info sections. Can the link open the page in the Help section?
I'm not sure but I suspect it cannot. May be we can create another file - Help.md?
And why "in the future"? :)
Because I'm currently busy with the other tasks and because all the images in the help need to stay somewhere (those cannot "live" in the MD file itself AFAIK) and I need to check that.
I'll close #43.
Thanks for closing that :+1:
Wouldn't it be better to add it to the menu (between "Settings..." and "About..." as the convention)?
I prefer this to be part of the "About" dialog. When a user is already aware of the CP capabilities and its workflow he/she wouldn't need the "Help" - it will only make the menu bigger and consume another N++ command without actual need. If you prefer we can rename the "About..." to "About and Help...".
Should the info displayed in the Plugin Manager be updated by PM?
When I release version 2.1 it will actually diverge from the original CP repo and then I'll change the "Source" link. I don't know what to do with the other info. Jean-Sebastien's repo is still there and is the "official" one but it is not maintained (except by us at the moment) but frankly speaking I would prefer to part from it entirely. I have difficulties setting-up the automatic build-release process (I have different AppVeyor account) our branches have diverged and it is difficult and time consuming for me to keep the official repo in sync with mine. I consider my version of CP to have diverged significantly from the official one to be considered as "new" Compare plugin (or at least "alternative private version" of the Compare plugin) having as base the old development and acknowledging the efforts and invaluable work of all other CP contributors/authors. Having said that it is not quite ethical for me to modify a lot the official CP data in the Plugin Manager's list and I'm trying to minimize those changes. So now I'm kind of juggling by keeping the info about the "original" repo (that is kind of history and of no real use) and directing the users to the new development place which actually is confusing for them. This is a problem for which I currently have no solution.
Do you think that "Prompt" in "Prompt to close files on match" is the right word?
Yes, I believe so. It seems quite appropriate at least for me. We can use "Ask" instead of "Prompt" if you think it will be better. What do you think?
Thank you once again Yaron for writing the help info and for all the suggestions and shared thoughts.
BR
Hello Pavel,
Your comments are all great (g-r-e-a-t). Thank you very much. I appreciate the sharp observation. Thanks also for the explanation.
Regarding the added/removed paragraphs: I'd like to get your kind reply in https://github.com/pnedev/compare-plugin/issues/113#issuecomment-283230180 and understand the issue better before rephrasing them. I'll make all the other modifications then.
Also all the images show N++'s horizontal/vertical sync toolbar icons enabled while those are disabled in the latest CP development versions.
And the CP buttons state in the first images should be updated too.
I'm not sure but I suspect it cannot. May be we can create another file - Help.md?
π
Because I'm currently busy with the other tasks and because all the images in the help need to stay somewhere (those cannot "live" in the MD file itself AFAIK) and I need to check that.
OK. I thought there was a "deeper" reason for "in the future". :)
I prefer this to be part of the "About" dialog. When a user is already aware of the CP capabilities and its workflow he/she wouldn't need the "Help" - it will only make the menu bigger and consume another N++ command without actual need. If you prefer we can rename the "About..." to "About and Help...".
This is a minor issue. Still, I'd like to use "my quota" and present a different view:
I usually attribute very little importance to conventions. But if we agree that "Help" is needed for v2.1 ("Align Preferences" and more), the convention of a separate "Help" command is good and important.
If a command is useful, why regard it as "making the menu bigger"? The "About" dialog is also not necessary for users who have already seen it.
"Help/About..." would be better than "About and Help..." but still confusing IMO.
If you're open about it, we can ask Claudia for her opinion. :)
Do you think that "Prompt" in "Prompt to close files on match" is the right word? ... Yes, I believe so. It seems quite appropriate at least for me. We can use "Ask" instead of "Prompt" if you think it will be better. What do you think?
I think that "Ask" in this context would mean "Ask to" - "Request". As you understand.
When I release version 2.1 it will actually diverge from the original CP repo and then I'll change the "Source" link. I don't know what to do with the other info. ...
I'd be the first to agree that your CP is virtually a new (and, obviously, much better) application. I also agree that you can not completely ignore the previous developers.
Still, you can certainly make the distinction between the current and old versions more prominent. For example, in the About dialog you can make the following changes:
Use "Old Version" instead of "Original Project Link".
Set the current version info within a frame (or use three lines separating that section).
Use a bold font for the current version.
Similar changes can be made wherever possible.
And why should you maintain the old repo?
Compare Plugin v2 is great! :) Thank you.
BR
Hello Yaron,
First, thank you for all the comments.
I'll make all the other modifications then.
OK, thanks for that too.
If a command is useful, why regard it as "making the menu bigger"? The "About" dialog is also not necessary for users who have already seen it.
A "command" is an "entrance" to the thing's functionality - it carries the purpose of it's existence and this is the most important thing. The "About" dialog is not necessary in this regard but it has ancillary function - It serves as entry point to all such functions (there is no other way to access those if it weren't there). And it is useful to get all kinds of information about the thing - what it is, what is its purpose, what is its current version, where it comes from, who is responsible for what, terms of its usage, how to use it, where to get support, etc. All of this is an important information but why should all those separate subjects have their own separate commands? The programs that have separate "Help" and "About" perhaps need the help as a reference - that is, you often need to look-up some detail of particular usage specifics.
"Help/About..." looks good to me.
If you're open about it, we can ask Claudia for her opinion. :)
OK, please do. One more opinion is always the better option. :+1:
This is an excerpt from Google Translate:
assist or encourage (a hesitating speaker) to say something. βAnd the picture?β he prompted
The "prompt" also implies user interaction / request for user action - the DOS or Shell prompt for example.
I think that "Ask" in this context would mean "Ask to" - "Request".
"Request" has a slight implication of "demanding" something and we are not requiring the user to close the files. We are simply querying / questioning him/her if he/she would like to close the files. But you are right - "Ask to" seems also quite OK. Please tell me if you prefer it to be "Ask to close files on match", thanks. You could ask Claudia about that too BTW. The more opinions - the better.
For example, in the About dialog you can make the following changes: ...
Those are good suggestions about the "About" dialog I'll incorporate them, thank you. I'm afraid those can be made only in the "About" dialog. And the main problem is the PM info as most of the users are updating/installing CP from there.
And why should you maintain the old repo?
I'm not obliged to but I'm trying to support the CP users and they are going to the official repo to seek help or file issue requests.
Compare Plugin v2 is great! :)
Thank you. You are also responsible for this ;)
BR
Hello Yaron,
Could you please have a look at https://github.com/pnedev/compare-plugin/issues/116#issuecomment-283321250 ? Thanks.
BR
Hello Pavel,
A "command" is an "entrance" to the thing's functionality - it carries the purpose of it's existence and this is the most important thing. ...
Your view about it seems quite firm, and I feel I shouldn't take it any further. :) Thank you for the explanation and for the change to "Help / About...".
I think that "Ask" in this context would mean "Ask to" - "Request". ... "Request" has a slight implication of "demanding" something and we are not requiring the user to close the files. We are simply querying / questioning him/her if he/she would like to close the files.
That's exactly what I meant! :) "Ask" would mean "Request" (a slight implication of "demanding") and we don't do that.
And "Prompt", IMO, implies "Induce" and we don't do that either.
Are you still considering the frame/ bold font in the About dialog?
Some lines or multiple - - -
should be easier to implement and would further distinguish the new version section.
IMO multiple - - -
can be used in the PM as well.
Compare Plugin v2 is great! :) ... Thank you. You are also responsible for this ;)
Thank you. "No base to compare - ignored.". :)
BR
Hello Yaron,
Your view about it seems quite firm, and I feel I shouldn't take it any further. :) Thank you for the explanation and for the change to "Help / About...".
No problem, thank you.
That's exactly what I meant! :)
Hm, I'm a bit confused here :) Could you please remind me what was your proposal for the setting string? Thanks.
Are you still considering the frame/ bold font in the About dialog? Some lines or multiple - - - should be easier to implement and would further distinguish the new version section. ...
I think that bold font will not look OK - it will kind of 'grab' the attention as if it is the most important thing to see in the About dialog which is not the case (all info there is of equal importance IMO).
I'll try and see how the dialog will look like with - - -
s added. That might be fine but I'm not sure. I don't want to hurt the overall style of the window (different sections separated by blank space is the current style).
The - - -
s in PM will certainly bring clarity, thanks.
BR
Hello Pavel,
Hm, I'm a bit confused here :) Could you please remind me what was your proposal for the setting string? Thanks.
Trying to "get into your head" and phrase it as short as possible I suggested: "Show "Close Files?" on Match". Thank you.
I think that bold font will not look OK...
I see. I'm sure you'll come up with some clever solution. :)
Allow me two more minor questions:
The "Old Version" and "Ongoing Development..." parts consist of separate texts and links. In "Help - Overview" the link contains the text (or vice-versa). Is that intentional?
"Ongoing Development and Support" and then "Help - Overview" is slightly confusing.
How about "Ongoing Development, Requests and Issue reporting" and then "Help - User Guide" (without ...
as it's a link)?
Thank you very much.
Again: I suppose you're currently busy with https://github.com/jsleroy/compare-plugin/issues/66 and replying is time consuming too. So, whenever you find the time.
BR
Hello Yaron,
Thank you for commenting.
I suggested: "Show "Close Files?" on Match"
I'll think about that later, thank you.
The "Old Version" and "Ongoing Development..." parts consist of separate texts and links. In "Help - Overview" the link contains the text (or vice-versa). Is that intentional?
Yes, that was the "legacy" implementation and I kept it - this way the user can see the actual repo URL. That is not necessary for the help link I think.
"Ongoing Development and Support" and then "Help - Overview" is slightly confusing. How about "Ongoing Development, Requests and Issue reporting" and then "Help - User Guide" (without ... as it's a link)?
"Requests and Issue reporting" is actually "Support" isn't it? I prefer the shorter version if you agree. "Help - User Guide" looks great, thank you π .
Have a good rest during the weekend :) BR
Hello Pavel,
I suggested: "Show "Close Files?" on Match" ... I'll think about that later, thank you.
We're both quite opinionated, aren't we? :) Thank you.
Yes, that was the "legacy" implementation and I kept it - this way the user can see the actual repo URL. That is not necessary for the help link I think.
I thought you wanted the users to see the URL and it's indeed useful.
"Requests and Issue reporting" is actually "Support" isn't it?
What I found slightly confusing was the proximity of "Support" and "Help" which are often used for the same purpose. Not really important. Thanks for that too.
Have a nice weekend. BR
Hello Yaron,
We're both quite opinionated, aren't we? :)
Indeed we are :)
What I found slightly confusing was the proximity of "Support" and "Help" which are often used for the same purpose.
You are right about that. Is limiting "Help - User guide" to just "User Guide" looking OK, how do you think? Thanks.
BR
Hello Pavel,
I thought you were skiing somewhere this weekend. :)
Is limiting "Help - User guide" to just "User Guide" looking OK, how do you think?
Great. π Thank you.
How about "Use extended dialog on match"? Or "Use extended Match dialog"?
Thank you for https://github.com/pnedev/compare-plugin/commit/51f0a00bbb1bc6ef601d7e25ecb05f3478ac2e8d. Could this have been the cause for the crash I had several months ago (not the memory issue)?
BR
Hello Yaron,
How about "Use extended dialog on match"? Or "Use extended Match dialog"?
Let me think about that later. I'm getting a little bit lost as the things that need fixing are increasing and I'm starting to forget the ones we mention between the lines in different issue threads.
Thank you for 51f0a00. Could this have been the cause for the crash I had several months ago (not the memory issue)?
You are welcome. It is possible but I cannot say for sure. It certainly could be leading to all kinds of strange not steadily reproduce-able behavior.
I'm not sure if I wrote you back about the memory issue - I tested it no my native Win7 machine and the size I could allocate was 1GB. This is big enough to NOT see the out-of-memory exception unless we are comparing some REALLY big files (which is not the case). I have no idea why we are getting that exception but it might be related to the issue fixed with https://github.com/pnedev/compare-plugin/commit/51f0a00bbb1bc6ef601d7e25ecb05f3478ac2e8d. Thanks.
BR
Hello Yaron,
I suggested: "Show "Close Files?" on Match"
and
Is limiting "Help - User guide" to just "User Guide" looking OK, how do you think?
and
IMO, the three first settings should have radio-buttons (as the convention in case you have only two options).
are all implemented in https://github.com/pnedev/compare-plugin/commit/6ad27c107ea9a20a3905314d65a31a520a800ec8. x86 binary: https://ci.appveyor.com/api/buildjobs/vb4o4ccslsux09qg/artifacts/ComparePlugin.dll
BR
Hello Pavel,
are all implemented in 6ad27c1.
Thank you very much. "Show "Close Files?" dialog on match" is nice and clear. π
It is possible but I cannot say for sure. It certainly could be leading to all kinds of strange not steadily reproduce-able behavior.
I'm not sure if I wrote you back about the memory issue...
Thanks for the explanation. I appreciate it.
BR
Hello Pavel,
A short question with your permission:
Why are the following lines marked as Changed?
convert Bookmark14.png -define h:format=rgba -depth 8 -size 14x14 Bookmark14.h
Bookmark14.h (rgba).
Only 16 characters in the second line exist in the first one.
Thank you.
BR
Hello Yaron,
I'll look into this once I finish the refactoring. Could you please file a new issue as a reminder? Thanks.
BR
Hello Pavel,
Probably for a later stage.
How about the following diff symbols for bright and dark themes? They're slightly smaller and less "in the face". Also - there's more space for some "badges" (depending on the Move discussion).
For using RGBA instead of XPM (the RGBA files are larger but the binary is slightly smaller):
In NppHelpers.cpp
:
Replace
static void DefineXpmSymbol(int type, const char **xpm)
{
::SendMessage(nppData._scintillaMainHandle, SCI_MARKERDEFINEPIXMAP, type, (LPARAM)xpm);
::SendMessage(nppData._scintillaSecondHandle, SCI_MARKERDEFINEPIXMAP, type, (LPARAM)xpm);
}
with
static void DefineRgbaSymbol(int symbol, const unsigned char *rgba)
{
::SendMessage(nppData._scintillaMainHandle, SCI_MARKERDEFINERGBAIMAGE, symbol, reinterpret_cast<LPARAM>(rgba));
::SendMessage(nppData._scintillaSecondHandle, SCI_MARKERDEFINERGBAIMAGE, symbol, reinterpret_cast<LPARAM>(rgba));
}
and
DefineXpmSymbol(MARKER_ADDED_SYMBOL, icon_add_16_xpm);
DefineXpmSymbol(MARKER_REMOVED_SYMBOL, icon_sub_16_xpm);
DefineXpmSymbol(MARKER_CHANGED_SYMBOL, icon_diff_16_xpm);
DefineXpmSymbol(MARKER_MOVED_SYMBOL, icon_moved_16_xpm);
with
DefineRgbaSymbol(MARKER_ADDED_SYMBOL, icon_add_16_xpm); // Just change the names here.
DefineRgbaSymbol(MARKER_REMOVED_SYMBOL, icon_sub_16_xpm);
DefineRgbaSymbol(MARKER_CHANGED_SYMBOL, icon_diff_16_xpm);
DefineRgbaSymbol(MARKER_MOVED_SYMBOL, icon_moved_16_xpm);
And how about this slightly cleaner image for Compare Selected Lines?
Thank you. BR
Hello Yaron,
I like the new symbols, thanks for providing them :+1:
They are added in this change: https://github.com/pnedev/compare-plugin/commit/cc315cefaf39d58b39b551e5560a5a26496178f5.
To see them in action use these binaries: x86: https://ci.appveyor.com/api/buildjobs/923f86rayv4m053o/artifacts/ComparePlugin.dll x64: https://ci.appveyor.com/api/buildjobs/ncsglmd7sc38m2cc/artifacts/ComparePlugin.dll
What about the Moved-multiple symbol? Do you think about removing the moved-multiple altogether?
Thanks once again.
BR
Hello Pavel,
I'm glad you like the new icons. And thank you for adding them to CP.
Here are the RGBA files for dark themes (I haven't tested them in CP).
BTW, the new Settings dialog is very nice. π
What about the Moved-multiple symbol?
I've been waiting for your decision in the Move discussion. I thought you wanted to complete the new alignment first.
BR
Hello Yaron,
Thank you once again for the icons. I'm not sure I'll add the dark theme icons. It won't be anytime soon that's for sure. I'll need to add a setting for dark theme as I see it but it will be something for the future.
BTW, the new Settings dialog is very nice.
Thanks.
I've been waiting for your decision in the Move discussion. I thought you wanted to complete the new alignment first.
I'm ready to continue the move discussion in parallel. I guess I'll have to read it again and answer you.
BR
Hello Pavel,
I'm not sure I'll add the dark theme icons. It won't be anytime soon that's for sure...
Sure, whenever you find the time.
I'm ready to continue the move discussion in parallel. I guess I'll have to read it again and answer you.
That would be great. Thank you.
I've had a busy day. With your permission, I'll reply to the other issues tomorrow.
BR
Hello Pavel,
Two minor questions:
Is Compare.ico used anywhere besides the NavBar's bottom tab when two docked windows are open?
If not, could you please replace it with this Compare NavBar.ico? The icon is re-scaled to 14x14 px and it's rather blurry. Also, a NavBar image would be better.
**
The new Settings dialog is very nice. :+1: Thank you.
Wouldn't it be nicer if Move caret on navigation
was placed second (after Warn about...
)?
And Show "Close Files"
last?
BR
Hello Yaron,
Thank you for the suggestions and the new ico. You will find the corresponding changes in the next binary I send you (with the new 'Move detection' implementation I suppose).
BR
Hello,
This is a place for comments, questions, proposals and discussions in general.
BR