Progress Bar about compareplus HOT 23 CLOSED

Hello again,

I've just seen you've modified the Progress Bar code. Thank you.
I wouldn't have posted this issue had I seen it earlier. :)
I'm watching your fork. Am I not supposed to get notified of new commits?

Best regards.

from compareplus.

Hi Yaron,

The features you propose in the link look good I'll see what I can do in the future but this would be 'low priority' task.

The progress bar doesn't show the exact progress so it is kind of approximation. It gives me some very rough feedback on the compare process for now. In the future I might remove it but from user perspective I would like to have some info on how the comparing is going and a way to cancel the process if it is overly-slow.
Why you want the progress removed?

I'm watching your fork. Am I not supposed to get notified of new commits?

Honestly, I have no idea :)

BR

from compareplus.

Hello Pavel,

Thanks for considering my suggestion.
Sure, for a future version.

The most serious problem with the Progress Bar was that the "Cancel" button didn't actually terminate the process.
Fixing that would be a major improvement. I appreciate your work.

I agree there should be some indication of the comparing process.
I hardly compare very large files and for me this feature would only slow down the process.
I can remove the relevant code for my personal use, but I think that many other users may see it as I do.
How about an option "Show Progress Bar"? Would it just complicate things and bloat the plugin?
As I've written before: I trust your judgment. :)

I guess I'll have to manually follow your commits.

Best regards.

from compareplus.

Hi Yaron,

As Notepad++ freezes while compare is ongoing I would prefer to have some indication to the user (progress or something) about the operation. That's why I think it shouldn't be an option.

The Cancel button works fine but the cancel command is actually sampled by the compare operation on certain points. That's why there might be some delay from clicking 'Cancel' to the actual canceling of the compare operation. This is a limitation of the current implementation. I might change that in the future but it's a lot of rework.

I will leave this issue open to point to your status suggestion.

BR,
Pavel

from compareplus.

Like Pavel, I would also prefer progress report, because the comparison blocks NPP.

from compareplus.

Hello Pavel,

Yes, a good solid argument.
And thanks for the explanation regarding the Cancel button. Interesting.
Could you please let me know when you've accomplished the work on the Progress Bar?

Best regards.

from compareplus.

Hi Yaron,

Sure, no problem.
The progress is working fine at the moment so I'm not planning to change it in the near future.

BR,
Pavel

from compareplus.

Accidentally closed the issue, I reopened it again.

from compareplus.

Hello Pavel,

Thanks again for your work. I do appreciate it.

May I ask you to upload the DLL?
I'd rather download your fork later. :)

Best regards.

from compareplus.

Here it is:
ComparePlugin.zip

BR

from compareplus.

Hello Pavel,

Thanks for uploading the DLL.

The Cancel button works like a charm and the new design is much nicer.
A great job! Thank you so much.

Some minor issues:

1. NPP (or the Files Match dialog) don't get the focus after comparing (the Title Bar is gray).
2. Clear NPP Recent Files list. Open and compare two files. -- NPP would minimize to the Task Bar.
3. I'd use "Comparing A vs. B".

BTW, nice addition to the Files Match dialog. Thanks.
And the crash is fixed. 👍

Best regards.

from compareplus.

Hi Yaron,

Thanks for the feedback.
1 and 3 are fixed, can you try if 2 is still present? Thanks!
ComparePlugin.zip

BR

from compareplus.

Hello Pavel,

Thanks again. I appreciate your time and work.
I'm beginning to have second thoughts about removing the Progress Bar. :) Really nice.

No. 1: The Title Bar is active but mouse-scrolling over either view doesn't work until pressing somewhere in the editor.
BTW, I remember having the same issue when UFO first integrated the Progress Bar.

No. 2 is fixed. 👍

No. 3: Please don't mind that: I meant "Comparing" only in the Progress Bar.
IMO it would be nicer if you wrapped the file names with double quotes. Adding an ellipsis would be good too.

• Comparing "File1" vs. "File2"...

EDIT:

In the Files Match dialog:

• [Title] Compare Plugin: Files Match
• The files "File1" and "File2" match.
• Close compared files?

What do you think?

Best regards.

from compareplus.

Hello again Pavel,

I've just intercepted UFO's reply to the mouse-scrolling issue:

That was fixed yesterday, tho not checked in yet. The N++ window must be re-enabled before closing the progress dialog. "SetForegroundWindow" and "SetFocus" aren't necessary then.

Could you please have a look at two blocks (search for // Hi Pavel) in the attached Compare.cpp?
If I remember correctly, it solved the problem.

Compare.zip

Thank you.

from compareplus.

Hi Yaron,

I did the fixes according your suggestions.
Please check the attached DLL - it contains all fixes so far. Write back if there are issues, thanks.
ComparePlugin.zip

BR,
Pavel

from compareplus.

Hello Pavel,

A brilliant work.
Thank you so much. I'm really grateful.

There's still one issue (sorry):
Open two files.
Move to Other View (the file at bottom is active).
Compare.

Result:
The file at top is active.

And excuse my perfectionism. :)
After your important addition to the Files Match dialog, the "match" word is bit lost.
How about making the title "Compare Plugin: Files Match"? - Just to make it instantly more prominent.

And wouldn't it be nicer if you add another "\n" before "Close compared files?"?
TEXT("The files \"%s\" and \"%s\" match.\n\nClose compared files?")
if (::MessageBox(nppData._nppHandle, buffer, TEXT("Compare Plugin: Files Match")

Best regards.

from compareplus.

Hello,

I've just seen this commit. Thanks again!

I haven't followed all your changes but allow me to ask this:

    progCounter = 0;
    progDlg = new CProgress();
    progDlg->Open(nppData._nppHandle, TEXT("Compare Plugin"));
    progDlg->SetInfo(buffer);

    UpdateWindow(nppData._scintillaMainHandle);
    UpdateWindow(nppData._scintillaSecondHandle);
    EnableWindow(nppData._nppHandle, FALSE);

    EnableWindow(nppData._nppHandle, TRUE);

    bool compareCanceled = progDlg->IsCancelled();
    progDlg->Close();
    delete progDlg;

    UpdateWindow(nppData._nppHandle);

Are the UpdateWindowcalls necessary due to your modifications?

BR.

from compareplus.

Hi Yaron,

The code excerpt

UpdateWindow(nppData._scintillaMainHandle); UpdateWindow(nppData._scintillaSecondHandle);

was needed before to refresh the scintilla views but it seems redundant now. I've removed it.

About the focus issue - this is a regression I've introduced while fixing the Progress. It is corrected now.

Message text suggestion is also implemented.

Thanks for testing and reporting, all is fixed in master branch.

BR

from compareplus.

Hello Pavel,

I've downloaded your fork. :)
I'd like to thank you again. You've done a remarkable job!

It seems that comparing large or completely different files is faster with your version.
Could it be related to the memory allocation handling?

And the DLL size is much smaller which is a good indication too.

With your permission two questions regarding the compiling:

1. I had to copy "compare.ico" to the "res" folder (I got an error without it).
2. I get a warning "Too many parameters" for log(TEXT("%s %s %s", argv[0], argv[1], argv[2])); in "Loader.cpp".

Thank you for all the fixes. I appreciate your patience.
The Progress bar is great and I'm definitely going to keep it. :)

Regarding

    // Save current N++ focus
    HWND hwnd = GetFocus();

    SetFocus(nppData._scintillaMainHandle);
    SendMessage(nppData._nppHandle, NPPM_GETFILENAME, 0, (LPARAM)filenameMain);
    SetFocus(nppData._scintillaSecondHandle);
    SendMessage(nppData._nppHandle, NPPM_GETFILENAME, 0, (LPARAM)filenameSecond);

    // Restore N++ focus
    SetFocus(hwnd);

Now that we're implementing SendMessage(nppData._nppHandle, WM_COMMAND, IDM_VIEW_TAB_PREV, 0); wouldn't it be simpler to get the file names in that section and pass them to compareNew()?
If we compare in Two-Views mode we would still need the HWND hwnd = GetFocus(); block (done also in startCompare()), but in One-View mode it would save us some extra switches between the views.

I'll do that if you think it's a good idea.

BTW, does MAX_PATH mean (as it seems) full path or just file name?
IOW, is the 256 characters limit to the file name alone or name + path?
Thanks.

Best regards.

from compareplus.

Hello Yaron,

Could it be related to the memory allocation handling?

Yes, most probably.

With your permission two questions regarding the compiling:

1. I had to copy "compare.ico" to the "res" folder (I got an error without it).
2. I get a warning "Too many parameters" for log(TEXT("%s %s %s", argv[0], argv[1], argv[2])); in "Loader.cpp".

I haven't built the Loader project after my code refactoring, thus I haven't noticed that problem. Sorry about that. I'll fix it ASAP.
Do you use the Loader by the way? I'm not quite interested in it at the moment and will not be fixing issues related to it at least for now. Only what is related to the refactoring.

I'll do that if you think it's a good idea.

Perhaps, I don't know for sure. Things are a bit messy at the moment and I don't have a clear idea yet how different parts of code interact. For example "Compare to last save" is not working now perhaps because of the changes we introduced. I'll need some time.
So let's hold this for a while, thanks.

BTW, does MAX_PATH mean (as it seems) full path or just file name?

It is the full path.

BR,
Pavel

from compareplus.

Hello Pavel,

Thank you for fixing the compilation issues.
I've never used the Loader. I didn't know there was an option to build the DLL only.

Things are a bit messy at the moment and I don't have a clear idea yet how different parts of code interact.

I think that's natural even for an experienced and smart developer.
I have leafed through your various commits, and was impressed by the amount of work you have been able to accomplish without even using the plugin before.

For example "Compare to last save" is not working now

I rarely use this command.

BTW, NPP crashes in some rare cases when using “Compare to last save”.
I hardly use that command and haven’t properly tested it.

https://notepad-plus-plus.org/community/topic/11307/plug-in-compare-bug/9

I don't know if you've seen this issue.
(Apropos: should I copy some of the issues we've opened in jsleroy's repo to your fork? - Just to have them all in one place).

Could you please list the STR to the problem you've found in v15.6.8?

So let's hold this for a while.

Sure.

And thanks also for the "It is the full path" reply.

Best regards.

from compareplus.

Hello Yaron,

I haven't seen the issue you pointed - it seems it is a known problem. It needs fixing then :)

Apropos: should I copy some of the issues we've opened in jsleroy's repo to your fork? - Just to have them all in one place

Yes, that is a good idea, thanks.

I'm closing this issue now, feel free to create a new one for the compare status implementation.
The idea is to keep the issue list as clear and concise as possible so we can easily see what remains to be done.

Thanks.

BR

from compareplus.

It needs fixing then :)

Indeed. :)

Thanks again.

from compareplus.