Skip to content
Open
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 internal/cmd/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func GetManifestCmd() *cobra.Command {
// Add subcommands
manifestCmd.AddCommand(GetManifestAddCmd())
manifestCmd.AddCommand(GetManifestListCmd())
manifestCmd.AddCommand(GetManifestDeleteCmd())

addStabilityInfo(manifestCmd)

Expand Down
90 changes: 90 additions & 0 deletions internal/cmd/manifest_delete.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package cmd

import (
"fmt"

"github.com/open-feature/cli/internal/config"
"github.com/open-feature/cli/internal/filesystem"
"github.com/open-feature/cli/internal/logger"
"github.com/open-feature/cli/internal/manifest"
"github.com/pterm/pterm"
"github.com/spf13/afero"
"github.com/spf13/cobra"
)

func GetManifestDeleteCmd() *cobra.Command {
manifestDeleteCmd := &cobra.Command{
Use: "delete <flag-name>",
Short: "Delete a flag from the manifest",
Long: `Delete a flag from the manifest file by its key.

Examples:
# Delete a flag named 'old-feature'
openfeature manifest delete old-feature

# Delete a flag from a specific manifest file
openfeature manifest delete old-feature --manifest path/to/flags.json`,
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
return initializeConfig(cmd, "manifest.delete")
},
RunE: func(cmd *cobra.Command, args []string) error {
flagName := args[0]
manifestPath := config.GetManifestPath(cmd)

// Check if manifest exists
exists, err := afero.Exists(filesystem.FileSystem(), manifestPath)
if err != nil {
return fmt.Errorf("failed to check manifest existence: %w", err)
}

if !exists {
return fmt.Errorf("manifest file does not exist: %s", manifestPath)
}

// Load existing manifest
fs, err := manifest.LoadFlagSet(manifestPath)
if err != nil {
return fmt.Errorf("failed to load manifest: %w", err)
}

// Check if manifest has any flags
if len(fs.Flags) == 0 {
return fmt.Errorf("manifest contains no flags")
}

// Check if flag exists
flagIndex := -1
for i, flag := range fs.Flags {
if flag.Key == flagName {
flagIndex = i
break
}
}

if flagIndex == -1 {
return fmt.Errorf("flag '%s' not found in manifest", flagName)
}

// Remove the flag by creating a new slice without it
fs.Flags = append(fs.Flags[:flagIndex], fs.Flags[flagIndex+1:]...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dig this one-liner, elegant!

Copy link
Member

@erka erka Dec 2, 2025

Choose a reason for hiding this comment

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

slices.DeleteFunc will be a better option here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason to use slices.DeleteFunc over slices.Delete here?
I didn't mind the append pattern but if we want to change it, .Delete seems fine here over .DeleteFunc

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a suggestion using slices.DeleteFunc. It allows us to get rid of the for loop entirely.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oic, thanks!

Comment on lines +51 to +70
Copy link
Contributor

@sahidvelji sahidvelji Dec 3, 2025

Choose a reason for hiding this comment

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

Suggested change
// Check if manifest has any flags
if len(fs.Flags) == 0 {
return fmt.Errorf("manifest contains no flags")
}
// Check if flag exists
flagIndex := -1
for i, flag := range fs.Flags {
if flag.Key == flagName {
flagIndex = i
break
}
}
if flagIndex == -1 {
return fmt.Errorf("flag '%s' not found in manifest", flagName)
}
// Remove the flag by creating a new slice without it
fs.Flags = append(fs.Flags[:flagIndex], fs.Flags[flagIndex+1:]...)
// Remove the flag
originalLen := len(fs.Flags)
fs.Flags = slices.DeleteFunc(fs.Flags, func(flag flagset.Flag) bool {
return flag.Key == flagName
})
// Check if flag was found (length unchanged means nothing was deleted)
if len(fs.Flags) == originalLen {
return fmt.Errorf("flag '%s' not found in manifest", flagName)
}

The zero length check can be skipped as it's not really an edge case and is covered by the "flag not found in manifest" check.


// Write updated manifest
if err := manifest.Write(manifestPath, *fs); err != nil {
return fmt.Errorf("failed to write manifest: %w", err)
}

// Success message
pterm.Success.Printfln("Flag '%s' deleted successfully from %s", flagName, manifestPath)
logger.Default.Debug(fmt.Sprintf("Deleted flag: name=%s, manifestPath=%s", flagName, manifestPath))

return nil
},
}

// Add command-specific flags
config.AddManifestDeleteFlags(manifestDeleteCmd)
addStabilityInfo(manifestDeleteCmd)

return manifestDeleteCmd
}
Loading