#1852 When conflictfile saved trailing line-ending is lost

Branch_+_Trunk
closed-fixed
5
2009-04-19
2009-01-30
No

WinMerge 2.10.4.0 Unicode

I've ran WinMerge from command line to resolve conflict. I've used the command:

winmergeu howto.txt

File howto.txt had following conflict markers:

<<<<<<< TREE
:Date: 2009/01/15
=======
:Date: 2009/01/29
>>>>>>> MERGE-SOURCE

There was trailing line-ending character (LF) in the end of file.

Unfortunately, when I've saved the file the trailing line-ending character is lost. I've tested with another file but I've encountered the same (wrong) behavior.

I've attached configuration log and howto.txt file as example.

Discussion

  • Alexander Belchenko

    file with conflict

     
  • Alexander Belchenko

    File Added: howto.txt

     
  • Alexander Belchenko

    The same behaviour with latest experimental version 2.13.2-svn

     
  • Kimmo Varis

    Kimmo Varis - 2009-02-03

    Indeed. I can reproduce this bug with your test file. I'll look at this later today.

     
  • Kimmo Varis

    Kimmo Varis - 2009-02-03
    • assigned_to: nobody --> kimmov
     
  • Kimmo Varis

    Kimmo Varis - 2009-02-23

    Reduced test case

     
  • Kimmo Varis

    Kimmo Varis - 2009-02-23

    Sorry, I got sidetracked to other things and forgot this.. Start looking at this right now.

    Also attaching a reduced test case, removing most of unnecessary lines from the file. (howto_reduced.txt)
    File Added: howto_reduced.txt

     
  • Kimmo Varis

    Kimmo Varis - 2009-02-24

    I've been looking at this and seems the bug is in our general file loading class. It does not affect to two-file cases (normal compare/merge) since there we apply some tricks for last EOL chars to synchronize files. But it shows up with single file (conflict file) case. That code is generally used elsewhere makes fixing this a bit tricky.

    Then, this probably explains some weird behaviors seen in couple of other reports...

     
  • Takashi Sawanaka

    The current conflict parser writes a line and the line's EOL chars to temporary files in the following order:

    1. EOL chars
    2. line

    This should be written in the following order:

    1. line
    2. EOL chars

    The patch below fixes this bug.

    Index: ConflictFileParser.cpp

    --- ConflictFileParser.cpp (revision 6634)
    +++ ConflictFileParser.cpp (working copy)
    @@ -98,8 +98,6 @@
    int state;
    int iNestingLevel = 0;
    bool bResult = false;
    - bool bFirstLineNR = true;
    - bool bFirstLineWC = true;
    String revision = _T("none");
    bNestedConflicts = false;

    @@ -132,17 +130,11 @@
    else
    {
    // we're in the common section, so write to both files
    - if (!bFirstLineNR)
    - newRevision.WriteString(eol);
    - else
    - bFirstLineNR = false;
    newRevision.WriteString(line.c_str());
    + newRevision.WriteString(eol);

    - if (!bFirstLineWC)
    - workingCopy.WriteString(eol);
    - else
    - bFirstLineWC = false;
    workingCopy.WriteString(line.c_str());
    + workingCopy.WriteString(eol);
    }
    break;

    @@ -154,11 +146,8 @@
    {
    // nested conflict section starts
    state = 3;
    - if(!bFirstLineWC)
    - workingCopy.WriteString(eol);
    - else
    - bFirstLineWC = false;
    workingCopy.WriteString(line.c_str());
    + workingCopy.WriteString(eol);
    }
    else
    {
    @@ -168,11 +157,8 @@
    line = line.substr(0, pos);
    if (!line.empty())
    {
    - if (!bFirstLineWC)
    - workingCopy.WriteString(eol);
    - else
    - bFirstLineWC = false;
    workingCopy.WriteString(line.c_str());
    + workingCopy.WriteString(eol);
    }

                    //  new revision section
    

    @@ -180,11 +166,8 @@
    }
    else
    {
    - if (!bFirstLineWC)
    - workingCopy.WriteString(eol);
    - else
    - bFirstLineWC = false;
    workingCopy.WriteString(line.c_str());
    + workingCopy.WriteString(eol);
    }
    }
    break;
    @@ -197,11 +180,8 @@
    {
    // nested conflict section starts
    state = 4;
    - if (!bFirstLineNR)
    - newRevision.WriteString(eol);
    - else
    - bFirstLineNR = false;
    newRevision.WriteString(line.c_str());
    + newRevision.WriteString(eol);
    }
    else
    {
    @@ -212,11 +192,8 @@
    line = line.substr(0, pos);
    if (!line.empty())
    {
    - if (!bFirstLineNR)
    - newRevision.WriteString(eol);
    - else
    - bFirstLineNR = false;
    newRevision.WriteString(line.c_str());
    + newRevision.WriteString(eol);
    }

                    //  common section
    

    @@ -224,11 +201,8 @@
    }
    else
    {
    - if (!bFirstLineNR)
    - newRevision.WriteString(eol);
    - else
    - bFirstLineNR = false;
    newRevision.WriteString(line.c_str());
    + newRevision.WriteString(eol);
    }
    }
    break;
    @@ -258,11 +232,8 @@
    }
    }
    }
    - if (!bFirstLineWC)
    - workingCopy.WriteString(eol);
    - else
    - bFirstLineWC = false;
    workingCopy.WriteString(line.c_str());
    + workingCopy.WriteString(eol);
    break;

            // in nested section in new revision section
    

    @@ -288,11 +259,8 @@
    }
    }
    }
    - if (!bFirstLineNR)
    - newRevision.WriteString(eol);
    - else
    - bFirstLineNR = false;
    newRevision.WriteString(line.c_str());
    + newRevision.WriteString(eol);
    break;
    }
    } while (linesToRead);

     
  • Kimmo Varis

    Kimmo Varis - 2009-04-03
    • labels: --> File handling
    • assigned_to: kimmov --> sdottaka
     
  • Kimmo Varis

    Kimmo Varis - 2009-04-03

    Oh, good catch. I "only" converted this parsing code from TortoiseCVS and didn't thoubght about that at all.

    Still I'm wondering if Uni*File classes have this problem with last EOL chars and we are only working around the problem in several places already.

    But anyway, the patch looks good, go ahead and apply it to trunk.

     
  • Takashi Sawanaka

    > Still I'm wondering if Uni*File classes have this problem with last EOL
    > chars and we are only working around the problem in several places
    > already.

    ?

    At least this bug #2550412 is fixed by the patch.

    Committed to SVN trunk. Completed: At revision: 6638

     
  • Alexander Belchenko

    Thank you.

    IIUC the fix will be available in the next Experimental build (svn), right?

     
  • Kimmo Varis

    Kimmo Varis - 2009-04-04

    > > Still I'm wondering if Uni*File classes have this problem with last EOL
    > > chars and we are only working around the problem in several places
    > > already.
    >
    > ?
    >
    > At least this bug #2550412 is fixed by the patch.

    When I debugged about this some time ago it looked like UniMemFile did not return last EOL byte of the file in ReadString(). Something that I need to look at later.

    > IIUC the fix will be available in the next Experimental build (svn),
    > right?

    Yes. I'll release next experimental next week. I also need to think about putting this into 2.12 branch (and to 2.12.4 stable).

     
  • Kimmo Varis

    Kimmo Varis - 2009-04-19

    Takashi, can you commit this to 2.12 branch?

     
  • Takashi Sawanaka

    • milestone: --> Branch_+_Trunk
    • status: open --> closed-fixed
     
  • Takashi Sawanaka

    Committed to 2.12 branch. Completed: At revision: 6675

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks