fix: use PersistentPreRun instead of OnInitialize for proper command isolation
Replace cobra.OnInitialize with PersistentPreRun pattern to fix initialization issues with setup and server commands. This follows Cobra best practices and allows subcommands to properly override initialization behavior. Problem: - OnInitialize runs globally before command parsing - os.Args check for 'setup' was fragile and broke with flags - Setup wizard failed on server: 'unable to open database file: no such file or directory' Solution: - Use rootCmd.PersistentPreRun for initialization (inherited by all commands) - setup and server commands override with their own PersistentPreRun - Directory overrides still applied correctly in all cases - Removes fragile os.Args parsing Benefits: - Works regardless of flag order - Follows Cobra's intended design patterns - Only 3 files modified (root.go, setup.go, server.go) - Commands that need custom init (setup/server) simply override - All other commands get automatic initialization - Cleaner, more maintainable code Testing: - ✓ opal setup works without initialization errors - ✓ opal list initializes database correctly - ✓ First-run detection still works - ✓ Directory overrides work with flags in any position - ✓ Server command handles its own initialization
This commit is contained in:
@@ -200,7 +200,10 @@ func init() {
|
||||
rootCmd.PersistentFlags().StringVar(&dataDirFlag, "data-dir", "",
|
||||
"Data directory (default: $XDG_DATA_HOME/opal or ~/.local/share/opal)")
|
||||
|
||||
cobra.OnInitialize(initializeApp)
|
||||
// Use PersistentPreRun for initialization (runs for all subcommands unless overridden)
|
||||
rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {
|
||||
initializeApp()
|
||||
}
|
||||
|
||||
// Add regular subcommands
|
||||
rootCmd.AddCommand(addCmd)
|
||||
@@ -224,11 +227,6 @@ func init() {
|
||||
}
|
||||
|
||||
func initializeApp() {
|
||||
// Skip initialization for commands that handle their own setup
|
||||
if len(os.Args) > 1 && os.Args[1] == "setup" {
|
||||
return
|
||||
}
|
||||
|
||||
// Set directory overrides from flags if provided
|
||||
if configDirFlag != "" {
|
||||
engine.SetConfigDirOverride(configDirFlag)
|
||||
|
||||
@@ -77,6 +77,16 @@ var serverCmd = &cobra.Command{
|
||||
Use: "server",
|
||||
Short: "Server management commands",
|
||||
Long: `Commands for running and managing the opal API server`,
|
||||
PersistentPreRun: func(cmd *cobra.Command, args []string) {
|
||||
// Override root's PersistentPreRun - server handles its own initialization
|
||||
// But still apply directory overrides from flags
|
||||
if configDirFlag != "" {
|
||||
engine.SetConfigDirOverride(configDirFlag)
|
||||
}
|
||||
if dataDirFlag != "" {
|
||||
engine.SetDataDirOverride(dataDirFlag)
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
var serverStartCmd = &cobra.Command{
|
||||
|
||||
+9
-10
@@ -36,7 +36,14 @@ Examples:
|
||||
opal setup --show-systemd # Show systemd service template
|
||||
opal setup --show-env # Show environment file template`,
|
||||
PersistentPreRun: func(cmd *cobra.Command, args []string) {
|
||||
// Skip normal initialization - setup command handles its own init
|
||||
// Override root's PersistentPreRun to skip initialization
|
||||
// But still apply directory overrides from flags
|
||||
if configDirFlag != "" {
|
||||
engine.SetConfigDirOverride(configDirFlag)
|
||||
}
|
||||
if dataDirFlag != "" {
|
||||
engine.SetDataDirOverride(dataDirFlag)
|
||||
}
|
||||
},
|
||||
Run: runSetup,
|
||||
}
|
||||
@@ -51,15 +58,7 @@ func init() {
|
||||
}
|
||||
|
||||
func runSetup(cmd *cobra.Command, args []string) {
|
||||
// Apply directory overrides from parent persistent flags
|
||||
if cmd.Parent() != nil {
|
||||
if configDir, _ := cmd.Parent().PersistentFlags().GetString("config-dir"); configDir != "" {
|
||||
engine.SetConfigDirOverride(configDir)
|
||||
}
|
||||
if dataDir, _ := cmd.Parent().PersistentFlags().GetString("data-dir"); dataDir != "" {
|
||||
engine.SetDataDirOverride(dataDir)
|
||||
}
|
||||
}
|
||||
// Directory overrides are already applied in PersistentPreRun
|
||||
|
||||
// Handle template flags
|
||||
if showSystemdFlag {
|
||||
|
||||
Reference in New Issue
Block a user