#2868 switch to new CompareBinaryFiles

open
nobody
GUI (476)
5
2012-12-21
2009-06-11
Matthias
No

this patch just provieds the way to switch to new CompareBinaryFiles()

Discussion

  • Matthias

    Matthias - 2009-06-11

    new switch to binarycompare

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-11

    You really make it impossible for me to review these patches... There are clearly some undocumented dependencies for these patches, but there is no any description of that. Patches are overlapping in functionality (one adds calls to function other calls).

     
  • Matthias

    Matthias - 2009-06-12

    sorry I forgot to tell you need patch
    'This is a new binarycompare as discussed in forum. ID: 2805046'

    But if you like it more I can combine this one and ID: 2805046' to one.

     
  • Matthias

    Matthias - 2009-06-24

    patch #2805036 GuessCodepageEncoding miss binary and UTF8
    must be installed also

    After detecting encoding (GuessCodepageEncoding) this patch will switch to new
    BinaryCompare #2805046 new CompareBinaryFiles

     
  • Matthias

    Matthias - 2009-06-24

    new switch for BinaryCompare

     
  • Matthias

    Matthias - 2009-06-24

    modified FilelLocation

     
  • Matthias

    Matthias - 2009-06-24

    removed DIFF.H
    modified Filelocation to hold the needed infos for compare.

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-24

    + int desc; /* File descriptor */

    I don' t think we want to cache this. This makes it a bit hard to follow who opens/closes files now. I think it must be simple - who uses the file opens and closes it. We don't want to keep files open longer than strictly necessary. There might be (and many times are) other applications wanting to open the files. And blocking their access is a bad thing.

    + if (nCompMethod == CMP_CONTENT &&
    + (m_diffFileData.m_FileLocation[0].encoding.m_binary ||
    + m_diffFileData.m_FileLocation[1].encoding.m_binary))
    + {
    + nCompMethod = CMP_BINARY;
    + }

    You only want to switch from full content compare? Why not from quick contents?

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-24

    I'm also wondering if we should expand DirItem / DiffFileInfo. Those already have most of the file info we will need. So why duplicate it to another struct.

     
  • Matthias

    Matthias - 2009-06-24

    >You only want to switch from full content compare? Why not from quick contents?
    will be done in next patch.

    >We don't want to keep files open longer than strictly necessary.
    All files are closed after comparing (m_diffFileData.Reset()).

    >I'm also wondering if we should expand DirItem / DiffFileInfo
    Hm, but that's now on you to take the decision.
    See DiffFileInfo has much more infos as needed to handle files.
    Should be better something subclass from FileLocation.
    Also the complete filehandling (open, close, convert)should be done there.
    Let be Filelocation part of file_data.

    I just wanted to show how we can do, so same can be done for quickcontent also.

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-25

    > All files are closed after comparing (m_diffFileData.Reset()).

    Yep. The DiffFileData is another bad structure from history. When we are re-thinking these classes we need to re-think DiffFileData too.

    I need to think about these file structures. Collecting thread already reads lots of the file data so I think we should use that data we already have read. Using FileLocation we forget important data for some compare engines.

     
  • Matthias

    Matthias - 2009-06-25

    >thread already reads lots of the file data so I think we should use that data we already
    >have read.
    That's what I want. Acutally we have the infos only in two diffent items.
    FileLocation and file_data. So transfering the filehandling from file_data to FileLocation
    or a inherit of that class would be fine. File_data used in FullContent can be used as now, temporary. So we can split the patches in small parts.
    FileLocation must hold only datainfos about files and handling. To hold the content of lines inside this class is not good. Here the engines should take care in there own class.

     
  • Matthias

    Matthias - 2009-06-26

    reduced to min infos needed

     
  • Matthias

    Matthias - 2009-06-26

    how about this solution.
    only passing the File.desc on an array for BinaryCompare.
    That can be also enough for QickContent.

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-26

    The reason the file_data (and most of FolderCmp class) exists is to run translations (file content, not GUI) and plugins. Removing plugins means we can probably simplify that translation/encoding code quite a bit. And probably don't need that file_data at all. And don't need it for keeping file handles open.

    I think we need to give full paths to files for compare engines. That way all engines have same kind of API. And it may even make sense to move translation/encoding code to inside compare engine.

    But yes, this needs to be done in small steps. I'm not sure what would be the good first step.

     
  • Matthias

    Matthias - 2009-06-26

    No, as we first have to know whitch kind of file we have, binary or text we need detection.
    If we give only the path+filename, we have to reopen the file again. If that goes fast enough?
    >Removing plugins
    No, not now. If you have a replacement, ok.

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-26

    You think only couple of compare engines, not all of them. There content compare engines are not only compare engines. Engines comparing just file attributes don't need to ever open files. Currently we have to determine in FolderCmp (or elsewhere) if file should be opened and whatever preliminary work needs to be done before doing the compare.

    It would be a lot easier to allow compare engine to decide what it needs to do. If and when it wants to open files. If it needs to read encoding or some other info.

    If we handle all that inside compare engine we only need to open file just once like now. We can for example structure compare engines so that there is kind of "master" engine for different major types (content and attributes currently) and that master then uses slave engines (current compare engines).

     
  • Matthias

    Matthias - 2009-06-26

    So if I understand correct.
    One class with engines (Full-Quick, Binary etc..)
    One class for atributes (Time, size, etc..)
    So what is the advance, we move are problems from foldercompare one level down only.
    We still need plugin and filetransform. Still we need m_diffFileData to get the results for GUI. The class FolderCmp should be able todo that in a good way.
    I have to think more in detail to get a real better structure.

     
  • Kimmo Varis

    Kimmo Varis - 2009-06-27

    You limit your thinking too much by current code. The point is to find good solutions and then modify current code. Even rewrite all the current code if needed. In proper steps of course.

    > We still need plugin and filetransform.
    Not necessarily in current form.

    Still we need m_diffFileData to get the results for GUI
    > definitely not. GUI needs list if Diffitems or Diffranges. How we create and update those lists in compare code is totally invisible for the GUI.

    > The class FolderCmp should be able todo that in a
    > good way.
    I really doubt it. I think FolderCmp's role should be to setup compare engines and compare options. We must be able to encapsulate compare code to compare engines. Then we can optimize inside engines how each engine works best. Not letting outside code (in FolderCmp and other classes) to limit compare code.

     

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

Sign up for the SourceForge newsletter:





No, thanks