-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: adding delete command and deprecate eject command #157
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
package cli | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
|
||
"github.com/futurice/jalapeno/internal/cli/option" | ||
"github.com/futurice/jalapeno/pkg/recipe" | ||
"github.com/futurice/jalapeno/pkg/ui/colors" | ||
"github.com/gofrs/uuid" | ||
"github.com/spf13/cobra" | ||
) | ||
|
||
type deleteOptions struct { | ||
SauceID string | ||
All bool | ||
|
||
option.Common | ||
option.WorkingDirectory | ||
} | ||
|
||
func NewDeleteCmd() *cobra.Command { | ||
var opts deleteOptions | ||
var cmd = &cobra.Command{ | ||
Use: "delete [SAUCE_ID]", | ||
Short: "Delete sauce(s) from the project", | ||
Long: `Delete sauce(s) from the project. This will remove the rendered files and the sauce entry from sauces.yml. | ||
If no sauce ID is provided and --all flag is not set, this command will fail.`, | ||
PreRunE: func(cmd *cobra.Command, args []string) error { | ||
if len(args) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
opts.SauceID = args[0] | ||
} | ||
return option.Parse(&opts) | ||
}, | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
err := runDelete(cmd, opts) | ||
return errorHandler(cmd, err) | ||
}, | ||
Example: `# Delete a specific sauce | ||
jalapeno delete 21872763-f48e-4728-bc49-57f5898e098a | ||
|
||
# Delete all sauces (same as 'jalapeno eject') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we remove the eject command completely, the note can be removed |
||
jalapeno delete --all`, | ||
} | ||
|
||
cmd.Flags().BoolVar(&opts.All, "all", false, "Delete all sauces from the project") | ||
|
||
if err := option.ApplyFlags(&opts, cmd.Flags()); err != nil { | ||
return nil | ||
} | ||
|
||
return cmd | ||
} | ||
|
||
func runDelete(cmd *cobra.Command, opts deleteOptions) error { | ||
if !opts.All && opts.SauceID == "" { | ||
return errors.New("either provide a sauce ID or use --all flag") | ||
} | ||
|
||
if opts.All { | ||
return deleteAll(cmd, opts) | ||
} | ||
|
||
return deleteSauce(cmd, opts) | ||
} | ||
|
||
func deleteSauce(cmd *cobra.Command, opts deleteOptions) error { | ||
id, err := uuid.FromString(opts.SauceID) | ||
if err != nil { | ||
return fmt.Errorf("invalid sauce ID: %w", err) | ||
} | ||
|
||
sauce, err := recipe.LoadSauceByID(opts.Dir, id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could refactor the |
||
if err != nil { | ||
if errors.Is(err, recipe.ErrSauceNotFound) { | ||
return fmt.Errorf("sauce with ID '%s' not found", opts.SauceID) | ||
} | ||
return err | ||
} | ||
|
||
// Delete rendered files | ||
for path := range sauce.Files { | ||
fullPath := filepath.Join(opts.Dir, path) | ||
err := os.Remove(fullPath) | ||
if err != nil && !errors.Is(err, os.ErrNotExist) { | ||
return fmt.Errorf("failed to delete file '%s': %w", path, err) | ||
} | ||
} | ||
|
||
// Delete sauce entry | ||
sauces, err := recipe.LoadSauces(opts.Dir) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
filteredSauces := make([]*recipe.Sauce, 0, len(sauces)) | ||
for _, s := range sauces { | ||
if s.ID != id { | ||
filteredSauces = append(filteredSauces, s) | ||
} | ||
} | ||
|
||
// If this was the last sauce, delete the entire .jalapeno directory | ||
if len(filteredSauces) == 0 { | ||
jalapenoPath := filepath.Join(opts.Dir, recipe.SauceDirName) | ||
err = os.RemoveAll(jalapenoPath) | ||
if err != nil { | ||
return err | ||
} | ||
} else { | ||
// Otherwise just save the filtered sauces | ||
err = recipe.SaveSauces(opts.Dir, filteredSauces) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
cmd.Printf("Sauce '%s' deleted %s\n", sauce.Recipe.Name, colors.Green.Render("successfully!")) | ||
return nil | ||
} | ||
|
||
func deleteAll(cmd *cobra.Command, opts deleteOptions) error { | ||
jalapenoPath := filepath.Join(opts.Dir, recipe.SauceDirName) | ||
|
||
if stat, err := os.Stat(jalapenoPath); os.IsNotExist(err) || !stat.IsDir() { | ||
return fmt.Errorf("'%s' is not a Jalapeno project", opts.Dir) | ||
} | ||
Comment on lines
+127
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can check is the directory "a Jalapeno project" by looking the return value of |
||
|
||
// Delete all rendered files first | ||
sauces, err := recipe.LoadSauces(opts.Dir) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
for _, sauce := range sauces { | ||
for path := range sauce.Files { | ||
fullPath := filepath.Join(opts.Dir, path) | ||
err := os.Remove(fullPath) | ||
if err != nil && !errors.Is(err, os.ErrNotExist) { | ||
return fmt.Errorf("failed to delete file '%s': %w", path, err) | ||
} | ||
} | ||
} | ||
|
||
// Delete .jalapeno directory | ||
cmd.Printf("Deleting %s...\n", jalapenoPath) | ||
err = os.RemoveAll(jalapenoPath) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
cmd.Printf("All sauces deleted %s\n", colors.Green.Render("successfully!")) | ||
return nil | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also refactor the end-to-end tests to work with this new command. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
package cli | ||
|
||
import ( | ||
"os" | ||
"path/filepath" | ||
"testing" | ||
|
||
"github.com/futurice/jalapeno/pkg/recipe" | ||
"github.com/gofrs/uuid" | ||
) | ||
|
||
func TestDelete(t *testing.T) { | ||
// Create a temporary directory for testing | ||
dir, err := os.MkdirTemp("", "jalapeno-test-delete") | ||
if err != nil { | ||
t.Fatalf("cannot create temp dir: %s", err) | ||
} | ||
defer os.RemoveAll(dir) | ||
|
||
// Create test files and directories | ||
if err = os.MkdirAll(filepath.Join(dir, recipe.SauceDirName), 0755); err != nil { | ||
t.Fatalf("cannot create metadata dir: %s", err) | ||
} | ||
|
||
// Create some test files that will be "managed" by the sauces | ||
testFiles := []string{"first.md", "second.md"} | ||
for _, f := range testFiles { | ||
if err = os.WriteFile(filepath.Join(dir, f), []byte("# "+f), 0644); err != nil { | ||
t.Fatalf("cannot write test file: %s", err) | ||
} | ||
} | ||
|
||
// Create test sauces | ||
id1 := uuid.Must(uuid.NewV4()) | ||
id2 := uuid.Must(uuid.NewV4()) | ||
|
||
sauces := []*recipe.Sauce{ | ||
{ | ||
APIVersion: "v1", | ||
ID: id1, | ||
Recipe: recipe.Recipe{ | ||
Metadata: recipe.Metadata{ | ||
APIVersion: "v1", | ||
Name: "foo", | ||
Version: "v1.0.0", | ||
}, | ||
}, | ||
Files: map[string]recipe.File{ | ||
"first.md": recipe.NewFile([]byte("# first")), | ||
}, | ||
}, | ||
{ | ||
APIVersion: "v1", | ||
ID: id2, | ||
Recipe: recipe.Recipe{ | ||
Metadata: recipe.Metadata{ | ||
APIVersion: "v1", | ||
Name: "bar", | ||
Version: "v2.0.0", | ||
}, | ||
}, | ||
Files: map[string]recipe.File{ | ||
"second.md": recipe.NewFile([]byte("# second")), | ||
}, | ||
}, | ||
} | ||
|
||
if err = recipe.SaveSauces(dir, sauces); err != nil { | ||
t.Fatalf("cannot save test sauces: %s", err) | ||
} | ||
|
||
t.Run("delete specific sauce", func(t *testing.T) { | ||
cmd := NewRootCmd() | ||
cmd.SetArgs([]string{"delete", id1.String(), "--dir", dir}) | ||
|
||
if err := cmd.Execute(); err != nil { | ||
t.Fatalf("failed to execute delete command: %s", err) | ||
} | ||
|
||
// Check that first.md was deleted | ||
if _, err := os.Stat(filepath.Join(dir, "first.md")); !os.IsNotExist(err) { | ||
t.Error("first.md should have been deleted") | ||
} | ||
|
||
// Check that second.md still exists | ||
if _, err := os.Stat(filepath.Join(dir, "second.md")); err != nil { | ||
t.Error("second.md should still exist") | ||
} | ||
|
||
// Check that only one sauce remains | ||
remainingSauces, err := recipe.LoadSauces(dir) | ||
if err != nil { | ||
t.Fatalf("failed to load sauces: %s", err) | ||
} | ||
|
||
if len(remainingSauces) != 1 { | ||
t.Errorf("expected 1 sauce, got %d", len(remainingSauces)) | ||
} | ||
|
||
if remainingSauces[0].ID != id2 { | ||
t.Error("wrong sauce was deleted") | ||
} | ||
}) | ||
|
||
t.Run("delete all sauces", func(t *testing.T) { | ||
cmd := NewRootCmd() | ||
cmd.SetArgs([]string{"delete", "--all", "--dir", dir}) | ||
|
||
if err := cmd.Execute(); err != nil { | ||
t.Fatalf("failed to execute delete command: %s", err) | ||
} | ||
|
||
// Check that both files were deleted | ||
for _, f := range testFiles { | ||
if _, err := os.Stat(filepath.Join(dir, f)); !os.IsNotExist(err) { | ||
t.Errorf("%s should have been deleted", f) | ||
} | ||
} | ||
|
||
// Check that .jalapeno directory was deleted | ||
if _, err := os.Stat(filepath.Join(dir, recipe.SauceDirName)); !os.IsNotExist(err) { | ||
t.Error(".jalapeno directory should have been deleted") | ||
} | ||
}) | ||
|
||
t.Run("delete with invalid sauce ID", func(t *testing.T) { | ||
cmd := NewRootCmd() | ||
cmd.SetArgs([]string{"delete", "invalid-uuid", "--dir", dir}) | ||
|
||
if err := cmd.Execute(); err == nil { | ||
t.Fatal("expected error with invalid sauce ID") | ||
} | ||
}) | ||
|
||
t.Run("delete without sauce ID or --all flag", func(t *testing.T) { | ||
cmd := NewRootCmd() | ||
cmd.SetArgs([]string{"delete", "--dir", dir}) | ||
|
||
if err := cmd.Execute(); err == nil { | ||
t.Fatal("expected error when no sauce ID or --all flag provided") | ||
} | ||
}) | ||
|
||
t.Run("delete non-existent sauce", func(t *testing.T) { | ||
nonExistentID := uuid.Must(uuid.NewV4()) | ||
cmd := NewRootCmd() | ||
cmd.SetArgs([]string{"delete", nonExistentID.String(), "--dir", dir}) | ||
|
||
if err := cmd.Execute(); err == nil { | ||
t.Fatal("expected error when deleting non-existent sauce") | ||
} | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,9 +20,10 @@ type ejectOptions struct { | |
func NewEjectCmd() *cobra.Command { | ||
var opts ejectOptions | ||
var cmd = &cobra.Command{ | ||
Use: "eject", | ||
Short: "Remove all Jalapeno-specific files from a project", | ||
Long: "Remove all the files and directories that are for Jalapeno internal use, and leave only the rendered project files.", | ||
Use: "eject", | ||
Short: "Remove all Jalapeno-specific files from a project", | ||
Long: "Remove all the files and directories that are for Jalapeno internal use, and leave only the rendered project files.", | ||
Deprecated: "use 'delete --all' instead", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a nice idea to deprecate the command instead of removing it, but since the command is so rarely used we can already remove it completely. |
||
PreRunE: func(cmd *cobra.Command, args []string) error { | ||
return option.Parse(&opts) | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -278,3 +278,28 @@ func LoadManifest(path string) (*Manifest, error) { | |
|
||
return manifest, nil | ||
} | ||
|
||
// SaveSauces saves the given sauces to the project directory | ||
func SaveSauces(projectDir string, sauces []*Sauce) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is no need for this helper function. We should utilize the existing
|
||
if err := os.MkdirAll(filepath.Join(projectDir, SauceDirName), 0755); err != nil { | ||
return fmt.Errorf("failed to create sauce directory: %w", err) | ||
} | ||
|
||
sauceFile := filepath.Join(projectDir, SauceDirName, SaucesFileName+YAMLExtension) | ||
f, err := os.Create(sauceFile) | ||
if err != nil { | ||
return fmt.Errorf("failed to create sauce file: %w", err) | ||
} | ||
defer f.Close() | ||
|
||
encoder := yaml.NewEncoder(f) | ||
defer encoder.Close() | ||
|
||
for _, sauce := range sauces { | ||
if err := encoder.Encode(sauce); err != nil { | ||
return fmt.Errorf("failed to encode sauce: %w", err) | ||
} | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note: I didn't mention this in the issue description, but you've probably also noticed that at the moment there is no clean way to get the sauce ID other than manually looking it up from the
sauces.yml
file. Later we could have a command to list all the sauces for easier access to IDs.We should document how to use this command at the end of the
usage
page (how to get the sauce ID etc).