Merge pull request #366 from Murmele/RevertChangesHunk

Fix discarding complete content of the file and submodules

CMakeLists.txt

@@ -56,6 +56,7 @@ option(USE_SYSTEM_QT "Don't copy QT files into the installation directory" OFF)
 option(USE_SYSTEM_LUA "Use the system-wide Lua installation" OFF)
 option(USE_SYSTEM_HUNSPELL "Use the system-wide hunspell installation" OFF)
 option(USE_SYSTEM_CMARK "Use the system-wide cmark installation" OFF)
+option(ENABLE_TESTS "Enable Gittyup Unittests" ON)
 
 set(LUA_MODULES_PATH
     CACHE
@@ -128,4 +129,7 @@ add_subdirectory(dep)
 add_subdirectory(src)
 add_subdirectory(l10n)
 add_subdirectory(pack)
-add_subdirectory(test)
+
+if(ENABLE_TESTS)
+  add_subdirectory(test)
+endif()

src/git/Patch.cpp

@@ -367,16 +367,25 @@ void Patch::apply(QList<QList<QByteArray>> &image, int hidx,
                          hidx)) // returns hunk_idx hunk
     return;
 
-  assert(hunk->new_start - 1 + hunk->new_lines <= image.length());
+  int start = hunk->new_start;
+  int numberLines = hunk->new_lines;
+  if (start == 0 && numberLines == 0) {
+    // Complete content of the file was deleted
+    start = hunk->old_start;
+    numberLines = hunk->old_lines;
+  }
+
+  assert(start > 0);
+  assert(start - 1 + numberLines <= image.length());
 
   // delete old data
-  for (int i = hunk->new_start - 1;
-       i < qMin(hunk->new_start - 1 + hunk->new_lines, image.length()); i++) {
+  for (int i = start - 1; i < qMin(start - 1 + numberLines, image.length());
+       i++) {
     image[i].clear();
   }
   // the length of image is not changed, so the function can be applied for
   // multiple hunks
-  image[hunk->new_start - 1].append(
+  image[start - 1].append(
       hunkData); // at least the line for the old_start must be available
 }
 

src/ui/DoubleTreeWidget.cpp

@@ -457,18 +457,17 @@ void DoubleTreeWidget::storeSelection() {
 
 void DoubleTreeWidget::loadSelection() {
   QModelIndex index;
-  git::Index::StagedState state;
+  Qt::CheckState state;
 
   if (mSelectedFile.filename != "") {
     index = mDiffTreeModel->index(mSelectedFile.filename);
-    state = static_cast<git::Index::StagedState>(
+    state = static_cast<Qt::CheckState>(
         mDiffTreeModel->data(index, Qt::CheckStateRole).toInt());
   }
 
   if (!index.isValid() ||
-      (mSelectedFile.stagedModel && state != git::Index::StagedState::Staged) ||
-      (!mSelectedFile.stagedModel &&
-       state != git::Index::StagedState::Unstaged)) {
+      (mSelectedFile.stagedModel && state != Qt::CheckState::Checked) ||
+      (!mSelectedFile.stagedModel && state != Qt::CheckState::Unchecked)) {
     mSelectedFile.filename = "";
     if (mDiffTreeModel->rowCount() > 0) {
       index = mDiffTreeModel->index(0, 0);

src/ui/FileContextMenu.cpp

@@ -189,47 +189,73 @@ FileContextMenu::FileContextMenu(RepoView *view, const QStringList &files,
   QStringList modified;
   QStringList untracked;
   if (diff.isValid()) {
-    foreach (const QString &file, files) {
-      handlePath(repo, file, diff, modified, untracked);
-    }
-
-    QAction *discard = addAction(tr("Discard Changes"), [view, modified] {
-      QMessageBox *dialog =
-          new QMessageBox(QMessageBox::Warning, tr("Discard Changes?"),
-                          tr("Are you sure you want to discard changes in "
-                             "the selected files?"),
-                          QMessageBox::Cancel, view);
-      dialog->setAttribute(Qt::WA_DeleteOnClose);
-      dialog->setInformativeText(tr("This action cannot be undone."));
-      dialog->setDetailedText(modified.join('\n'));
-
-      // Expand the Show Details
-      foreach (QAbstractButton *button, dialog->buttons()) {
-        if (dialog->buttonRole(button) == QMessageBox::ActionRole) {
-          button->click(); // click it to expand the text
+    // copied from DiffTreeModel.cpp
+    // handle submodules
+    auto s = repo.submodules();
+    QList<git::Submodule> submodules;
+    QStringList filePatches;
+    for (auto trackedPatch : files) {
+      bool is_submodule = false;
+      for (auto submodule : s) {
+        if (submodule.path() == trackedPatch) {
+          is_submodule = true;
+          submodules.append(submodule);
           break;
         }
       }
+      if (!is_submodule)
+        filePatches.append(trackedPatch);
+    }
 
-      QString text = tr("Discard Changes");
-      QPushButton *discard = dialog->addButton(text, QMessageBox::AcceptRole);
-      discard->setObjectName("DiscardButton");
-      connect(discard, &QPushButton::clicked, [view, modified] {
-        git::Repository repo = view->repo();
-        int strategy = GIT_CHECKOUT_FORCE;
-        if (!repo.checkout(git::Commit(), nullptr, modified, strategy)) {
-          QString text = tr("%1 files").arg(modified.size());
-          LogEntry *parent = view->addLogEntry(text, tr("Discard"));
-          view->error(parent, tr("discard"), text);
-        }
+    // handle files not submodules
+    foreach (const QString &file, filePatches) {
+      handlePath(repo, file, diff, modified, untracked);
+    }
 
-        // FIXME: Work dir changed?
-        view->refresh();
-      });
+    QAction *discard =
+        addAction(tr("Discard Changes"), [view, modified, submodules] {
+          QMessageBox *dialog =
+              new QMessageBox(QMessageBox::Warning, tr("Discard Changes?"),
+                              tr("Are you sure you want to discard changes in "
+                                 "the selected files?"),
+                              QMessageBox::Cancel, view);
+          dialog->setAttribute(Qt::WA_DeleteOnClose);
+          dialog->setInformativeText(tr("This action cannot be undone."));
+          QString detailedText = modified.join('\n');
+          for (const auto s : submodules)
+            detailedText += s.path() + " " + tr("(Submodule)") + "\n";
+          dialog->setDetailedText(detailedText);
+
+          // Expand the Show Details
+          foreach (QAbstractButton *button, dialog->buttons()) {
+            if (dialog->buttonRole(button) == QMessageBox::ActionRole) {
+              button->click(); // click it to expand the text
+              break;
+            }
+          }
 
-      dialog->open();
-    });
-    discard->setEnabled(!modified.isEmpty());
+          QString text = tr("Discard Changes");
+          QPushButton *discard =
+              dialog->addButton(text, QMessageBox::AcceptRole);
+          discard->setObjectName("DiscardButton");
+          connect(discard, &QPushButton::clicked, [view, modified, submodules] {
+            git::Repository repo = view->repo();
+            int strategy = GIT_CHECKOUT_FORCE;
+            if (modified.count() &&
+                !repo.checkout(git::Commit(), nullptr, modified, strategy)) {
+              QString text = tr("%1 files").arg(modified.size());
+              LogEntry *parent = view->addLogEntry(text, tr("Discard"));
+              view->error(parent, tr("discard"), text);
+            }
+            view->updateSubmodules(submodules, true, false, true);
+
+            // FIXME: Work dir changed?
+            view->refresh(); // TODO: check that refresh is called only once!
+          });
+
+          dialog->open();
+        });
+    discard->setEnabled(!modified.isEmpty() || submodules.count());
 
     QAction *remove = addAction(tr("Remove Untracked Files"),
                                 [view, untracked] { view->clean(untracked); });

test/CMakeLists.txt

@@ -2,7 +2,7 @@
 macro(test)
   cmake_parse_arguments(ARG "NO_WIN32_OFFSCREEN" "NAME" "" ${ARGN})
 
-  add_executable(test_${ARG_NAME} EXCLUDE_FROM_ALL ${ARG_NAME}.cpp)
+  add_executable(test_${ARG_NAME} ${ARG_NAME}.cpp)
   target_link_libraries(test_${ARG_NAME} testlib)
 
   set_target_properties(test_${ARG_NAME} PROPERTIES OUTPUT_NAME ${ARG_NAME}

test/EditorLineInfos.cpp

@@ -130,6 +130,9 @@ private slots:
 
 #endif // EXECUTE_ONLY_LAST_TEST == 0
 
+  void discardCompleteDeletedContent();
+  void discardCompleteAddedContent();
+
   //  void deleteCompleteContent();
 
 private:
@@ -980,6 +983,68 @@ void TestEditorLineInfo::sameContentAddLine() {
 //      QVector<int>({23}));
 //}
 
+void TestEditorLineInfo::discardCompleteDeletedContent() {
+  INIT_REPO("19_discardCompleteDeletedContent.zip", true)
+  QVERIFY(diff.count() > 0);
+  git::Patch patch = diff.patch(0);
+  // no staged lines yet, so no staged patch
+  QCOMPARE(mRepo.diffTreeToIndex(commit.tree()).count(), 0);
+  git::Patch stagedPatch = git::Patch();
+
+  QString name = patch.name();
+  QString path_ = mRepo.workdir().filePath(name);
+  bool submodule = mRepo.lookupSubmodule(name).isValid();
+  {
+    FileWidget fw(&diffView, diff, patch, stagedPatch, QModelIndex(), name,
+                  path_, submodule, repoView);
+    fw.setStageState(git::Index::StagedState::Unstaged);
+
+    auto hunks = fw.hunks();
+    QVERIFY(hunks.count() == 1);
+    for (auto *hunk : hunks)
+      hunk->load();
+
+    QCOMPARE(hunks.at(0)->hunk(), QByteArray());
+
+    hunks.at(0)->discardSelected(0, 1);
+  }
+
+  // Check that discard was successfull and the application does not crash
+  QFile f(path + "/File.txt");
+  QVERIFY(f.open(QIODevice::ReadOnly));
+  QCOMPARE(f.readAll(), "Content\n");
+}
+
+void TestEditorLineInfo::discardCompleteAddedContent() {
+  INIT_REPO("20_discardCompleteAddedContent.zip", true)
+  QVERIFY(diff.count() > 0);
+  git::Patch patch = diff.patch(0);
+  // no staged lines yet, so no staged patch
+  QCOMPARE(mRepo.diffTreeToIndex(commit.tree()).count(), 0);
+  git::Patch stagedPatch = git::Patch();
+
+  QString name = patch.name();
+  QString path_ = mRepo.workdir().filePath(name);
+  bool submodule = mRepo.lookupSubmodule(name).isValid();
+  {
+    FileWidget fw(&diffView, diff, patch, stagedPatch, QModelIndex(), name,
+                  path_, submodule, repoView);
+    fw.setStageState(git::Index::StagedState::Unstaged);
+
+    auto hunks = fw.hunks();
+    QVERIFY(hunks.count() == 1);
+    for (auto *hunk : hunks)
+      hunk->load();
+
+    hunks.at(0)->discardSelected(0, 1);
+  }
+
+  // Check that discard was successfull and the application does not crash
+  QFile f(path + "/Test.txt");
+  QVERIFY(f.open(QIODevice::ReadOnly));
+  QCOMPARE(f.readAll(), "");
+}
+
 void TestEditorLineInfo::cleanupTestCase() { qWait(closeDelay); }
 
 TEST_MAIN(TestEditorLineInfo)

test/TreeView.cpp

@@ -99,7 +99,7 @@ void TestTreeView::discardFiles() {
   INIT_REPO("TestRepository.zip", false);
 
   git::Commit commit =
-      repo.lookupCommit("51198ba9b2b2b2c25ea6576cf7ca3e9f2a7c3fc7");
+      repo.lookupCommit("5c61b24e236310ad4a8a64f7cd1ccc968f1eec20");
   QVERIFY(commit);
 
   // modifying all files
@@ -214,7 +214,7 @@ void TestTreeView::fileMergeCrash() {
   QAbstractItemModel *stagedModel = stagedTree->model();
 
   // Wait for refresh
-  while (stagedModel->rowCount() < 4)
+  while (stagedModel->rowCount() < 3)
     qWait(300);
 
   QAbstractItemModel *unstagedModel = unstagedTree->model();