mirror of
https://github.com/89luca89/distrobox.git
synced 2026-11-02 20:30:16 -06:00
fix(rm): fix(rm): confirm deletions once and default the prompts to yes, match shell behaviour
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
This commit is contained in:
3 files changed
+62
-2
No files matched your search
@@ -72,6 +72,10 @@ func rmAction(ctx context.Context, cmd *cli.Command, cfg *config.Values) error {
|
||||
|
||||
rmCmd := commands.NewRmCommand(cfg, containerManager, prompter)
|
||||
_, err := rmCmd.Execute(ctx, options)
|
||||
if errors.Is(err, commands.ErrRmAbortedByUser) {
|
||||
ui.NewPrinter(os.Stdout, true).Println("Aborted.")
|
||||
return nil
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to execute rm command: %w", err)
|
||||
}
|
||||
|
||||
+21
-1
@@ -19,6 +19,10 @@ type RmResult struct {
|
||||
Containers []containermanager.Container
|
||||
}
|
||||
|
||||
// ErrRmAbortedByUser is returned when the user declines the top-level deletion
|
||||
// confirmation, so the CLI can print "Aborted." and exit cleanly.
|
||||
var ErrRmAbortedByUser = errors.New("rm operation aborted by user")
|
||||
|
||||
type RmCommand struct {
|
||||
cfg *config.Values
|
||||
containerManager containermanager.ContainerManager
|
||||
@@ -64,6 +68,21 @@ func (c *RmCommand) Execute(ctx context.Context, options RmOptions) (*RmResult,
|
||||
|
||||
distroboxesToRemove := getContainersToRemove(listResult.Containers, options.ContainerNames, options.All)
|
||||
|
||||
// Single top-level confirmation, matching the shell (distrobox-rm:413-419):
|
||||
// default "yes", skipped under --force/--yes; a bare Enter proceeds.
|
||||
if !options.Force && !options.NoTTY && len(distroboxesToRemove) > 0 {
|
||||
names := make([]string, 0, len(distroboxesToRemove))
|
||||
for _, d := range distroboxesToRemove {
|
||||
names = append(names, d.Name)
|
||||
}
|
||||
if !c.prompter.Prompt(
|
||||
fmt.Sprintf("Do you really want to delete containers: %s?", strings.Join(names, " ")),
|
||||
true,
|
||||
) {
|
||||
return nil, ErrRmAbortedByUser
|
||||
}
|
||||
}
|
||||
|
||||
userEnv := userenv.LoadUserEnvironment(ctx)
|
||||
userHome := userEnv.Home
|
||||
|
||||
@@ -91,7 +110,8 @@ func (c *RmCommand) removeContainer(
|
||||
) error {
|
||||
forceRemove := force
|
||||
if !forceRemove && !noTTY && strings.Contains(container.Status, "Up") {
|
||||
if c.prompter.Prompt("Container is running, do you want to force delete it?", false) {
|
||||
// Shell defaults this prompt to "yes" (distrobox-rm:424-426).
|
||||
if c.prompter.Prompt("Container is running, do you want to force delete it?", true) {
|
||||
forceRemove = true
|
||||
} else {
|
||||
return nil
|
||||
|
||||
+37
-1
@@ -53,12 +53,15 @@ func TestRmCommand_RmHome_NotRequested_NeverPrompts(t *testing.T) {
|
||||
},
|
||||
InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/custom/home/dir"},
|
||||
}
|
||||
// Seed a "y" — if the prompt were (wrongly) shown, the home would be removed.
|
||||
// Seed a "y" — if the home prompt were (wrongly) shown, it would be consumed
|
||||
// and the home removed. Force bypasses the top-level deletion confirmation so
|
||||
// the only prompt that could fire is the home one.
|
||||
cmd := newTestRmCommandWithInput(mock, "y\n")
|
||||
|
||||
_, err := cmd.Execute(context.Background(), commands.RmOptions{
|
||||
ContainerNames: []string{containerName},
|
||||
RemoveHome: false,
|
||||
Force: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, lastRemoveOptions(t, mock).RemoveHome,
|
||||
@@ -76,11 +79,14 @@ func TestRmCommand_RmHome_Requested_ConfirmedRemoves(t *testing.T) {
|
||||
},
|
||||
InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/custom/home/dir"},
|
||||
}
|
||||
// Force bypasses the top-level deletion confirmation; the seeded "y" answers
|
||||
// the home-removal prompt.
|
||||
cmd := newTestRmCommandWithInput(mock, "y\n")
|
||||
|
||||
_, err := cmd.Execute(context.Background(), commands.RmOptions{
|
||||
ContainerNames: []string{containerName},
|
||||
RemoveHome: true,
|
||||
Force: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, lastRemoveOptions(t, mock).RemoveHome,
|
||||
@@ -111,6 +117,36 @@ func TestRmCommand_RmHome_Requested_NonInteractiveKeepsHome(t *testing.T) {
|
||||
"non-interactive rm must not remove the home even with --rm-home")
|
||||
}
|
||||
|
||||
func TestRmCommand_TopLevelConfirmation_DeclineAborts(t *testing.T) {
|
||||
t.Setenv("HOME", t.TempDir())
|
||||
mock := &testutil.MockContainerManager{
|
||||
ListContainersResult: []containermanager.Container{
|
||||
{Name: "boxa", Status: "Exited", Labels: map[string]string{"manager": "distrobox"}},
|
||||
},
|
||||
InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/home/x"},
|
||||
}
|
||||
cmd := newTestRmCommandWithInput(mock, "n\n")
|
||||
|
||||
_, err := cmd.Execute(context.Background(), commands.RmOptions{ContainerNames: []string{"boxa"}})
|
||||
require.ErrorIs(t, err, commands.ErrRmAbortedByUser)
|
||||
assert.Empty(t, mock.Spy.Remove, "declining the confirmation must remove nothing")
|
||||
}
|
||||
|
||||
func TestRmCommand_TopLevelConfirmation_DefaultYesProceeds(t *testing.T) {
|
||||
t.Setenv("HOME", t.TempDir())
|
||||
mock := &testutil.MockContainerManager{
|
||||
ListContainersResult: []containermanager.Container{
|
||||
{Name: "boxa", Status: "Exited", Labels: map[string]string{"manager": "distrobox"}},
|
||||
},
|
||||
InspectContainerResult: &containermanager.InspectResult{ContainerHome: "/home/x"},
|
||||
}
|
||||
cmd := newTestRmCommandWithInput(mock, "\n")
|
||||
|
||||
_, err := cmd.Execute(context.Background(), commands.RmOptions{ContainerNames: []string{"boxa"}})
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, mock.Spy.Remove, 1, "a bare Enter (default yes) must proceed with deletion")
|
||||
}
|
||||
|
||||
// writeExportedDesktopApp writes a minimal desktop file that
|
||||
// findExportedDesktopApps will match for the given containerName (i.e.
|
||||
// the per-app desktop entries created by distrobox-export, not the
|
||||
|
||||
Reference in new issue
Block a user