From 51519fc848e23494510d8c8268f7a734c1de3e97 Mon Sep 17 00:00:00 2001 From: GreyMerlin Date: Sat, 16 Jun 2018 17:11:17 -0700 Subject: [PATCH] Improve compare+display of last lines in files (1) Symptoms ... ASSERT could occur (with Debug build), and possibly a fault could occur with Release builds, if longest file was the Left File and that file did not have an explicit EOL marking after the last line of the file, and there were visible differences at the very end of the file. Discussion ... The internal `m_ptBuf[]->m_aLines[]` buffers could end up with different numbers of line entries in situations where the last line of a file had no explicit EOL marking. Normally, the two (or three) `m_aLines[]` buffers would have "ghost" line entries to represent "missing" lines, so that all `m_aLines[]` buffers would be of the same length. The lack of a final EOL could confuse that logic. At some points in the comparison logic, the length used for processing the two (or three) buffers is determined simply by the length of `m_aLines[0]` Solution ... By the end of the `PrimeTextBuffers()` procedure, the two (or three) `m_ptBuf[]->m_aLines[]` buffers will now **always** have the same length. Furthermore, there will no longer be entries at the end of the `m_aLines[]` arrays. These will now be deleted (if **all** `m_aLines[]` buffers have them), or they will be replaced with appropriate "ghost" lines (if only **some** of the buffers have final entries) Also, a number of comments are improved. --- .../crystaledit/editlib/ccrystaltextbuffer.cpp | 2 +- Src/DiffList.cpp | 5 +- Src/DiffList.h | 2 +- Src/DiffWrapper.cpp | 2 +- Src/GhostTextBuffer.cpp | 2 + Src/MergeDoc.cpp | 81 +++++++++++++++------- Src/MergeLineFlags.h | 3 +- 7 files changed, 67 insertions(+), 30 deletions(-) diff --git a/Externals/crystaledit/editlib/ccrystaltextbuffer.cpp b/Externals/crystaledit/editlib/ccrystaltextbuffer.cpp index 05ba8160f..5efae45d2 100644 --- a/Externals/crystaledit/editlib/ccrystaltextbuffer.cpp +++ b/Externals/crystaledit/editlib/ccrystaltextbuffer.cpp @@ -242,7 +242,7 @@ AppendLine (int nLineIndex, LPCTSTR pszChars, size_t nLength ) * Example#1: * MoveLine(5,7,100) * line1=5 - * line2=7 + * line2=10 * newline1=100 * ldiff=95 * l=10 lines[105] = lines[10] diff --git a/Src/DiffList.cpp b/Src/DiffList.cpp index 1935540a0..92c211826 100644 --- a/Src/DiffList.cpp +++ b/Src/DiffList.cpp @@ -894,7 +894,7 @@ void DiffList::Swap(int index1, int index2) * @param [out] nLeftLines Number of lines to add to left side. * @param [out] nRightLines Number of lines to add to right side. */ -void DiffList::GetExtraLinesCounts(int nFiles, int extras[]) +void DiffList::GetExtraLinesCounts(int nFiles, int extras[3]) { extras[0]=0; extras[1]=0; @@ -908,7 +908,8 @@ void DiffList::GetExtraLinesCounts(int nFiles, int extras[]) // this guarantees that all the diffs are synchronized assert(curDiff.begin[0]+extras[0] == curDiff.begin[1]+extras[1]); - int nline[3]; + assert(nFiles<3 || curDiff.begin[0]+extras[0] == curDiff.begin[2]+extras[2]); + int nline[3] = { 0,0,0 }; int nmaxline = 0; int file; for (file = 0; file < nFiles; file++) diff --git a/Src/DiffList.h b/Src/DiffList.h index 8d529e15c..a8a7d63c2 100644 --- a/Src/DiffList.h +++ b/Src/DiffList.h @@ -166,7 +166,7 @@ public: void ConstructSignificantChain(); // must be called after diff list is entirely populated void Swap(int index1, int index2); - void GetExtraLinesCounts(int nFiles, int extras[]); + void GetExtraLinesCounts(int nFiles, int extras[3]); std::vector& GetDiffRangeInfoVector() { return m_diffs; } diff --git a/Src/DiffWrapper.cpp b/Src/DiffWrapper.cpp index 8c813bfa0..27141ee3f 100644 --- a/Src/DiffWrapper.cpp +++ b/Src/DiffWrapper.cpp @@ -946,7 +946,7 @@ void CDiffWrapper::AddDiffRange(DiffList *pDiffList, DIFFRANGE &dr) * @param [in] leftBufferLines size of array pane left * @param [in] rightBufferLines size of array pane right * @param [in] left on whitch side we have to insert - * @param [in] bIgnoreBlankLines, if true we allways add a new diff and make as trivial + * @param [in] bIgnoreBlankLines, if true we always add a new diff and mark as trivial */ void CDiffWrapper::FixLastDiffRange(int nFiles, int bufferLines[], bool bMissingNL[], bool bIgnoreBlankLines) { diff --git a/Src/GhostTextBuffer.cpp b/Src/GhostTextBuffer.cpp index a14e43365..110744625 100644 --- a/Src/GhostTextBuffer.cpp +++ b/Src/GhostTextBuffer.cpp @@ -699,6 +699,7 @@ void CGhostTextBuffer::RecomputeRealityMapping() // state 1, i-1 not real line passingGhosts: + ASSERT( i <= GetLineCount() ); if (i == GetLineCount()) return; if (GetLineFlags(i) & LF_GHOST) @@ -715,6 +716,7 @@ passingGhosts: // state 2, i - 1 is real line inReality: + ASSERT( i <= GetLineCount() ); if (i == GetLineCount() || (GetLineFlags(i) & LF_GHOST)) { // i-1 is the last line of a reality block diff --git a/Src/MergeDoc.cpp b/Src/MergeDoc.cpp index 9863cbb15..4d7d2baf8 100644 --- a/Src/MergeDoc.cpp +++ b/Src/MergeDoc.cpp @@ -471,13 +471,13 @@ int CMergeDoc::Rescan(bool &bBinary, IDENTLEVEL &identical, // If comparing whitespaces and // other file has EOL before EOF and other not... - if (!diffOptions.nIgnoreWhitespace && !diffOptions.bIgnoreBlankLines) + if (diffOptions.nIgnoreWhitespace == WHITESPACE_COMPARE_ALL || !diffOptions.bIgnoreBlankLines) { if (std::count(status.bMissingNL, status.bMissingNL + m_nBuffers, status.bMissingNL[0]) < m_nBuffers) { - // ..lasf DIFFRANGE of file which has EOL must be + // ..last DIFFRANGE of file which has EOL must be // fixed to contain last line too - int lineCount[3]; + int lineCount[3] = { 0,0,0 }; for (nBuffer = 0; nBuffer < m_nBuffers; nBuffer++) lineCount[nBuffer] = m_ptBuf[nBuffer]->GetLineCount(); m_diffWrapper.FixLastDiffRange(m_nBuffers, lineCount, status.bMissingNL, diffOptions.bIgnoreBlankLines); @@ -1889,12 +1889,7 @@ void CMergeDoc::OnUpdateDiffContext(CCmdUI* pCmdUI) /** * @brief Build the diff array and prepare buffers accordingly (insert ghost lines, set WinMerge flags) * - * @note Buffers may have different length after PrimeTextBuffers. Indeed, no - * synchronization is needed after the last line. So no ghost line will be created - * to face an ignored difference in the last line (typically : 'ignore blank lines' - * + empty last line on one side). - * If you fell that different length buffers are really strange, CHANGE FIRST - * the last diff to take into account the empty last line. + * @note After PrimeTextBuffers(), all buffers should have the same length. */ void CMergeDoc::PrimeTextBuffers() { @@ -1905,21 +1900,24 @@ void CMergeDoc::PrimeTextBuffers() int file; // walk the diff list and calculate numbers of extra lines to add - int extras[3]={0}; // extra lines added to each view + int extras[3] = {0, 0, 0}; // extra lines added to each view m_diffList.GetExtraLinesCounts(m_nBuffers, extras); // resize m_aLines once for each view - UINT lcount[3] = {0}; - UINT lcountnew[3] = {0}; + UINT lcount[3] = {0, 0, 0}; + UINT lcountnew[3] = {0, 0, 0}; + UINT lcountmax = 0; for (file = 0; file < m_nBuffers; file++) { lcount[file] = m_ptBuf[file]->GetLineCount(); lcountnew[file] = lcount[file] + extras[file]; - m_ptBuf[file]->m_aLines.resize(lcountnew[file]); + lcountmax = max(lcountmax, lcountnew[file]); + } + for (file = 0; file < m_nBuffers; file++) + { + m_ptBuf[file]->m_aLines.resize(lcountmax); } -// this ASSERT may be false because of empty last line (see function's note) -// ASSERT(lcount0new == lcount1new); // walk the diff list backward, move existing lines to proper place, // add ghost lines, and set flags @@ -1929,7 +1927,7 @@ void CMergeDoc::PrimeTextBuffers() VERIFY(m_diffList.GetDiff(nDiff, curDiff)); // move matched lines after curDiff - int nline[3] = { 0 }; + int nline[3] = { 0, 0, 0 }; for (file = 0; file < m_nBuffers; file++) nline[file] = lcount[file] - curDiff.end[file] - 1; // #lines on left/middle/right after current diff // Matched lines should really match... @@ -1952,14 +1950,14 @@ void CMergeDoc::PrimeTextBuffers() for (file = 0; file < m_nBuffers; file++) { + DWORD dflag = LF_GHOST; + if ((file == 0 && curDiff.op == OP_3RDONLY) || (file == 2 && curDiff.op == OP_1STONLY)) + dflag |= LF_SNP; m_ptBuf[file]->MoveLine(curDiff.begin[file], curDiff.end[file], lcountnew[file]-nmaxline); int nextra = nmaxline - nline[file]; if (nextra > 0) { m_ptBuf[file]->SetEmptyLine(lcountnew[file] - nextra, nextra); - DWORD dflag = LF_GHOST; - if ((file == 0 && curDiff.op == OP_3RDONLY) || (file == 2 && curDiff.op == OP_1STONLY)) - dflag |= LF_SNP; for (int i = 1; i <= nextra; i++) m_ptBuf[file]->SetLineFlag(lcountnew[file]-i, dflag, true, false, false); } @@ -2009,7 +2007,7 @@ void CMergeDoc::PrimeTextBuffers() if ((file == 0 && curDiff.op == OP_3RDONLY) || (file == 2 && curDiff.op == OP_1STONLY)) dflag |= LF_SNP; m_ptBuf[file]->SetLineFlag(i, dflag, true, false, false); -m_ptBuf[file]->SetLineFlag(i, LF_INVISIBLE, false, false, false); + m_ptBuf[file]->SetLineFlag(i, LF_INVISIBLE, false, false, false); } else { @@ -2028,11 +2026,46 @@ m_ptBuf[file]->SetLineFlag(i, LF_INVISIBLE, false, false, false); m_diffList.ConstructSignificantChain(); - // Used to strip trivial diffs out of the diff chain - // if m_nTrivialDiffs - // via copying them all to a new chain, then copying only non-trivials back - // but now we keep all diffs, including trivial diffs + // Buffers `m_ptBuf[]` may have a final line entry that is . + // (a) If ALL buffers have that final , remove them all. + // (b) If only SOME have that final , change those + // lines into proper ghost-image lines. + // Note too: By this point all buffers must have the same number + // of line entries; eventual buffer processing typically + // uses the line count from `m_ptBuf[0]` for all buffer processing. + int nFinalNullLines = 0; + for (file = 0; file < m_nBuffers; file++) + { + // First, decide how many buffers have a final line. + ASSERT(m_ptBuf[0]->GetLineCount() == m_ptBuf[file]->GetLineCount()); + if (m_ptBuf[file]->GetLineChars(m_ptBuf[file]->GetLineCount()-1) == nullptr) + nFinalNullLines++; + } + if (nFinalNullLines != 0) + { + // Some (maybe all) of the buffers have a final line. + // Handle them (per comment above) in the loop below. + for (file = 0; file < m_nBuffers; file++) + { + UINT LastLineInx = m_ptBuf[file]->GetLineCount() - 1; + if (m_ptBuf[file]->GetLineChars(LastLineInx) == nullptr) + if (nFinalNullLines == m_nBuffers) + { + // ALL buffers have a final line; discard them. + m_ptBuf[file]->m_aLines.resize(m_ptBuf[file]->GetLineCount() - 1); + } + else + { + // Only SOME buffers have final line; set them to a valid GHOST line. + // And copy the LF_SNP flag from the previous line (if it exists). + DWORD dFlags = (LastLineInx > 0 ? m_ptBuf[file]->GetLineFlags(LastLineInx-1) & LF_SNP : 0) | LF_GHOST; + m_ptBuf[file]->SetEmptyLine(LastLineInx, 1); + m_ptBuf[file]->SetLineFlag(LastLineInx, dFlags, true, false, false); + } + ASSERT(m_ptBuf[0]->GetLineCount() == m_ptBuf[file]->GetLineCount()); + } + } for (file = 0; file < m_nBuffers; file++) m_ptBuf[file]->FinishLoading(); diff --git a/Src/MergeLineFlags.h b/Src/MergeLineFlags.h index a30dc842f..bcd0736f0 100644 --- a/Src/MergeLineFlags.h +++ b/Src/MergeLineFlags.h @@ -15,6 +15,7 @@ enum MERGE_LINEFLAGS { LF_DIFF = 0x00200000L, +// LF_GHOST = 0x00400000L, LF_TRIVIAL = 0x00800000L, LF_MOVED = 0x01000000L, LF_SNP = 0x02000000L, @@ -22,7 +23,7 @@ enum MERGE_LINEFLAGS // WINMERGE_FLAGS is MERGE_LINEFLAGS | GHOST_LINEFLAGS | LF_TRIVIAL | LF_MOVED -#define LF_WINMERGE_FLAGS 0x01E00000 +#define LF_WINMERGE_FLAGS 0x03E00000 // Flags for non-ignored difference // Note that we must include ghost flag to include ghost lines -- 2.11.0