Skip to content
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 commands/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ var DaemonCommand *cli.Command = &cli.Command{
Subcommands: []*cli.Command{
DaemonInstallCommand,
DaemonUninstallCommand,
DaemonReinstallCommand,
},
}
31 changes: 31 additions & 0 deletions commands/daemon.reinstall.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package commands

import (
"github.com/gookit/color"
"github.com/urfave/cli/v2"
)

var DaemonReinstallCommand = &cli.Command{
Name: "reinstall",
Usage: "Reinstall the shelltime daemon service (uninstall then install)",
Action: commandDaemonReinstall,
}

func commandDaemonReinstall(c *cli.Context) error {
color.Yellow.Println("🔄 Starting daemon service reinstallation...")

// First, uninstall the existing service
color.Yellow.Println("🗑 Uninstalling existing daemon service...")
if err := commandDaemonUninstall(c); err != nil {
return err
}

// Then, install the service
color.Yellow.Println("📦 Installing daemon service...")
if err := commandDaemonInstall(c); err != nil {
return err
}

color.Green.Println("✅ Daemon service has been successfully reinstalled!")
return nil
}
Comment on lines +1 to +31
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The reinstall command should perform its own root check. Currently, it relies on the uninstall and install sub-commands to do this. This can lead to confusing error messages for the user (e.g., sudo shelltime daemon uninstall required, when they ran reinstall).

By adding an explicit root check to commandDaemonReinstall, you provide a clearer error message and make the command's preconditions explicit. This also avoids the redundant root check that would happen in both the uninstall and install steps.

package commands

import (
	"fmt"
	"os"

	"github.com/gookit/color"
	"github.com/urfave/cli/v2"
)

var DaemonReinstallCommand = &cli.Command{
	Name:   "reinstall",
	Usage:  "Reinstall the shelltime daemon service (uninstall then install)",
	Action: commandDaemonReinstall,
}

func commandDaemonReinstall(c *cli.Context) error {
	if os.Geteuid() != 0 {
		return fmt.Errorf("this command must be run as root (sudo shelltime daemon reinstall)")
	}

	color.Yellow.Println("🔄 Starting daemon service reinstallation...")

	// First, uninstall the existing service
	color.Yellow.Println("🗑 Uninstalling existing daemon service...")
	if err := commandDaemonUninstall(c); err != nil {
		return err
	}

	// Then, install the service
	color.Yellow.Println("📦 Installing daemon service...")
	if err := commandDaemonInstall(c); err != nil {
		return err
	}

	color.Green.Println("✅ Daemon service has been successfully reinstalled!")
	return nil
}

Comment on lines +14 to +31
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

By calling commandDaemonUninstall and commandDaemonInstall directly, their success messages are printed during the reinstall process. This results in multiple, somewhat redundant success messages being shown to the user, for example:

✅ Daemon service has been successfully uninstalled!
✅ Daemon service has been installed and started successfully!
✅ Daemon service has been successfully reinstalled!

This can be confusing. For a better user experience, only the final 'reinstalled' message should be shown.

A long-term solution would be to refactor the core logic of install and uninstall into separate functions that don't print final success messages. The install, uninstall, and reinstall command actions could then call these core functions and each print their own appropriate, non-redundant success message. While this refactoring is outside the scope of the current changes, it's a good improvement to consider for enhancing maintainability and user experience.

Loading