Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't suggest deleting slid-out branch when it has children #1967

Merged
merged 1 commit into from
Jan 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGE-NOTES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## v5.1.2
- Fixed: don't suggest deleting slid-out branch when it has children (to make work easier for the fork point algorithm)

## v5.1.1
- Fixed: compatibility issues with latest 2024.3 EAPs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,31 @@ public void run() {

@ContinuesInBackground
public void run(@UI Runnable doInUIThreadWhenReady) {
val slideOutBranchIsCurrent = branchToSlideOutName.equals(gitRepository.getCurrentBranchName());
if (slideOutBranchIsCurrent) {
LOG.debug("Skipping (optional) local branch deletion because it is equal to current branch");
val branchToSlideOutIsCurrent = branchToSlideOutName.equals(gitRepository.getCurrentBranchName());
val branchToSlideOut = branchLayout.getEntryByName(branchToSlideOutName);
if (branchToSlideOut == null) {
// Unlikely, let's handle this case to calm down Checker Framework.
return;
}
// If a branch has children in machete layout,
// it's better to NOT delete it so that fork points for the children are still computed correctly.
// See the point on "too many commits taken into rebase" in https://github.com/VirtusLab/git-machete#faq
val branchToSlideOutHasChildren = branchToSlideOut.getChildren().nonEmpty();
if (branchToSlideOutIsCurrent) {
LOG.debug("Skipping (optional) local branch deletion because it is current branch");
slideOutBranchAndRunPostSlideOutHookIfPresent(() -> {
VcsNotifier.getInstance(project).notifySuccess(/* displayId */ null,
/* title */ "",
getString("action.GitMachete.SlideOut.notification.title.slide-out-success.is-current.HTML").fmt(
branchToSlideOutName));
ModalityUiUtil.invokeLaterIfNeeded(ModalityState.NON_MODAL, doInUIThreadWhenReady);
});
} else if (branchToSlideOutHasChildren) {
LOG.debug("Skipping (optional) local branch deletion because it has children");
slideOutBranchAndRunPostSlideOutHookIfPresent(() -> {
VcsNotifier.getInstance(project).notifySuccess(/* displayId */ null,
/* title */ "",
getString("action.GitMachete.SlideOut.notification.title.slide-out-success.of-current.HTML").fmt(
getString("action.GitMachete.SlideOut.notification.title.slide-out-success.has-children.HTML").fmt(
branchToSlideOutName));
ModalityUiUtil.invokeLaterIfNeeded(ModalityState.NON_MODAL, doInUIThreadWhenReady);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ action.GitMachete.BaseSlideOutAction.description=Slide out ''{0}''
action.GitMachete.SlideOut.notification.message.slide-out-info.canceled.HTML=<html>Action canceled. Branch <b>{0}</b> has not been slid out</html>
action.GitMachete.SlideOut.notification.title.slide-out-fail.HTML=<html>Slide out of <b>{0}</b> failed</html>
action.GitMachete.SlideOut.notification.title.slide-out-info.canceled=Slide out failed
action.GitMachete.SlideOut.notification.title.slide-out-success.of-current.HTML=<html>Branch <b>{0}</b> has been slid out but not deleted as it is currently checked out</html>
action.GitMachete.SlideOut.notification.title.slide-out-success.has-children.HTML=<html>Branch <b>{0}</b> has been slid out but not deleted because it had child branches</html>
action.GitMachete.SlideOut.notification.title.slide-out-success.is-current.HTML=<html>Branch <b>{0}</b> has been slid out but not deleted because it is currently checked out</html>
action.GitMachete.SlideOut.notification.title.slide-out-success.with-delete.HTML=<html>Branch <b>{0}</b> has been slid out and deleted</html>
action.GitMachete.SlideOut.notification.title.slide-out-success.without-delete.HTML=<html>Branch <b>{0}</b> has been slid out but not deleted</html>
string.GitMachete.SlideOut.post-slide-out-hook-task.title=Running machete-post-slide-out hook\u2026
Expand Down
4 changes: 4 additions & 0 deletions src/uiTest/resources/project.rhino.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,10 @@ function Project(underlyingProject) {
return hash.asString();
};

this.doesBranchExist = function (branchName) {
return this.getHashOfCommitPointedByBranch(branchName) != null;
}

this.getSyncToParentStatus = function (child) {
const snapshot = getGraphTable().getGitMacheteRepositorySnapshot();
const managedBranch = snapshot.getManagedBranchByName(child);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ trait RunningIntelliJFixtureExtension extends RobotPluginExtension { this: IdePr
}
}

def doesBranchExist(branch: String): Boolean = {
callJs[java.lang.Boolean](s"project.doesBranchExist('$branch')")
}

def getCurrentBranchName(): String = {
callJs[String]("project.getCurrentBranchName()")
}
Expand Down Expand Up @@ -297,10 +301,6 @@ trait RunningIntelliJFixtureExtension extends RobotPluginExtension { this: IdePr
callJs("project.refreshGraphTableModel().getRowCount()")
}

def rejectBranchDeletionOnSlideOut(): Unit = doAndAwait {
runJs("project.rejectBranchDeletionOnSlideOut()")
}

def resetCurrentToRemote(): Unit = doAndAwait {
runJs(s"project.resetCurrentToRemote()")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ package com.virtuslab.gitmachete.uitest

import com.virtuslab.gitmachete.testcommon.SetupScripts.SETUP_WITH_SINGLE_REMOTE
import com.virtuslab.gitmachete.testcommon.TestGitRepository
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.{
assertEquals,
assertFalse,
assertNotEquals,
assertNotNull,
assertNull,
assertTrue
}
import org.junit.jupiter.api.extension.{ExtendWith, ExtensionContext, TestWatcher}
import org.junit.jupiter.api.{AfterEach, BeforeEach, Test}
import org.virtuslab.ideprobe.Extensions._
Expand Down Expand Up @@ -108,15 +115,19 @@ class UITestSuite extends TestGitRepository(SETUP_WITH_SINGLE_REMOTE) {

// Let's slide out a root branch now
project.slideOutSelected("develop")
project.acceptBranchDeletionOnSlideOut()
// There shouldn't be a branch deletion dialog as `develop` has children (so it will be slid out but not deleted)
assertTrue(project.doesBranchExist("develop"))
branchAndCommitRowsCount = project.refreshModelAndGetRowCount()
// 6 branch rows (`develop` is no longer there) + 7 commit rows
// (1 commit of `allow-ownership-link` and 3 commits of `call-ws` are all gone)
assertEquals(13, branchAndCommitRowsCount)

project.checkoutBranch("master")
assertTrue(project.doesBranchExist("call-ws"))
project.slideOutSelected("call-ws")
project.rejectBranchDeletionOnSlideOut()
project.acceptBranchDeletionOnSlideOut()
assertFalse(project.doesBranchExist("call-ws"))

val managedBranchesAfterSlideOut = project.refreshModelAndGetManagedBranches()
// Non-existent branches should be skipped while causing no error (only a low-severity notification).
assertEquals(
Expand Down