Factoring out some commons synchronization checklist functions

Source commit: 476fef1cfeea061ec1d35cdb24b2528263eaf247

source/core/RemoteFiles.cpp

@@ -2913,6 +2913,16 @@ __int64 __fastcall TSynchronizeChecklist::TItem::GetBaseSize(TAction AAction) co
   }
 }
 //---------------------------------------------------------------------------
+UnicodeString TSynchronizeChecklist::TItem::GetLocalPath() const
+{
+  return CombinePaths(Local.Directory, Local.FileName);
+}
+//---------------------------------------------------------------------------
+UnicodeString TSynchronizeChecklist::TItem::GetRemotePath() const
+{
+  return UnixCombinePaths(Remote.Directory, Remote.FileName);
+}
+//---------------------------------------------------------------------------
 //---------------------------------------------------------------------------
 TSynchronizeChecklist::TSynchronizeChecklist() :
   FList(new TList())
@@ -2933,11 +2943,8 @@ void TSynchronizeChecklist::Add(TItem * Item)
   FList->Add(Item);
 }
 //---------------------------------------------------------------------------
-int __fastcall TSynchronizeChecklist::Compare(void * AItem1, void * AItem2)
+int TSynchronizeChecklist::Compare(const TItem * Item1, const TItem * Item2)
 {
-  TItem * Item1 = static_cast<TItem *>(AItem1);
-  TItem * Item2 = static_cast<TItem *>(AItem2);
-
   int Result;
   if (!Item1->Local.Directory.IsEmpty())
   {
@@ -2957,6 +2964,11 @@ int __fastcall TSynchronizeChecklist::Compare(void * AItem1, void * AItem2)
   return Result;
 }
 //---------------------------------------------------------------------------
+int __fastcall TSynchronizeChecklist::Compare(void * AItem1, void * AItem2)
+{
+  return Compare(static_cast<TItem *>(AItem1), static_cast<TItem *>(AItem2));
+}
+//---------------------------------------------------------------------------
 void TSynchronizeChecklist::Sort()
 {
   FList->Sort(Compare);
@@ -2980,6 +2992,22 @@ int TSynchronizeChecklist::GetCheckedCount() const
   return Result;
 }
 //---------------------------------------------------------------------------
+bool TSynchronizeChecklist::GetNextChecked(int & Index, const TItem *& AItem) const
+{
+  while (Index < Count)
+  {
+    const TItem * TheItem = Item[Index];
+    Index++;
+    if (TheItem->Checked)
+    {
+      AItem = TheItem;
+      return true;
+    }
+  }
+  AItem = NULL;
+  return false;
+}
+//---------------------------------------------------------------------------
 const TSynchronizeChecklist::TItem * TSynchronizeChecklist::GetItem(int Index) const
 {
   return static_cast<TItem *>(FList->Items[Index]);
@@ -3119,13 +3147,11 @@ __int64 TSynchronizeProgress::GetProcessed(const TFileOperationProgressType * Cu
   {
     FTotalSize = 0;
 
-    for (int Index = 0; Index < FChecklist->Count; Index++)
+    int Index = 0;
+    const TSynchronizeChecklist::TItem * ChecklistItem;
+    while (FChecklist->GetNextChecked(Index, ChecklistItem))
     {
-      const TSynchronizeChecklist::TItem * ChecklistItem = FChecklist->Item[Index];
-      if (ChecklistItem->Checked)
-      {
-        FTotalSize += ItemSize(ChecklistItem);
-      }
+      FTotalSize += ItemSize(ChecklistItem);
     }
   }
 

source/core/RemoteFiles.h

@@ -499,6 +499,9 @@ public:
     __int64 __fastcall GetBaseSize() const;
     __int64 __fastcall GetSize() const;
     __int64 __fastcall GetSize(TAction AAction) const;
+    UnicodeString GetLocalPath() const;
+    // Contrary to RemoteFile->FullFileName, this does not include trailing slash for directories
+    UnicodeString GetRemotePath() const;
 
     ~TItem();
 
@@ -520,11 +523,14 @@ public:
 
   static TAction __fastcall Reverse(TAction Action);
   static bool __fastcall IsItemSizeIrrelevant(TAction Action);
+  bool GetNextChecked(int & Index, const TItem *& Item) const;
 
   __property int Count = { read = GetCount };
   __property int CheckedCount = { read = GetCheckedCount };
   __property const TItem * Item[int Index] = { read = GetItem };
 
+  static int Compare(const TItem * Item1, const TItem * Item2);
+
 protected:
   TSynchronizeChecklist();
 

source/core/Script.cpp

@@ -1899,56 +1899,54 @@ void __fastcall TScript::SynchronizePreview(
   LocalDirectory = IncludeTrailingBackslash(LocalDirectory);
   RemoteDirectory = UnixIncludeTrailingBackslash(RemoteDirectory);
 
-  for (int Index = 0; (Index < Checklist->Count); Index++)
+  int Index = 0;
+  const TSynchronizeChecklist::TItem * Item;
+  while (Checklist->GetNextChecked(Index, Item))
   {
-    const TSynchronizeChecklist::TItem * Item = Checklist->Item[Index];
-    if (Item->Checked)
-    {
-      TDifferenceSessionAction Action(FTerminal->ActionLog, Item);
+    TDifferenceSessionAction Action(FTerminal->ActionLog, Item);
 
-      UnicodeString Message;
-      UnicodeString LocalRecord = SynchronizeFileRecord(LocalDirectory, Item, true);
-      UnicodeString RemoteRecord = SynchronizeFileRecord(RemoteDirectory, Item, false);
+    UnicodeString Message;
+    UnicodeString LocalRecord = SynchronizeFileRecord(LocalDirectory, Item, true);
+    UnicodeString RemoteRecord = SynchronizeFileRecord(RemoteDirectory, Item, false);
 
-      switch (Item->Action)
-      {
-        case TSynchronizeChecklist::saUploadNew:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_UPLOAD_NEW, (LocalRecord));
-          break;
+    switch (Item->Action)
+    {
+      case TSynchronizeChecklist::saUploadNew:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_UPLOAD_NEW, (LocalRecord));
+        break;
 
-        case TSynchronizeChecklist::saDownloadNew:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_DOWNLOAD_NEW, (RemoteRecord));
-          break;
+      case TSynchronizeChecklist::saDownloadNew:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_DOWNLOAD_NEW, (RemoteRecord));
+        break;
 
-        case TSynchronizeChecklist::saUploadUpdate:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_UPLOAD_UPDATE,
-              (LocalRecord, RemoteRecord));
-          break;
+      case TSynchronizeChecklist::saUploadUpdate:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_UPLOAD_UPDATE,
+            (LocalRecord, RemoteRecord));
+        break;
 
-        case TSynchronizeChecklist::saDownloadUpdate:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_DOWNLOAD_UPDATE,
-              (RemoteRecord, LocalRecord));
-          break;
+      case TSynchronizeChecklist::saDownloadUpdate:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_DOWNLOAD_UPDATE,
+            (RemoteRecord, LocalRecord));
+        break;
 
-        case TSynchronizeChecklist::saDeleteRemote:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_DELETE_REMOTE, (RemoteRecord));
-          break;
+      case TSynchronizeChecklist::saDeleteRemote:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_DELETE_REMOTE, (RemoteRecord));
+        break;
 
-        case TSynchronizeChecklist::saDeleteLocal:
-          Message =
-            FMTLOAD(SCRIPT_SYNC_DELETE_LOCAL, (LocalRecord));
-          break;
+      case TSynchronizeChecklist::saDeleteLocal:
+        Message =
+          FMTLOAD(SCRIPT_SYNC_DELETE_LOCAL, (LocalRecord));
+        break;
 
-      default:
-        DebugFail();
-      }
-      PrintLine(Message);
+    default:
+      DebugFail();
     }
+    PrintLine(Message);
   }
 }
 //---------------------------------------------------------------------------
@@ -2089,13 +2087,9 @@ void __fastcall TScript::SynchronizeProc(TScriptProcParams * Parameters)
           &CopyParam, SynchronizeParams, OnTerminalSynchronizeDirectory, NULL);
       try
       {
-        bool AnyChecked = false;
-        for (int Index = 0; !AnyChecked && (Index < Checklist->Count); Index++)
-        {
-          AnyChecked = Checklist->Item[Index]->Checked;
-        }
-
-        if (AnyChecked)
+        int Index = 0;
+        const TSynchronizeChecklist::TItem * DummyItem;
+        if (Checklist->GetNextChecked(Index, DummyItem))
         {
           if (Preview)
           {

source/core/SessionInfo.cpp

@@ -364,13 +364,11 @@ public:
 
     if (RecordLocal)
     {
-      UnicodeString FileName = CombinePaths(Item->Local.Directory, Item->Local.FileName);
-      SynchronizeChecklistItemFileInfo(FileName, Item->IsDirectory, Item->Local);
+      SynchronizeChecklistItemFileInfo(Item->GetLocalPath(), Item->IsDirectory, Item->Local);
     }
     if (RecordRemote)
     {
-      UnicodeString FileName = UnixCombinePaths(Item->Remote.Directory, Item->Remote.FileName);
-      SynchronizeChecklistItemFileInfo(FileName, Item->IsDirectory, Item->Remote);
+      SynchronizeChecklistItemFileInfo(Item->GetRemotePath(), Item->IsDirectory, Item->Remote);
     }
   }
 

source/core/Terminal.cpp

@@ -6586,12 +6586,13 @@ void __fastcall TTerminal::SynchronizeApply(
     SyncCopyParam.CalculateSize = false;
 
     TSynchronizeChecklist::TItemList Items;
-    for (int Index = 0; Index < Checklist->Count; Index++)
+    int Index = 0;
+    const TSynchronizeChecklist::TItem * ChecklistItem;
+    while (Checklist->GetNextChecked(Index, ChecklistItem))
     {
-      const TSynchronizeChecklist::TItem * ChecklistItem = Checklist->Item[Index];
       // TSynchronizeChecklistDialog relies on us not to update a size of an item that had size already
       // See TSynchronizeChecklistDialog::UpdatedSynchronizationChecklistItems
-      if (ChecklistItem->Checked && !TSynchronizeChecklist::IsItemSizeIrrelevant(ChecklistItem->Action) &&
+      if (!TSynchronizeChecklist::IsItemSizeIrrelevant(ChecklistItem->Action) &&
           !ChecklistItem->HasSize() && DebugAlwaysTrue(ChecklistItem->IsDirectory))
       {
         Items.push_back(ChecklistItem);
@@ -6615,99 +6616,94 @@ void __fastcall TTerminal::SynchronizeApply(
   try
   {
     int Index = 0;
-    while (Index < Checklist->Count)
+    const TSynchronizeChecklist::TItem * ChecklistItem;
+    while (Checklist->GetNextChecked(Index, ChecklistItem))
     {
-      const TSynchronizeChecklist::TItem * ChecklistItem = Checklist->Item[Index];
-      if (ChecklistItem->Checked)
+      if (!SamePaths(Data.LocalDirectory, ChecklistItem->Local.Directory) ||
+          !UnixSamePath(Data.RemoteDirectory, ChecklistItem->Remote.Directory))
       {
-        if (!SamePaths(Data.LocalDirectory, ChecklistItem->Local.Directory) ||
-            !UnixSamePath(Data.RemoteDirectory, ChecklistItem->Remote.Directory))
-        {
-          Data.LocalDirectory = IncludeTrailingBackslash(ChecklistItem->Local.Directory);
-          Data.RemoteDirectory = UnixIncludeTrailingBackslash(ChecklistItem->Remote.Directory);
+        Data.LocalDirectory = IncludeTrailingBackslash(ChecklistItem->Local.Directory);
+        Data.RemoteDirectory = UnixIncludeTrailingBackslash(ChecklistItem->Remote.Directory);
 
-          LogEvent(
-            FORMAT(L"Synchronizing local directory '%s' with remote directory '%s', params = 0x%x (%s)",
-            (Data.LocalDirectory, Data.RemoteDirectory, int(Params), SynchronizeParamsStr(Params))));
+        LogEvent(
+          FORMAT(L"Synchronizing local directory '%s' with remote directory '%s', params = 0x%x (%s)",
+          (Data.LocalDirectory, Data.RemoteDirectory, int(Params), SynchronizeParamsStr(Params))));
 
-          DoSynchronizeProgress(Data, false);
-        }
+        DoSynchronizeProgress(Data, false);
+      }
 
-        std::unique_ptr<TStringList> FileList(new TStringList());
+      std::unique_ptr<TStringList> FileList(new TStringList());
 
-        UnicodeString LocalPath = IncludeTrailingBackslash(ChecklistItem->Local.Directory) + ChecklistItem->Local.FileName;
-        UnicodeString RemotePath = UnixIncludeTrailingBackslash(ChecklistItem->Remote.Directory) + ChecklistItem->Remote.FileName;
-        bool Result = true;
+      UnicodeString LocalPath = ChecklistItem->GetLocalPath();
+      UnicodeString RemotePath = ChecklistItem->GetRemotePath();
+      bool Result = true;
 
-        if (FLAGSET(Params, spTimestamp))
+      if (FLAGSET(Params, spTimestamp))
+      {
+        // used by SynchronizeLocalTimestamp and SynchronizeRemoteTimestamp
+        TObject * ChecklistItemToken = const_cast<TObject *>(reinterpret_cast<const TObject *>(ChecklistItem));
+        switch (ChecklistItem->Action)
         {
-          // used by SynchronizeLocalTimestamp and SynchronizeRemoteTimestamp
-          TObject * ChecklistItemToken = const_cast<TObject *>(reinterpret_cast<const TObject *>(ChecklistItem));
-          switch (ChecklistItem->Action)
-          {
-            case TSynchronizeChecklist::saDownloadUpdate:
-              FileList->AddObject(RemotePath, ChecklistItemToken);
-              ProcessFiles(FileList.get(), foSetProperties, SynchronizeLocalTimestamp, NULL, osLocal);
-              break;
-
-            case TSynchronizeChecklist::saUploadUpdate:
-              FileList->AddObject(LocalPath, ChecklistItemToken);
-              ProcessFiles(FileList.get(), foSetProperties, SynchronizeRemoteTimestamp);
-              break;
-
-            default:
-              DebugFail();
-              Result = false;
-              break;
-          }
+          case TSynchronizeChecklist::saDownloadUpdate:
+            FileList->AddObject(RemotePath, ChecklistItemToken);
+            ProcessFiles(FileList.get(), foSetProperties, SynchronizeLocalTimestamp, NULL, osLocal);
+            break;
+
+          case TSynchronizeChecklist::saUploadUpdate:
+            FileList->AddObject(LocalPath, ChecklistItemToken);
+            ProcessFiles(FileList.get(), foSetProperties, SynchronizeRemoteTimestamp);
+            break;
+
+          default:
+            DebugFail();
+            Result = false;
+            break;
         }
-        else
+      }
+      else
+      {
+        TCopyParamType ItemCopyParam = SyncCopyParam;
+        ItemCopyParam.Size = ChecklistItem->HasSize() ? ChecklistItem->GetSize() : -1;
+        switch (ChecklistItem->Action)
         {
-          TCopyParamType ItemCopyParam = SyncCopyParam;
-          ItemCopyParam.Size = ChecklistItem->HasSize() ? ChecklistItem->GetSize() : -1;
-          switch (ChecklistItem->Action)
-          {
-            case TSynchronizeChecklist::saDownloadNew:
-            case TSynchronizeChecklist::saDownloadUpdate:
-              FileList->AddObject(RemotePath, ChecklistItem->RemoteFile);
-              Result = CopyToLocal(FileList.get(), Data.LocalDirectory, &ItemCopyParam, CopyParams, NULL);
-              break;
-
-            case TSynchronizeChecklist::saDeleteRemote:
-              FileList->AddObject(RemotePath, ChecklistItem->RemoteFile);
-              Result = DeleteFiles(FileList.get());
-              break;
-
-            case TSynchronizeChecklist::saUploadNew:
-            case TSynchronizeChecklist::saUploadUpdate:
-              FileList->Add(LocalPath);
-              Result = CopyToRemote(FileList.get(), Data.RemoteDirectory, &ItemCopyParam, CopyParams, NULL);
-              break;
-
-            case TSynchronizeChecklist::saDeleteLocal:
-              FileList->Add(LocalPath);
-              Result = DeleteLocalFiles(FileList.get());
-              break;
-
-            default:
-              DebugFail();
-              Result = false;
-              break;
-          }
-        }
+          case TSynchronizeChecklist::saDownloadNew:
+          case TSynchronizeChecklist::saDownloadUpdate:
+            FileList->AddObject(RemotePath, ChecklistItem->RemoteFile);
+            Result = CopyToLocal(FileList.get(), Data.LocalDirectory, &ItemCopyParam, CopyParams, NULL);
+            break;
 
-        if (!Result)
-        {
-          Abort();
-        }
+          case TSynchronizeChecklist::saDeleteRemote:
+            FileList->AddObject(RemotePath, ChecklistItem->RemoteFile);
+            Result = DeleteFiles(FileList.get());
+            break;
 
-        if (OnProcessedItem != NULL)
-        {
-          OnProcessedItem(Token, ChecklistItem);
+          case TSynchronizeChecklist::saUploadNew:
+          case TSynchronizeChecklist::saUploadUpdate:
+            FileList->Add(LocalPath);
+            Result = CopyToRemote(FileList.get(), Data.RemoteDirectory, &ItemCopyParam, CopyParams, NULL);
+            break;
+
+          case TSynchronizeChecklist::saDeleteLocal:
+            FileList->Add(LocalPath);
+            Result = DeleteLocalFiles(FileList.get());
+            break;
+
+          default:
+            DebugFail();
+            Result = false;
+            break;
         }
       }
 
-      Index++;
+      if (!Result)
+      {
+        Abort();
+      }
+
+      if (OnProcessedItem != NULL)
+      {
+        OnProcessedItem(Token, ChecklistItem);
+      }
     }
   }
   __finally
@@ -6738,11 +6734,12 @@ void __fastcall TTerminal::SynchronizeChecklistCalculateSize(
     {
       if (ChecklistItem->IsRemoteOnly())
       {
-        RemoteFileList->AddObject(ChecklistItem->RemoteFile->FullFileName, ChecklistItem->RemoteFile);
+        DebugAssert(UnixSamePath(ChecklistItem->RemoteFile->FullFileName, ChecklistItem->GetRemotePath()));
+        RemoteFileList->AddObject(ChecklistItem->GetRemotePath(), ChecklistItem->RemoteFile);
       }
       else if (ChecklistItem->IsLocalOnly())
       {
-        LocalFileList->Add(IncludeTrailingBackslash(ChecklistItem->Local.Directory) + ChecklistItem->Local.FileName);
+        LocalFileList->Add(ChecklistItem->GetLocalPath());
       }
       else
       {
@@ -6841,9 +6838,7 @@ void __fastcall TTerminal::SynchronizeLocalTimestamp(const UnicodeString /*FileN
   const TSynchronizeChecklist::TItem * ChecklistItem =
     reinterpret_cast<const TSynchronizeChecklist::TItem *>(File);
 
-  UnicodeString LocalFile =
-    IncludeTrailingBackslash(ChecklistItem->Local.Directory) +
-      ChecklistItem->Local.FileName;
+  UnicodeString LocalFile = ChecklistItem->GetLocalPath();
 
   FILE_OPERATION_LOOP_BEGIN
   {
@@ -6874,9 +6869,7 @@ void __fastcall TTerminal::SynchronizeRemoteTimestamp(const UnicodeString /*File
   Properties.Modification = ConvertTimestampToUnix(ChecklistItem->FLocalLastWriteTime,
     SessionData->DSTMode);
 
-  ChangeFileProperties(
-    UnixIncludeTrailingBackslash(ChecklistItem->Remote.Directory) + ChecklistItem->Remote.FileName,
-    NULL, &Properties);
+  ChangeFileProperties(ChecklistItem->GetRemotePath(), NULL, &Properties);
 }
 //---------------------------------------------------------------------------
 void __fastcall TTerminal::FileFind(UnicodeString FileName,

source/forms/SynchronizeChecklist.cpp

@@ -203,12 +203,12 @@ struct TMoveActionData
         TObject * Object;
         if (Action == TSynchronizeChecklist::saDeleteRemote)
         {
-          FileName = UnixCombinePaths(ChecklistItem->Remote.Directory, ChecklistItem->Remote.FileName);
+          FileName = ChecklistItem->GetRemotePath();
           Object = ChecklistItem->RemoteFile;
         }
         else if (Action == TSynchronizeChecklist::saDeleteLocal)
         {
-          FileName = CombinePaths(ChecklistItem->Local.Directory, ChecklistItem->Local.FileName);
+          FileName = ChecklistItem->GetLocalPath();
         }
 
         bool CollectFile = !FileName.IsEmpty();
@@ -1087,20 +1087,7 @@ void __fastcall TSynchronizeChecklistDialog::ListViewCompare(
 
   if (Compare == 0)
   {
-    if (!ChecklistItem1->Local.Directory.IsEmpty())
-    {
-      Compare = AnsiCompareText(ChecklistItem1->Local.Directory, ChecklistItem2->Local.Directory);
-    }
-    else
-    {
-      DebugAssert(!ChecklistItem1->Remote.Directory.IsEmpty());
-      Compare = AnsiCompareText(ChecklistItem1->Remote.Directory, ChecklistItem2->Remote.Directory);
-    }
-
-    if (Compare == 0)
-    {
-      Compare = AnsiCompareText(ChecklistItem1->GetFileName(), ChecklistItem2->GetFileName());
-    }
+    Compare = TSynchronizeChecklist::Compare(ChecklistItem1, ChecklistItem2);
   }
 
   if (!ColProperties->SortAscending)
@@ -1147,16 +1134,10 @@ void __fastcall TSynchronizeChecklistDialog::CustomCommandsActionExecute(
              (GetChecklistItemAction(ChecklistItem) == TSynchronizeChecklist::saDownloadUpdate));
       DebugAssert(ChecklistItem->RemoteFile != NULL);
 
-      UnicodeString LocalPath =
-        IncludeTrailingBackslash(ChecklistItem->Local.Directory) +
-        ChecklistItem->Local.FileName;
-
+      UnicodeString LocalPath = ChecklistItem->GetLocalPath();
       LocalFileList->Add(LocalPath);
 
-      UnicodeString RemotePath =
-        UnixIncludeTrailingBackslash(ChecklistItem->Remote.Directory) +
-        ChecklistItem->Remote.FileName;
-
+      UnicodeString RemotePath = ChecklistItem->GetRemotePath();
       RemoteFileList->AddObject(RemotePath, ChecklistItem->RemoteFile);
     }
   }