#2400 VS2008 and VS2005 problems

Trunk
closed-accepted
nobody
None
5
2007-12-10
2007-12-09
No

Winmerge Beta 2.7.6

The compilation with VS2005 and VS2008 run well, but it is not possible to run the pgm due to some range error checking.

This patch give the modification needed to overcome this situation.

The Patch was build with the WinMerge 'Generate Patch' utility, and concern 3 files:

Src/MainFrm.cpp
Src/Splash.cpp
Src/StdAfx.h

As it is the first time I submit such patch for open software, I hope all is ok, otherwise let me know throug my account e-mail

Regards
RobiOneKenobi

Discussion

  • Robert Grange

    Robert Grange - 2007-12-09

    File created with WinMerge 'Generate Patch' utility

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-09
    • labels: 631061 -->
    • milestone: --> Trunk
     
  • Kimmo Varis

    Kimmo Varis - 2007-12-09

    Logged In: YES
    user_id=631874
    Originator: NO

    Removing private flag and changing category and group.

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-09

    Same patch against current trunk in TortoiseSVN format

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-09

    Logged In: YES
    user_id=631874
    Originator: NO

    Looking at the patch now.. Couple of comments:
    - MainFrm.cpp change: wouldn't be better just always do this VS2005/VS2008 required check as I don't see it doing any harm for other VS versions? Differencing code for compiler versions should be avoided as much as possible. It just causes more bugs as most people cannot test with all compiler versions.
    - same comment for Splash.cpp change
    - stdafx.h change: I'd rather not define with > comparison for compiler versions, as we cannot know what future versions do... So if you want to check for VS2008 you should check MSC_VER==1500.

    I'll also attach your patch as a TortoiseSVN patch against current SVN trunk.

    File Added: VS2005and2008fix.patch

     
  • Robert Grange

    Robert Grange - 2007-12-09

    Logged In: YES
    user_id=1955814
    Originator: YES

    For me it's OK, as I tested what you said (No more check for _MSC_VER in MainFrm.cpp and Splash.cpp, and I also tested the if _MSC_VER == 1500 in stdafx.h

    Need I to send you the new patch, or do you make the change your-self (what I prefer, because it's directly set in your repository

    Regards
    RobiOneKenobi

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-09

    Logged In: YES
    user_id=631874
    Originator: NO

    I'd prefer you to attach a new patch to this item, as you already made the changes and tested them, right? I didn't really test my suggestions, so I'd have to do the changes first. Not a big deal, but..

     
  • Robert Grange

    Robert Grange - 2007-12-09

    New Patch (WinMerge 'Generate Patch' style unified)

     
  • Robert Grange

    Robert Grange - 2007-12-09

    Logged In: YES
    user_id=1955814
    Originator: YES

    Here is the new patch, which contain the simplified modification, + one brand new concerning the same problem but for the Common/LanguageSelect.cpp.
    I've also corrected this, and put it in the new patch
    File Added: WinMerge-2.7.6-Patch_2.txt

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-10

    Logged In: YES
    user_id=631874
    Originator: NO

    Thanks a lot!

    I committed your patch with minor changes for coding style in couple of places. And I removed those "changed by " -comment lines. We can see from version control which line was modified by who so those lines are just noise in the code. I also added your name into contributors list.

    Committed to SVN trunk (development version)
    Completed: At revision: 4823

    Will be a part of next 2.7.x releases.

     
  • Kimmo Varis

    Kimmo Varis - 2007-12-10
    • status: open --> closed-accepted
     
  • Robert Grange

    Robert Grange - 2007-12-11

    Logged In: YES
    user_id=1955814
    Originator: YES

    Thanks for all.
    Next time i will not add such comment, but only the changes

     

Log in to post a comment.

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

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks