Logged In: YES
user_id=631874
Originator: NO

And the MFC parser is also buggy -- see the workarounds in current code.

This patch seems to solve many problems in a very nice way. I like this patch.

Can you also remove the empty MergeCmdLineInfo class destructor.

You can commit this to trunk, and after few days to 2.10 branch too. Only thing I'm a bit nervous about is those single-instance-related changes. But they also look simple enough that there should not be any problems.

Yep, I want this to 2.10 branch as it solves those idiotic problems we've had with "\" and quotation marks ending paths and "/" always recognized as switch. At least how I understood the code, "/" should not be a problem with this patch, right? (See comments in current WinMergeCmdLineParser.cpp, around line 159)

Logged In: YES
user_id=766060
Originator: YES

Completed: At revision: 5862

> ... single-instance-related changes

An arguable effect of this patch is that it limits single instance logic to instances using the same character encoding environment (i.e. Unicode vs. ANSI).

Logged In: YES
user_id=631874
Originator: NO

> An arguable effect of this patch is that it
> limits single instance logic to instances
> using the same character encoding environment
> (i.e. Unicode vs. ANSI).

I don't see this as a problem. We don't install both ANSI/Unicode executables to any configuration. And I don't see a situation where one wants to run both...

Logged In: YES
user_id=631874
Originator: NO

Can you commit this to 2.10 branch?

Logged In: YES
user_id=766060
Originator: YES

Completed: At revision: 5892

Logged In: YES
user_id=954028
Originator: NO

This patch is really nice. But there is a problem related to plugins (like CompareMSExcelFiles.dll).

To reproduce:
1. Check 'Allow only one instance to run' in Option dialog.
2. Select 'Plugins' menu -> 'Automatic Unpacking' menu item.
3. Run the second instance with command line parameters as follows.

WinMergeU.exe c:\test1.xls c:\test2.xls

Expected result:
WinMerge compares these excel workbooks using CompareMSExcelFiles.dll.

Actual result:
CompareMSExcelFiles.dll plugin didn't work.

You deleted WM_USER message handler. But it is needed to handle OLE plugin correctly.

Index: MainFrm.cpp

--- MainFrm.cpp (revision 5917)
+++ MainFrm.cpp (working copy)
@@ -203,6 +203,7 @@
ON_COMMAND(ID_VIEW_RESIZE_PANES, OnResizePanes)
ON_COMMAND(ID_FILE_OPENPROJECT, OnFileOpenproject)
ON_MESSAGE(WM_COPYDATA, OnCopyData)
+ ON_MESSAGE(WM_USER, OnUser)
ON_COMMAND(ID_WINDOW_CLOSEALL, OnWindowCloseAll)
ON_UPDATE_COMMAND_UI(ID_WINDOW_CLOSEALL, OnUpdateWindowCloseAll)
ON_COMMAND(ID_FILE_SAVEPROJECT, OnSaveProject)
@@ -2815,13 +2816,22 @@
LRESULT CMainFrame::OnCopyData(WPARAM wParam, LPARAM lParam)
{
COPYDATASTRUCT *pCopyData = (COPYDATASTRUCT*)lParam;
- LPCTSTR pchData = (LPCTSTR)pCopyData->lpData;
// Bail out if data isn't zero-terminated
DWORD cchData = pCopyData->cbData / sizeof(TCHAR);
- if (cchData == 0 || pchData[cchData - 1] != _T('\0'))
+ if (cchData == 0 || ((LPCTSTR)(pCopyData->lpData))[cchData - 1] != _T('\0'))
return FALSE;
+ LPTSTR pchData = new TCHAR[cchData];
+ memcpy(pchData, pCopyData->lpData, pCopyData->cbData);
+ PostMessage(WM_USER, (WPARAM)pchData, (LPARAM)0);
+ return TRUE;
+}
+
+LRESULT CMainFrame::OnUser(WPARAM wParam, LPARAM lParam)
+{
+ LPCTSTR pchData = (LPCTSTR)wParam;
MergeCmdLineInfo cmdInfo = pchData;
theApp.ParseArgsAndDoOpen(cmdInfo, this);
+ delete pchData;
return TRUE;
}

Index: MainFrm.h

--- MainFrm.h (revision 5917)
+++ MainFrm.h (working copy)
@@ -324,6 +324,7 @@
afx_msg void OnResizePanes();
afx_msg void OnFileOpenproject();
afx_msg LRESULT OnCopyData(WPARAM wParam, LPARAM lParam);
+ afx_msg LRESULT OnUser(WPARAM wParam, LPARAM lParam);
afx_msg void OnTimer(UINT_PTR nIDEvent);
afx_msg void OnWindowCloseAll();
afx_msg void OnUpdateWindowCloseAll(CCmdUI* pCmdUI);

Logged In: YES
user_id=631874
Originator: NO

Oh. Please commit that fix to trunk and 2.10 branch. I'm planning to do a 2.10 RC tomorrow so this would be nice to have fixed.

Logged In: YES
user_id=954028
Originator: NO

Committed to SVN trunk. Completed: At revision: 5923
Committed to 2.10 branch. Completed: At revision: 5924

Logged In: YES
user_id=766060
Originator: YES

A simpler fix is to ReplyMessage(TRUE) before attempting to deal with plugins:

--- C:/svnroot2.2k3/trunk.svn/Src/MainFrm.cpp Fri Aug 29 22:38:45 2008
+++ C:/svnroot2.2k3/trunk/Src/MainFrm.cpp Mon Sep 08 01:34:28 2008
@@ -2820,6 +2820,7 @@
DWORD cchData = pCopyData->cbData / sizeof(TCHAR);
if (cchData == 0 || pchData[cchData - 1] != _T('\0'))
return FALSE;
+ ReplyMessage(TRUE);
MergeCmdLineInfo cmdInfo = pchData;
theApp.ParseArgsAndDoOpen(cmdInfo, this);
return TRUE;

Logged In: YES
user_id=954028
Originator: NO

Oh! It is far better than my patch.

Logged In: YES
user_id=631874
Originator: NO

That is good for trunk, but lets keep the 2.10 branch "old way".

Committed Jochen's fix to SVN trunk. Completed: At revision: 5932

Matthias Mayer sent me this fix by e-mail:

His comment was:
"if no parameter is there, at runtime this param.assign(p, q - p); breaks."

Looks like a proper change.

Index: MergeCmdLineInfo.cpp

--- MergeCmdLineInfo.cpp (revision 5932)
+++ MergeCmdLineInfo.cpp (working copy)
@@ -67,8 +67,8 @@
*flag = false;
flag = 0;
}
+ param.assign(p, q - p);
}
- param.assign(p, q - p);
if (q > p && flag)
{
CharLower(&*param.begin());

Matthias patch below committed to SVN trunk:
Completed: At revision: 5949

Revision 5949 broke flags accepting arguments (such as -dl, -dr).

Completed: At revision: 5955

So assigning zero characters from a null pointer to an STL string is probably illegal...

This is now in experimental and stable release. So time to close this patch. Further improvements and fixes in separate patches.