Skip to content

Commit

Permalink
Fix issue #1985: Wimerge saves changes to the wrong file (#1988)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sdottaka
sdottaka authored Aug 19, 2023
1 parent 409280b commit a121466
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 90 deletions.
36 changes: 28 additions & 8 deletions Src/HexMergeDoc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,20 +231,20 @@ bool CHexMergeDoc::PromptAndSaveIfNeeded(bool bAllowCancel)
if (!bAllowCancel)
dlg.m_bDisableCancel = true;
if (!pathLeft.empty())
dlg.m_sLeftFile = pathLeft;
dlg.m_sLeftFile = m_strSaveAsPath.empty() ? pathLeft : m_strSaveAsPath;
else
dlg.m_sLeftFile = m_strDesc[0];
dlg.m_sLeftFile = m_strSaveAsPath.empty() ? m_strDesc[0] : m_strSaveAsPath;
if (m_nBuffers == 3)
{
if (!pathMiddle.empty())
dlg.m_sMiddleFile = pathMiddle;
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? pathMiddle : m_strSaveAsPath;
else
dlg.m_sMiddleFile = m_strDesc[1];
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? m_strDesc[1] : m_strSaveAsPath;
}
if (!pathRight.empty())
dlg.m_sRightFile = pathRight;
dlg.m_sRightFile = m_strSaveAsPath.empty() ? pathRight : m_strSaveAsPath;
else
dlg.m_sRightFile = m_strDesc[1];
dlg.m_sRightFile = m_strSaveAsPath.empty() ? m_strDesc[1] : m_strSaveAsPath;

if (dlg.DoModal() == IDOK)
{
Expand Down Expand Up @@ -329,15 +329,35 @@ bool CHexMergeDoc::DoFileSave(int nBuffer)
result = DoFileSaveAs(nBuffer);
else
{
const String &path = m_filePaths.GetPath(nBuffer);
HRESULT hr = m_pView[nBuffer]->SaveFile(path.c_str());
String strSavePath = m_strSaveAsPath.empty() ? m_filePaths.GetPath(nBuffer) : m_strSaveAsPath;
// Warn user in case file has been changed by someone else
if (m_pView[nBuffer]->IsFileChangedOnDisk(m_filePaths.GetPath(nBuffer).c_str()) == IMergeDoc::FileChange::Changed)
{
String msg = strutils::format_string1(_("Another application has updated file\n%1\nsince WinMerge loaded it.\n\nOverwrite changed file?"), m_filePaths.GetPath(nBuffer));
if (AfxMessageBox(msg.c_str(), MB_ICONWARNING | MB_YESNO) == IDNO)
return false;
}
// Ask user what to do about FILE_ATTRIBUTE_READONLY
String strPath = strSavePath;
bool bApplyToAll = false;
if (CMergeApp::HandleReadonlySave(strPath, false, bApplyToAll) == IDCANCEL)
return false;
strSavePath = strPath.c_str();
// Take a chance to create a backup
if (!CMergeApp::CreateBackup(false, strSavePath))
return false;
HRESULT hr = m_pView[nBuffer]->SaveFile(strSavePath.c_str());
if (Try(hr) == IDCANCEL)
return false;
if (FAILED(hr))
return DoFileSaveAs(nBuffer);
result = true;
if (result)
{
m_filePaths[nBuffer] = strSavePath;
UpdateHeaderPath(nBuffer);
UpdateDiffItem(m_pDirDoc);
}
}
}
return result;
Expand Down
4 changes: 4 additions & 0 deletions Src/HexMergeDoc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ class CHexMergeDoc : public CDocument, public IMergeDoc
String GetDescription(int pane) const override { return m_strDesc[pane]; };
void SetDescription(int pane, const String& strDesc) { m_strDesc[pane] = strDesc; };
void SaveAs(int nBuffer, bool packing = true) { DoFileSaveAs(nBuffer, packing); }
String GetSaveAsPath() const { return m_strSaveAsPath; }
void SetSaveAsPath(const String& strSaveAsPath) { m_strSaveAsPath = strSaveAsPath; }

private:
bool DoFileSave(int nBuffer);
bool DoFileSaveAs(int nBuffer, bool packing = true);
Expand All @@ -99,6 +102,7 @@ class CHexMergeDoc : public CDocument, public IMergeDoc
String m_strDesc[3]; /**< Left/right side description text */
BUFFERTYPE m_nBufferType[3];
PackingInfo m_infoUnpacker;
String m_strSaveAsPath; /**< "3rd path" where output saved if given */

// Generated message map functions
protected:
Expand Down
16 changes: 0 additions & 16 deletions Src/HexMergeView.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,22 +318,6 @@ HRESULT CHexMergeView::LoadFile(const tchar_t* path)
*/
HRESULT CHexMergeView::SaveFile(const tchar_t* path, bool packing)
{
// Warn user in case file has been changed by someone else
if (IsFileChangedOnDisk(path) == IMergeDoc::FileChange::Changed)
{
String msg = strutils::format_string1(_("Another application has updated file\n%1\nsince WinMerge loaded it.\n\nOverwrite changed file?"), path);
if (AfxMessageBox(msg.c_str(), MB_ICONWARNING | MB_YESNO) == IDNO)
return E_FAIL;
}
// Ask user what to do about FILE_ATTRIBUTE_READONLY
String strPath = path;
bool bApplyToAll = false;
if (CMergeApp::HandleReadonlySave(strPath, false, bApplyToAll) == IDCANCEL)
return E_FAIL;
path = strPath.c_str();
// Take a chance to create a backup
if (!CMergeApp::CreateBackup(false, path))
return E_FAIL;
// Write data to an intermediate file
String tempPath = env::GetTemporaryPath();
String sIntermediateFilename = env::GetTemporaryFileName(tempPath, _T("MRG_"), 0);
Expand Down
32 changes: 10 additions & 22 deletions Src/ImgMergeFrm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,14 +722,17 @@ bool CImgMergeFrame::DoFileSave(int pane)
return false;
CMergeApp::CreateBackup(false, m_filePaths[pane]);
int savepoint = m_pImgMergeWindow->GetSavePoint(pane);
if (!m_pImgMergeWindow->SaveImage(pane))
bool result = m_strSaveAsPath.empty() ? m_pImgMergeWindow->SaveImage(pane) : m_pImgMergeWindow->SaveImageAs(pane, m_strSaveAsPath.c_str());
if (!result)
{
String str = strutils::format_string2(_("Saving file failed.\n%1\n%2\nDo you want to:\n\t- use a different filename (Press OK)\n\t- abort the current operation (Press Cancel)?"), filename, GetSysError());
int answer = AfxMessageBox(str.c_str(), MB_OKCANCEL | MB_ICONWARNING);
if (answer == IDOK)
return DoFileSaveAs(pane);
return false;
}
if (!m_strSaveAsPath.empty())
m_filePaths[pane] = m_strSaveAsPath;
if (filename != m_filePaths[pane])
{
if (!m_infoUnpacker.Packing(filename, m_filePaths[pane], m_unpackerSubcodes[pane], { m_filePaths[pane] }))
Expand Down Expand Up @@ -1214,35 +1217,20 @@ bool CImgMergeFrame::PromptAndSaveIfNeeded(bool bAllowCancel)
if (!bAllowCancel)
dlg.m_bDisableCancel = true;
if (!m_filePaths.GetLeft().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sLeftFile = m_filePaths.GetLeft();
else
dlg.m_sLeftFile = theApp.m_strSaveAsPath;
}
dlg.m_sLeftFile = m_strSaveAsPath.empty() ? m_filePaths.GetLeft() : m_strSaveAsPath;
else
dlg.m_sLeftFile = m_strDesc[0];
dlg.m_sLeftFile = m_strSaveAsPath.empty() ?m_strDesc[0] : m_strSaveAsPath;
if (m_pImgMergeWindow->GetPaneCount() == 3)
{
if (!m_filePaths.GetMiddle().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sMiddleFile = m_filePaths.GetMiddle();
else
dlg.m_sMiddleFile = theApp.m_strSaveAsPath;
}
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? m_filePaths.GetMiddle() : m_strSaveAsPath;
else
dlg.m_sMiddleFile = m_strDesc[1];
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? m_strDesc[1] : m_strSaveAsPath;
}
if (!m_filePaths.GetRight().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sRightFile = m_filePaths.GetRight();
else
dlg.m_sRightFile = theApp.m_strSaveAsPath;
}
dlg.m_sRightFile = m_strSaveAsPath.empty() ? m_filePaths.GetRight() : m_strSaveAsPath;
else
dlg.m_sRightFile = m_strDesc[m_pImgMergeWindow->GetPaneCount() - 1];
dlg.m_sRightFile = m_strSaveAsPath.empty() ? m_strDesc[m_pImgMergeWindow->GetPaneCount() - 1] : m_strSaveAsPath;

if (dlg.DoModal() == IDOK)
{
Expand Down
3 changes: 3 additions & 0 deletions Src/ImgMergeFrm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class CImgMergeFrame : public CMergeFrameCommon,public IMergeDoc
void CheckFileChanged(void) override;
String GetDescription(int pane) const override { return m_strDesc[pane]; }
static bool IsLoadable();
String GetSaveAsPath() const { return m_strSaveAsPath; }
void SetSaveAsPath(const String& strSaveAsPath) { m_strSaveAsPath = strSaveAsPath; }

// Attributes
protected:
Expand Down Expand Up @@ -121,6 +123,7 @@ class CImgMergeFrame : public CMergeFrameCommon,public IMergeDoc
BUFFERTYPE m_nBufferType[3];
DiffFileInfo m_fileInfo[3];
bool m_bRO[3];
String m_strSaveAsPath; /**< "3rd path" where output saved if given */
bool m_bAutoMerged;
CDirDoc *m_pDirDoc;
int m_nActivePane;
Expand Down
16 changes: 13 additions & 3 deletions Src/MainFrm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,9 @@ bool CMainFrame::ShowTextOrTableMergeDoc(std::optional<bool> table, CDirDoc * pD
true,
pOpenParams ? pOpenParams->m_char: -1);

if (pOpenParams && !pOpenParams->m_strSaveAsPath.empty())
pMergeDoc->SetSaveAsPath(pOpenParams->m_strSaveAsPath);

if (!sReportFile.empty())
pMergeDoc->GenerateReport(sReportFile);

Expand Down Expand Up @@ -1015,6 +1018,9 @@ bool CMainFrame::ShowHexMergeDoc(CDirDoc * pDirDoc, int nFiles, const FileLocati

pHexMergeDoc->MoveOnLoad(GetActivePaneFromFlags(nFiles, dwFlags));

if (pOpenParams && !pOpenParams->m_strSaveAsPath.empty())
pHexMergeDoc->SetSaveAsPath(pOpenParams->m_strSaveAsPath);

if (!sReportFile.empty())
pHexMergeDoc->GenerateReport(sReportFile);

Expand Down Expand Up @@ -1044,6 +1050,9 @@ bool CMainFrame::ShowImgMergeDoc(CDirDoc * pDirDoc, int nFiles, const FileLocati

pImgMergeFrame->MoveOnLoad(GetActivePaneFromFlags(nFiles, dwFlags));

if (pOpenParams && !pOpenParams->m_strSaveAsPath.empty())
pImgMergeFrame->SetSaveAsPath(pOpenParams->m_strSaveAsPath);

if (!sReportFile.empty())
pImgMergeFrame->GenerateReport(sReportFile);

Expand Down Expand Up @@ -2794,15 +2803,16 @@ bool CMainFrame::DoOpenConflict(const String& conflictFile, const String strDesc
{
// Open two parsed files to WinMerge, telling WinMerge to
// save over original file (given as third filename).
theApp.m_strSaveAsPath = conflictFile;
OpenTextFileParams openParams;
openParams.m_strSaveAsPath = conflictFile;
if (!threeWay)
{
String strDesc2[2] = {
(strDesc && !strDesc[0].empty()) ? strDesc[0] : _("Theirs File"),
(strDesc && !strDesc[2].empty()) ? strDesc[2] : _("Mine File") };
fileopenflags_t dwFlags[2] = {FFILEOPEN_READONLY | FFILEOPEN_NOMRU, FFILEOPEN_NOMRU | FFILEOPEN_MODIFIED};
PathContext tmpPathContext(revFile, workFile);
conflictCompared = DoFileOrFolderOpen(&tmpPathContext, dwFlags, strDesc2);
conflictCompared = DoFileOrFolderOpen(&tmpPathContext, dwFlags, strDesc2, L"", false, nullptr, nullptr, nullptr, 0, &openParams);
}
else
{
Expand All @@ -2812,7 +2822,7 @@ bool CMainFrame::DoOpenConflict(const String& conflictFile, const String strDesc
(strDesc && !strDesc[2].empty()) ? strDesc[2] : _("Mine File") };
PathContext tmpPathContext(baseFile, revFile, workFile);
fileopenflags_t dwFlags[3] = {FFILEOPEN_READONLY | FFILEOPEN_NOMRU, FFILEOPEN_READONLY | FFILEOPEN_NOMRU, FFILEOPEN_NOMRU | FFILEOPEN_MODIFIED};
conflictCompared = DoFileOrFolderOpen(&tmpPathContext, dwFlags, strDesc3);
conflictCompared = DoFileOrFolderOpen(&tmpPathContext, dwFlags, strDesc3, L"", false, nullptr, nullptr, nullptr, 0, &openParams);
}
}
else
Expand Down
3 changes: 3 additions & 0 deletions Src/MainFrm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class CMainFrame : public CMDIFrameWnd
int m_line = -1;
int m_char = -1;
String m_fileExt;
String m_strSaveAsPath; /**< "3rd path" where output saved if given */
};

struct OpenTableFileParams : public OpenTextFileParams
Expand All @@ -98,13 +99,15 @@ class CMainFrame : public CMDIFrameWnd
{
virtual ~OpenBinaryFileParams() {}
int m_address = -1;
String m_strSaveAsPath; /**< "3rd path" where output saved if given */
};

struct OpenImageFileParams : public OpenFileParams
{
virtual ~OpenImageFileParams() {}
int m_x = -1;
int m_y = -1;
String m_strSaveAsPath; /**< "3rd path" where output saved if given */
};

struct OpenWebPageParams : public OpenFileParams
Expand Down
12 changes: 10 additions & 2 deletions Src/Merge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,6 @@ bool CMergeApp::ParseArgsAndDoOpen(MergeCmdLineInfo& cmdInfo, CMainFrame* pMainF
m_bExitIfNoDiff = cmdInfo.m_bExitIfNoDiff;
m_bEscShutdown = cmdInfo.m_bEscShutdown;

m_strSaveAsPath = cmdInfo.m_sOutputpath;

strDesc[0] = cmdInfo.m_sLeftDesc;
if (cmdInfo.m_Files.GetSize() < 3)
{
Expand All @@ -877,13 +875,23 @@ bool CMergeApp::ParseArgsAndDoOpen(MergeCmdLineInfo& cmdInfo, CMainFrame* pMainF
pOpenTextFileParams->m_line = cmdInfo.m_nLineIndex;
pOpenTextFileParams->m_char = cmdInfo.m_nCharIndex;
pOpenTextFileParams->m_fileExt = cmdInfo.m_sFileExt;
pOpenTextFileParams->m_strSaveAsPath = cmdInfo.m_sOutputpath;

}
if (auto* pOpenTableFileParams = dynamic_cast<CMainFrame::OpenTableFileParams*>(pOpenParams.get()))
{
pOpenTableFileParams->m_tableDelimiter = cmdInfo.m_cTableDelimiter;
pOpenTableFileParams->m_tableQuote = cmdInfo.m_cTableQuote;
pOpenTableFileParams->m_tableAllowNewlinesInQuotes = cmdInfo.m_bTableAllowNewlinesInQuotes;
}
if (auto* pOpenBinaryFileParams = dynamic_cast<CMainFrame::OpenBinaryFileParams*>(pOpenParams.get()))
{
pOpenBinaryFileParams->m_strSaveAsPath = cmdInfo.m_sOutputpath;
}
if (auto* pOpenImageFileParams = dynamic_cast<CMainFrame::OpenImageFileParams*>(pOpenParams.get()))
{
pOpenImageFileParams->m_strSaveAsPath = cmdInfo.m_sOutputpath;
}
if (cmdInfo.m_Files.GetSize() > 2)
{
cmdInfo.m_dwLeftFlags |= FFILEOPEN_CMDLINE;
Expand Down
1 change: 0 additions & 1 deletion Src/Merge.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class CMergeApp : public CWinApp
std::unique_ptr<CLanguageSelect> m_pLangDlg;
std::unique_ptr<SyntaxColors> m_pSyntaxColors; /**< Syntax color container */
std::unique_ptr<CCrystalTextMarkers> m_pMarkers; /**< Marker container */
String m_strSaveAsPath; /**< "3rd path" where output saved if given */
bool m_bEscShutdown; /**< If commandline switch -e given ESC closes appliction */
SyntaxColors * GetMainSyntaxColors() { return m_pSyntaxColors.get(); }
CCrystalTextMarkers * GetMainMarkers() const { return m_pMarkers.get(); }
Expand Down
46 changes: 8 additions & 38 deletions Src/MergeDoc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1610,7 +1610,7 @@ bool CMergeDoc::TrySaveAs(String &strPath, int &nSaveResult, String & sError,
if (nSaveResult == SAVE_DONE)
{
// We are saving scratchpad (unnamed file)
if (strPath.empty())
if (m_nBufferType[nBuffer] == BUFFERTYPE::UNNAMED)
{
m_nBufferType[nBuffer] = BUFFERTYPE::UNNAMED_SAVED;
m_strDesc[nBuffer].erase();
Expand Down Expand Up @@ -1657,7 +1657,7 @@ bool CMergeDoc::TrySaveAs(String &strPath, int &nSaveResult, String & sError,
bool CMergeDoc::DoSave(const tchar_t* szPath, bool &bSaveSuccess, int nBuffer)
{
DiffFileInfo fileInfo;
String strSavePath(szPath);
String strSavePath(m_strSaveAsPath.empty() ? szPath : m_strSaveAsPath);
FileChange fileChanged;
bool bApplyToAll = false;
int nRetVal = -1;
Expand All @@ -1680,21 +1680,6 @@ bool CMergeDoc::DoSave(const tchar_t* szPath, bool &bSaveSuccess, int nBuffer)

bSaveSuccess = false;

// Check third arg possibly given from command-line
if (!theApp.m_strSaveAsPath.empty())
{
if (paths::DoesPathExist(theApp.m_strSaveAsPath) == paths::IS_EXISTING_DIR)
{
// third arg was a directory, so get append the filename
String sname;
paths::SplitFilename(szPath, 0, &sname, 0);
strSavePath = theApp.m_strSaveAsPath;
strSavePath = paths::ConcatPath(strSavePath, sname);
}
else
strSavePath = theApp.m_strSaveAsPath;
}

nRetVal = CMergeApp::HandleReadonlySave(strSavePath, false, bApplyToAll);
if (nRetVal == IDCANCEL)
return false;
Expand Down Expand Up @@ -2644,35 +2629,20 @@ bool CMergeDoc::PromptAndSaveIfNeeded(bool bAllowCancel)
if (!bAllowCancel)
dlg.m_bDisableCancel = true;
if (!m_filePaths.GetLeft().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sLeftFile = m_filePaths.GetLeft();
else
dlg.m_sLeftFile = theApp.m_strSaveAsPath;
}
dlg.m_sLeftFile = m_strSaveAsPath.empty() ? m_filePaths.GetLeft() : m_strSaveAsPath;
else
dlg.m_sLeftFile = m_strDesc[0];
dlg.m_sLeftFile = m_strSaveAsPath.empty() ? m_strDesc[0] : m_strSaveAsPath;
if (m_nBuffers == 3)
{
if (!m_filePaths.GetMiddle().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sMiddleFile = m_filePaths.GetMiddle();
else
dlg.m_sMiddleFile = theApp.m_strSaveAsPath;
}
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? m_filePaths.GetMiddle() : m_strSaveAsPath;
else
dlg.m_sMiddleFile = m_strDesc[1];
dlg.m_sMiddleFile = m_strSaveAsPath.empty() ? m_strDesc[1] : m_strSaveAsPath;
}
if (!m_filePaths.GetRight().empty())
{
if (theApp.m_strSaveAsPath.empty())
dlg.m_sRightFile = m_filePaths.GetRight();
else
dlg.m_sRightFile = theApp.m_strSaveAsPath;
}
dlg.m_sRightFile = m_strSaveAsPath.empty() ?m_filePaths.GetRight() : m_strSaveAsPath;
else
dlg.m_sRightFile = m_strDesc[m_nBuffers - 1];
dlg.m_sRightFile = m_strSaveAsPath.empty() ? m_strDesc[m_nBuffers - 1] : m_strSaveAsPath;

if (dlg.DoModal() == IDOK)
{
Expand Down
Loading

0 comments on commit a121466

Please sign in to comment.