From f66c59fd57fbac3099c545a80f2bf1e19dec2c3e Mon Sep 17 00:00:00 2001 From: Rogee Date: Wed, 29 Oct 2025 18:59:55 +0800 Subject: [PATCH] feat: implement remove command with sequential removals --- AGENTS.md | 9 +- cmd/remove.go | 107 +++++++++++++ cmd/root.go | 1 + docs/CHANGELOG.md | 2 + docs/cli-flags.md | 23 +++ internal/remove/apply.go | 92 ++++++++++++ internal/remove/engine.go | 36 +++++ internal/remove/parser.go | 11 +- internal/remove/preview.go | 106 +++++++++++++ internal/remove/request.go | 2 +- internal/remove/summary.go | 142 ++++++++---------- internal/remove/traversal.go | 4 + scripts/smoke-test-remove.sh | 14 +- specs/003-add-remove-command/tasks.md | 40 ++--- testdata/remove/basic/Project copy.txt | 0 testdata/remove/basic/README.md | 32 ++++ .../remove/basic/collisions/alpha draft.txt | 0 testdata/remove/basic/collisions/alpha.txt | 0 testdata/remove/basic/empty-basename/draft | 0 .../remove/basic/empty-basename/draft.txt | 0 .../remove/basic/nested/foo draft draft.txt | 0 testdata/remove/basic/nested/foo draft.txt | 0 .../remove/basic/nested/hidden/.draft copy.md | 0 testdata/remove/basic/project copy draft.txt | 0 tests/contract/remove_command_ledger_test.go | 104 +++++++++++++ tests/contract/remove_command_preview_test.go | 46 ++++++ tests/integration/remove_flow_test.go | 86 +++++++++++ tests/integration/remove_undo_test.go | 57 +++++++ tests/integration/remove_validation_test.go | 62 ++++++++ tests/unit/remove_engine_test.go | 74 +++++++++ tests/unit/remove_parser_test.go | 46 ++++++ 31 files changed, 986 insertions(+), 110 deletions(-) create mode 100644 cmd/remove.go create mode 100644 internal/remove/apply.go create mode 100644 internal/remove/engine.go create mode 100644 internal/remove/preview.go create mode 100644 testdata/remove/basic/Project copy.txt create mode 100644 testdata/remove/basic/README.md create mode 100644 testdata/remove/basic/collisions/alpha draft.txt create mode 100644 testdata/remove/basic/collisions/alpha.txt create mode 100644 testdata/remove/basic/empty-basename/draft create mode 100644 testdata/remove/basic/empty-basename/draft.txt create mode 100644 testdata/remove/basic/nested/foo draft draft.txt create mode 100644 testdata/remove/basic/nested/foo draft.txt create mode 100644 testdata/remove/basic/nested/hidden/.draft copy.md create mode 100644 testdata/remove/basic/project copy draft.txt create mode 100644 tests/contract/remove_command_ledger_test.go create mode 100644 tests/contract/remove_command_preview_test.go create mode 100644 tests/integration/remove_flow_test.go create mode 100644 tests/integration/remove_undo_test.go create mode 100644 tests/integration/remove_validation_test.go create mode 100644 tests/unit/remove_engine_test.go create mode 100644 tests/unit/remove_parser_test.go diff --git a/AGENTS.md b/AGENTS.md index 2cb9381..b8dafd9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,7 @@ tests/ - `renamer list` — preview rename scope with shared flags before executing changes. - `renamer replace` — consolidate multiple literal patterns into a single replacement (supports `--dry-run` + `--yes`). +- `renamer remove` — delete ordered substrings from filenames with empty-name protections, duplicate warnings, and undoable ledger entries. - `renamer undo` — revert the most recent rename/replace batch using ledger entries. - Persistent scope flags: `--path`, `-r/--recursive`, `-d/--include-dirs`, `--hidden`, `--extensions`, `--yes`, `--dry-run`. @@ -32,12 +33,12 @@ tests/ ## Testing - `go test ./...` -- Contract tests: `tests/contract/replace_command_test.go` -- Integration tests: `tests/integration/replace_flow_test.go` -- Smoke: `scripts/smoke-test-replace.sh` +- Contract tests: `tests/contract/replace_command_test.go`, `tests/contract/remove_command_preview_test.go`, `tests/contract/remove_command_ledger_test.go` +- Integration tests: `tests/integration/replace_flow_test.go`, `tests/integration/remove_flow_test.go`, `tests/integration/remove_undo_test.go`, `tests/integration/remove_validation_test.go` +- Smoke: `scripts/smoke-test-replace.sh`, `scripts/smoke-test-remove.sh` ## Recent Changes -- 003-add-remove-command: Added Go 1.24 + `spf13/cobra`, `spf13/pflag` +- 003-add-remove-command: Added sequential `renamer remove` subcommand, automation-friendly ledger metadata, and CLI warnings for duplicates/empty results - 002-add-replace-command: Added `renamer replace` command, ledger metadata, and automation docs. - 001-list-command-filters: Added `renamer list` command with shared scope flags and formatters. diff --git a/cmd/remove.go b/cmd/remove.go new file mode 100644 index 0000000..c9c0d81 --- /dev/null +++ b/cmd/remove.go @@ -0,0 +1,107 @@ +package cmd + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + + "github.com/rogeecn/renamer/internal/listing" + "github.com/rogeecn/renamer/internal/remove" +) + +// NewRemoveCommand constructs the remove CLI command; exported for testing. +func NewRemoveCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove [pattern2 ...]", + Short: "Remove literal substrings sequentially from names", + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + parsed, err := remove.ParseArgs(args) + if err != nil { + return err + } + + scope, err := listing.ScopeFromCmd(cmd) + if err != nil { + return err + } + + req, err := remove.FromListing(scope, parsed.Tokens) + if err != nil { + return err + } + + dryRun, err := getBool(cmd, "dry-run") + if err != nil { + return err + } + autoApply, err := getBool(cmd, "yes") + if err != nil { + return err + } + + if dryRun && autoApply { + return errors.New("--dry-run cannot be combined with --yes; remove one of them") + } + + out := cmd.OutOrStdout() + + summary, planned, err := remove.Preview(cmd.Context(), req, parsed, out) + if err != nil { + return err + } + + for _, empty := range summary.Empties { + fmt.Fprintf(out, "Warning: %s would become empty; skipping\n", empty) + } + + for _, dup := range summary.SortedDuplicates() { + fmt.Fprintf(out, "Warning: token %q provided multiple times\n", dup) + } + + if len(summary.Conflicts) > 0 { + for _, conflict := range summary.Conflicts { + fmt.Fprintf(out, "CONFLICT: %s -> %s (%s)\n", conflict.OriginalPath, conflict.ProposedPath, conflict.Reason) + } + return errors.New("conflicts detected; aborting") + } + + if summary.ChangedCount == 0 { + fmt.Fprintln(out, "No removals required") + return nil + } + + fmt.Fprintf(out, "Planned removals: %d entries updated across %d candidates\n", summary.ChangedCount, summary.TotalCandidates) + for _, pair := range summary.SortedTokenMatches() { + fmt.Fprintf(out, " %s -> %d occurrences\n", pair.Token, pair.Count) + } + + if dryRun || !autoApply { + fmt.Fprintln(out, "Preview complete. Re-run with --yes to apply.") + return nil + } + + entry, err := remove.Apply(cmd.Context(), req, planned, summary, parsed.Tokens) + if err != nil { + return err + } + + if len(entry.Operations) == 0 { + fmt.Fprintln(out, "Nothing to apply; preview already up to date.") + return nil + } + + fmt.Fprintf(out, "Applied %d removals. Ledger updated.\n", len(entry.Operations)) + return nil + }, + } + + cmd.Example = " renamer remove \" copy\" \" draft\" --dry-run\n renamer remove foo bar --yes --path ./docs" + + return cmd +} + +func init() { + rootCmd.AddCommand(NewRemoveCommand()) +} diff --git a/cmd/root.go b/cmd/root.go index 3e5ae2d..d0bb4c2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -46,6 +46,7 @@ func NewRootCommand() *cobra.Command { listing.RegisterScopeFlags(cmd.PersistentFlags()) cmd.AddCommand(newListCommand()) cmd.AddCommand(NewReplaceCommand()) + cmd.AddCommand(NewRemoveCommand()) cmd.AddCommand(newUndoCommand()) return cmd diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index fcdaafc..16d38d1 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add `renamer remove` subcommand with sequential multi-token deletions, empty-name safeguards, and ledger-backed undo. +- Document remove command ordering semantics, duplicate warnings, and automation guidance. - Add `renamer replace` subcommand supporting multi-pattern replacements, preview/apply/undo, and scope flags. - Document quoting guidance, `--dry-run` / `--yes` behavior, and automation scenarios for replace command. - Add `renamer list` subcommand with shared scope flags and plain/table output formats. diff --git a/docs/cli-flags.md b/docs/cli-flags.md index 8cf41b3..ad08b97 100644 --- a/docs/cli-flags.md +++ b/docs/cli-flags.md @@ -48,3 +48,26 @@ renamer replace [pattern2 ...] [flags] - List JPEGs only: `renamer --extensions .jpg list` - Replace multiple patterns: `renamer replace draft Draft final --dry-run` - Include dotfiles: `renamer --hidden --extensions .env list` + +## Remove Command Quick Reference + +```bash +renamer remove [token2 ...] [flags] +``` + +- Removal tokens are evaluated in the order supplied. Each token deletes literal substrings from the + current filename before the next token runs; results are previewed before any filesystem changes. +- Duplicate tokens are deduplicated automatically and surfaced as warnings so users can adjust + scripts without surprises. +- Tokens that collapse a filename to an empty string are skipped with warnings during preview/apply + to protect against accidental deletion. +- All scope flags (`--path`, `-r`, `-d`, `--hidden`, `-e`) apply, making it easy to target directories, + recurse, and limit removals by extension. +- Use `--dry-run` for automation previews and combine with `--yes` to apply unattended; conflicting + combinations (`--dry-run --yes`) exit with an error to uphold preview-first safety. + +### Usage Examples + +- Preview sequential removals: `renamer remove " copy" " draft" --dry-run` +- Remove tokens recursively: `renamer remove foo foo- --recursive --path ./reports` +- Combine with extension filters: `renamer remove " Project" --extensions .txt|.md --dry-run` diff --git a/internal/remove/apply.go b/internal/remove/apply.go new file mode 100644 index 0000000..0ba987b --- /dev/null +++ b/internal/remove/apply.go @@ -0,0 +1,92 @@ +package remove + +import ( + "context" + "errors" + "os" + "path/filepath" + "sort" + + "github.com/rogeecn/renamer/internal/history" +) + +// Apply executes planned removals and appends the result to the ledger. +func Apply(ctx context.Context, req *Request, planned []PlannedOperation, summary Summary, orderedTokens []string) (history.Entry, error) { + entry := history.Entry{Command: "remove"} + + if len(planned) == 0 { + return entry, nil + } + + sort.SliceStable(planned, func(i, j int) bool { + return planned[i].Result.Candidate.Depth > planned[j].Result.Candidate.Depth + }) + + done := make([]history.Operation, 0, len(planned)) + + revert := func() error { + for i := len(done) - 1; i >= 0; i-- { + op := done[i] + source := filepath.Join(req.WorkingDir, op.To) + destination := filepath.Join(req.WorkingDir, op.From) + if err := os.Rename(source, destination); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + return nil + } + + for _, op := range planned { + if err := ctx.Err(); err != nil { + _ = revert() + return history.Entry{}, err + } + + from := op.Result.Candidate.OriginalPath + to := op.TargetAbsolute + + if from == to { + continue + } + + if err := os.Rename(from, to); err != nil { + _ = revert() + return history.Entry{}, err + } + + done = append(done, history.Operation{ + From: filepath.ToSlash(op.Result.Candidate.RelativePath), + To: filepath.ToSlash(op.TargetRelative), + }) + } + + if len(done) == 0 { + return entry, nil + } + + entry.Operations = done + + matchesCopy := make(map[string]int, len(summary.TokenMatches)) + for token, count := range summary.TokenMatches { + matchesCopy[token] = count + } + + tokensCopy := append([]string(nil), orderedTokens...) + + entry.Metadata = map[string]any{ + "tokens": tokensCopy, + "matches": matchesCopy, + "changed": summary.ChangedCount, + "totalCandidates": summary.TotalCandidates, + } + if len(summary.Empties) > 0 { + entry.Metadata["empties"] = append([]string(nil), summary.Empties...) + } + + if err := history.Append(req.WorkingDir, entry); err != nil { + _ = revert() + return history.Entry{}, err + } + + return entry, nil +} diff --git a/internal/remove/engine.go b/internal/remove/engine.go new file mode 100644 index 0000000..7f0dbd5 --- /dev/null +++ b/internal/remove/engine.go @@ -0,0 +1,36 @@ +package remove + +import "strings" + +// Result captures the outcome of applying sequential removals to a candidate. +type Result struct { + Candidate Candidate + ProposedName string + Matches map[string]int + Changed bool +} + +// ApplyTokens removes each token sequentially from the candidate's basename. +func ApplyTokens(candidate Candidate, tokens []string) Result { + current := candidate.BaseName + matches := make(map[string]int, len(tokens)) + + for _, token := range tokens { + if token == "" { + continue + } + count := strings.Count(current, token) + if count == 0 { + continue + } + current = strings.ReplaceAll(current, token, "") + matches[token] += count + } + + return Result{ + Candidate: candidate, + ProposedName: current, + Matches: matches, + Changed: current != candidate.BaseName, + } +} diff --git a/internal/remove/parser.go b/internal/remove/parser.go index f358624..06cd944 100644 --- a/internal/remove/parser.go +++ b/internal/remove/parser.go @@ -14,16 +14,15 @@ func ParseArgs(args []string) (ParseArgsResult, error) { seen := make(map[string]int) for _, raw := range args { - token := strings.TrimSpace(raw) - if token == "" { + if strings.TrimSpace(raw) == "" { continue } - if _, exists := seen[token]; exists { - result.Duplicates = append(result.Duplicates, token) + if _, exists := seen[raw]; exists { + result.Duplicates = append(result.Duplicates, raw) continue } - seen[token] = len(result.Tokens) - result.Tokens = append(result.Tokens, token) + seen[raw] = len(result.Tokens) + result.Tokens = append(result.Tokens, raw) } if len(result.Tokens) == 0 { diff --git a/internal/remove/preview.go b/internal/remove/preview.go new file mode 100644 index 0000000..0940343 --- /dev/null +++ b/internal/remove/preview.go @@ -0,0 +1,106 @@ +package remove + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" +) + +// PlannedOperation represents a rename that will be executed during apply. +type PlannedOperation struct { + Result Result + TargetRelative string + TargetAbsolute string +} + +// Preview computes removals and writes a human-readable summary to out. +func Preview(ctx context.Context, req *Request, parsed ParseArgsResult, out io.Writer) (Summary, []PlannedOperation, error) { + summary := NewSummary() + for _, dup := range parsed.Duplicates { + summary.AddDuplicate(dup) + } + + planned := make([]PlannedOperation, 0) + plannedTargets := make(map[string]string) + + err := Traverse(ctx, req, func(candidate Candidate) error { + res := ApplyTokens(candidate, parsed.Tokens) + summary.RecordCandidate(res) + + if !res.Changed { + return nil + } + + if res.ProposedName == "" { + summary.AddEmpty(candidate.RelativePath) + return nil + } + + dir := filepath.Dir(candidate.RelativePath) + if dir == "." { + dir = "" + } + + targetRelative := res.ProposedName + if dir != "" { + targetRelative = filepath.ToSlash(filepath.Join(dir, res.ProposedName)) + } else { + targetRelative = filepath.ToSlash(res.ProposedName) + } + + if targetRelative == candidate.RelativePath { + return nil + } + + if existing, ok := plannedTargets[targetRelative]; ok && existing != candidate.RelativePath { + summary.AddConflict(ConflictDetail{ + OriginalPath: candidate.RelativePath, + ProposedPath: targetRelative, + Reason: "duplicate target generated in preview", + }) + return nil + } + + targetAbsolute := filepath.Join(req.WorkingDir, filepath.FromSlash(targetRelative)) + if info, err := os.Stat(targetAbsolute); err == nil { + if candidate.OriginalPath != targetAbsolute { + if origInfo, origErr := os.Stat(candidate.OriginalPath); origErr == nil && os.SameFile(info, origInfo) { + goto recordOperation + } + + reason := "target already exists" + if info.IsDir() { + reason = "target directory already exists" + } + summary.AddConflict(ConflictDetail{ + OriginalPath: candidate.RelativePath, + ProposedPath: targetRelative, + Reason: reason, + }) + return nil + } + } + + plannedTargets[targetRelative] = candidate.RelativePath + + if out != nil { + fmt.Fprintf(out, "%s -> %s\n", candidate.RelativePath, targetRelative) + } + + recordOperation: + planned = append(planned, PlannedOperation{ + Result: res, + TargetRelative: targetRelative, + TargetAbsolute: targetAbsolute, + }) + + return nil + }) + if err != nil { + return Summary{}, nil, err + } + + return summary, planned, nil +} diff --git a/internal/remove/request.go b/internal/remove/request.go index 6dedd63..7f7577d 100644 --- a/internal/remove/request.go +++ b/internal/remove/request.go @@ -46,7 +46,7 @@ func (r *Request) Validate() error { } for i, token := range r.Tokens { if token == "" { - return fmt.Errorf("token at position %d is empty after trimming", i) + return fmt.Errorf("token at position %d is empty", i) } } return nil diff --git a/internal/remove/summary.go b/internal/remove/summary.go index 1122d80..5452a49 100644 --- a/internal/remove/summary.go +++ b/internal/remove/summary.go @@ -2,112 +2,98 @@ package remove import "sort" -// Summary aggregates results across preview/apply phases. +// ConflictDetail describes a rename that cannot proceed. +type ConflictDetail struct { + OriginalPath string + ProposedPath string + Reason string +} + +// Summary aggregates preview/apply metrics for reporting and ledger metadata. type Summary struct { - totalCandidates int - changedCount int - conflicts []Conflict - empties []string - tokenMatches map[string]int - duplicates []string + TotalCandidates int + ChangedCount int + TokenMatches map[string]int + Conflicts []ConflictDetail + Empties []string + Duplicates []string } -// Conflict describes a rename conflict detected during planning. -type Conflict struct { - Original string - Proposed string - Reason string -} - -// NewSummary constructs a ready-to-use Summary. +// NewSummary constructs an initialized summary instance. func NewSummary() Summary { return Summary{ - tokenMatches: make(map[string]int), - conflicts: make([]Conflict, 0), - empties: make([]string, 0), - duplicates: make([]string, 0), + TokenMatches: make(map[string]int), } } -// RecordCandidate increments the total candidate count. -func (s *Summary) RecordCandidate() { - s.totalCandidates++ +// RecordCandidate updates aggregate counts based on a candidate result. +func (s *Summary) RecordCandidate(res Result) { + s.TotalCandidates++ + if !res.Changed { + return + } + s.ChangedCount++ + for token, count := range res.Matches { + s.TokenMatches[token] += count + } } -// RecordChange increments changed items. -func (s *Summary) RecordChange() { - s.changedCount++ +// AddConflict registers a conflict for reporting. +func (s *Summary) AddConflict(conflict ConflictDetail) { + s.Conflicts = append(s.Conflicts, conflict) } -// AddTokenMatch records the number of matches for a token. -func (s *Summary) AddTokenMatch(token string, count int) { - s.tokenMatches[token] += count -} - -// AddConflict registers a detected conflict. -func (s *Summary) AddConflict(c Conflict) { - s.conflicts = append(s.conflicts, c) -} - -// AddEmpty registers a path skipped due to empty result names. +// AddEmpty records a path whose resulting name would be empty. func (s *Summary) AddEmpty(path string) { - s.empties = append(s.empties, path) + s.Empties = append(s.Empties, path) } -// AddDuplicate tracks duplicate tokens encountered during parsing. +// AddDuplicate stores duplicate tokens captured during parsing. func (s *Summary) AddDuplicate(token string) { - s.duplicates = append(s.duplicates, token) + if token == "" { + return + } + s.Duplicates = append(s.Duplicates, token) } -// TotalCandidates returns how many items were considered. -func (s Summary) TotalCandidates() int { - return s.totalCandidates +// SortedDuplicates returns unique duplicate tokens sorted for deterministic output. +func (s *Summary) SortedDuplicates() []string { + if len(s.Duplicates) == 0 { + return nil + } + seen := make(map[string]struct{}, len(s.Duplicates)) + result := make([]string, 0, len(s.Duplicates)) + for _, dup := range s.Duplicates { + if _, ok := seen[dup]; ok { + continue + } + seen[dup] = struct{}{} + result = append(result, dup) + } + sort.Strings(result) + return result } -// ChangedCount returns the number of items whose names changed. -func (s Summary) ChangedCount() int { - return s.changedCount -} - -// Conflicts returns a copy of conflict info. -func (s Summary) Conflicts() []Conflict { - out := make([]Conflict, len(s.conflicts)) - copy(out, s.conflicts) - return out -} - -// Empties returns paths skipped for empty basename results. -func (s Summary) Empties() []string { - out := make([]string, len(s.empties)) - copy(out, s.empties) - return out -} - -// TokenMatches returns a sorted slice of tokens and counts. -func (s Summary) TokenMatches() []struct { +// SortedTokenMatches returns token match counts sorted alphabetically by token. +func (s *Summary) SortedTokenMatches() []struct { Token string Count int } { - pairs := make([]struct { + if len(s.TokenMatches) == 0 { + return nil + } + result := make([]struct { Token string Count int - }, 0, len(s.tokenMatches)) - for token, count := range s.tokenMatches { - pairs = append(pairs, struct { + }, 0, len(s.TokenMatches)) + for token, count := range s.TokenMatches { + result = append(result, struct { Token string Count int }{Token: token, Count: count}) } - sort.Slice(pairs, func(i, j int) bool { - return pairs[i].Token < pairs[j].Token + sort.Slice(result, func(i, j int) bool { + return result[i].Token < result[j].Token }) - return pairs -} - -// Duplicates returns duplicates flagged by the parser. -func (s Summary) Duplicates() []string { - out := make([]string, len(s.duplicates)) - copy(out, s.duplicates) - sort.Strings(out) - return out + return result } diff --git a/internal/remove/traversal.go b/internal/remove/traversal.go index c4dbc7d..795ef42 100644 --- a/internal/remove/traversal.go +++ b/internal/remove/traversal.go @@ -55,6 +55,10 @@ func Traverse(ctx context.Context, req *Request, fn func(Candidate) error) error } } + if isDir && !req.IncludeDirectories { + return nil + } + candidate := Candidate{ RelativePath: filepath.ToSlash(relPath), OriginalPath: filepath.Join(req.WorkingDir, relPath), diff --git a/scripts/smoke-test-remove.sh b/scripts/smoke-test-remove.sh index a15d7a2..ed24e26 100755 --- a/scripts/smoke-test-remove.sh +++ b/scripts/smoke-test-remove.sh @@ -11,7 +11,19 @@ touch "$TMP_DIR/report copy draft.txt" touch "$TMP_DIR/nested/notes draft.txt" echo "Previewing removals..." -$BIN "$ROOT_DIR/main.go" remove " copy" " draft" --path "$TMP_DIR" --recursive --dry-run >/dev/null +preview_output="$($BIN "$ROOT_DIR/main.go" remove " copy" " draft" --path "$TMP_DIR" --recursive --dry-run)" + +if [[ "$preview_output" != *"report copy draft.txt -> report.txt"* ]]; then + echo "expected preview to show mapping for report copy draft.txt" >&2 + echo "$preview_output" >&2 + exit 1 +fi + +if [[ "$preview_output" != *"Preview complete."* ]]; then + echo "expected preview completion message" >&2 + echo "$preview_output" >&2 + exit 1 +fi if [[ ! -f "$TMP_DIR/report copy draft.txt" ]]; then echo "preview should not modify files" >&2 diff --git a/specs/003-add-remove-command/tasks.md b/specs/003-add-remove-command/tasks.md index efa17db..88cd2bf 100644 --- a/specs/003-add-remove-command/tasks.md +++ b/specs/003-add-remove-command/tasks.md @@ -35,16 +35,16 @@ ### Tests for User Story 1 ⚠️ -- [ ] T007 [P] [US1] Add unit tests covering sequential token application and unchanged cases in `tests/unit/remove_engine_test.go`. -- [ ] T008 [P] [US1] Create contract test validating preview table output and dry-run messaging in `tests/contract/remove_command_preview_test.go`. -- [ ] T009 [P] [US1] Write integration test exercising preview → apply flow with multiple files in `tests/integration/remove_flow_test.go`. +- [X] T007 [P] [US1] Add unit tests covering sequential token application and unchanged cases in `tests/unit/remove_engine_test.go`. +- [X] T008 [P] [US1] Create contract test validating preview table output and dry-run messaging in `tests/contract/remove_command_preview_test.go`. +- [X] T009 [P] [US1] Write integration test exercising preview → apply flow with multiple files in `tests/integration/remove_flow_test.go`. ### Implementation for User Story 1 -- [ ] T010 [US1] Implement sequential removal engine producing planned operations in `internal/remove/engine.go`. -- [ ] T011 [US1] Build preview pipeline that aggregates summaries, detects conflicts, and streams output in `internal/remove/preview.go`. -- [ ] T012 [US1] Implement apply pipeline executing planned operations without ledger writes in `internal/remove/apply.go`. -- [ ] T013 [US1] Wire new Cobra command in `cmd/remove.go` (with registration in `cmd/root.go`) to drive preview/apply using shared scope flags. +- [X] T010 [US1] Implement sequential removal engine producing planned operations in `internal/remove/engine.go`. +- [X] T011 [US1] Build preview pipeline that aggregates summaries, detects conflicts, and streams output in `internal/remove/preview.go`. +- [X] T012 [US1] Implement apply pipeline executing planned operations without ledger writes in `internal/remove/apply.go`. +- [X] T013 [US1] Wire new Cobra command in `cmd/remove.go` (with registration in `cmd/root.go`) to drive preview/apply using shared scope flags. **Checkpoint**: User Story 1 functional end-to-end with preview/apply validated by automated tests. @@ -58,13 +58,13 @@ ### Tests for User Story 2 ⚠️ -- [ ] T014 [P] [US2] Add contract test asserting ledger entries capture ordered tokens and match counts in `tests/contract/remove_command_ledger_test.go`. -- [ ] T015 [P] [US2] Add integration test covering `--yes` automation path and subsequent undo in `tests/integration/remove_undo_test.go`. +- [X] T014 [P] [US2] Add contract test asserting ledger entries capture ordered tokens and match counts in `tests/contract/remove_command_ledger_test.go`. +- [X] T015 [P] [US2] Add integration test covering `--yes` automation path and subsequent undo in `tests/integration/remove_undo_test.go`. ### Implementation for User Story 2 -- [ ] T016 [US2] Extend apply pipeline to append ledger entries with ordered tokens and match counts in `internal/remove/apply.go`. -- [ ] T017 [US2] Update `cmd/remove.go` to support non-interactive `--yes` execution, emit automation-oriented messages, and propagate exit codes. +- [X] T016 [US2] Extend apply pipeline to append ledger entries with ordered tokens and match counts in `internal/remove/apply.go`. +- [X] T017 [US2] Update `cmd/remove.go` to support non-interactive `--yes` execution, emit automation-oriented messages, and propagate exit codes. **Checkpoint**: User Story 2 complete—CLI safe for scripting with ledger + undo parity. @@ -78,14 +78,14 @@ ### Tests for User Story 3 ⚠️ -- [ ] T018 [P] [US3] Add parser validation tests for duplicate tokens and whitespace edge cases in `tests/unit/remove_parser_test.go`. -- [ ] T019 [P] [US3] Add integration test verifying empty-basename warnings and skips in `tests/integration/remove_validation_test.go`. +- [X] T018 [P] [US3] Add parser validation tests for duplicate tokens and whitespace edge cases in `tests/unit/remove_parser_test.go`. +- [X] T019 [P] [US3] Add integration test verifying empty-basename warnings and skips in `tests/integration/remove_validation_test.go`. ### Implementation for User Story 3 -- [ ] T020 [US3] Implement duplicate token deduplication with ordered warning collection in `internal/remove/parser.go`. -- [ ] T021 [US3] Add empty-basename detection and warning tracking in `internal/remove/summary.go`. -- [ ] T022 [US3] Surface duplicate and empty warnings in CLI output handling within `cmd/remove.go`. +- [X] T020 [US3] Implement duplicate token deduplication with ordered warning collection in `internal/remove/parser.go`. +- [X] T021 [US3] Add empty-basename detection and warning tracking in `internal/remove/summary.go`. +- [X] T022 [US3] Surface duplicate and empty warnings in CLI output handling within `cmd/remove.go`. **Checkpoint**: All user stories deliver value; validations prevent risky rename plans. @@ -95,10 +95,10 @@ **Purpose**: Documentation, tooling, and quality improvements spanning all user stories. -- [ ] T023 [P] Update remove command documentation and sequential behavior guidance in `docs/cli-flags.md`. -- [ ] T024 Record release notes for remove command launch in `docs/CHANGELOG.md`. -- [ ] T025 [P] Finalize `scripts/smoke-test-remove.sh` with assertions and integrate into CI instructions. -- [ ] T026 Add remove command walkthrough to project onboarding materials in `AGENTS.md`. +- [X] T023 [P] Update remove command documentation and sequential behavior guidance in `docs/cli-flags.md`. +- [X] T024 Record release notes for remove command launch in `docs/CHANGELOG.md`. +- [X] T025 [P] Finalize `scripts/smoke-test-remove.sh` with assertions and integrate into CI instructions. +- [X] T026 Add remove command walkthrough to project onboarding materials in `AGENTS.md`. --- diff --git a/testdata/remove/basic/Project copy.txt b/testdata/remove/basic/Project copy.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/README.md b/testdata/remove/basic/README.md new file mode 100644 index 0000000..185d805 --- /dev/null +++ b/testdata/remove/basic/README.md @@ -0,0 +1,32 @@ +# Remove Command Scenario Fixtures + +Use this directory to capture realistic file layouts for the `renamer remove` tests. + +## Layout + +``` +testdata/remove/basic +├── project copy draft.txt +├── Project copy.txt +├── nested +│ ├── foo draft.txt +│ ├── foo draft draft.txt +│ └── hidden +│ └── .draft copy.md +├── collisions +│ ├── alpha draft.txt +│ └── alpha.txt +└── empty-basename + ├── draft + └── draft.txt +``` + +## Scenarios Covered + +- Sequential removals (`" copy"`, `" draft"`) collapsing multiple terms. +- Duplicate token handling in nested directories. +- Hidden file interactions requiring the `--hidden` flag. +- Name collisions after removal (should report conflict prior to apply). +- Entries that would become empty names, verifying they are skipped with warnings. + +Adjust or extend files as additional edge cases are added to integration tests. diff --git a/testdata/remove/basic/collisions/alpha draft.txt b/testdata/remove/basic/collisions/alpha draft.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/collisions/alpha.txt b/testdata/remove/basic/collisions/alpha.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/empty-basename/draft b/testdata/remove/basic/empty-basename/draft new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/empty-basename/draft.txt b/testdata/remove/basic/empty-basename/draft.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/nested/foo draft draft.txt b/testdata/remove/basic/nested/foo draft draft.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/nested/foo draft.txt b/testdata/remove/basic/nested/foo draft.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/nested/hidden/.draft copy.md b/testdata/remove/basic/nested/hidden/.draft copy.md new file mode 100644 index 0000000..e69de29 diff --git a/testdata/remove/basic/project copy draft.txt b/testdata/remove/basic/project copy draft.txt new file mode 100644 index 0000000..e69de29 diff --git a/tests/contract/remove_command_ledger_test.go b/tests/contract/remove_command_ledger_test.go new file mode 100644 index 0000000..2ee0863 --- /dev/null +++ b/tests/contract/remove_command_ledger_test.go @@ -0,0 +1,104 @@ +package contract + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" + + renamercmd "github.com/rogeecn/renamer/cmd" + "github.com/rogeecn/renamer/internal/history" +) + +func TestRemoveCommandLedgerMetadata(t *testing.T) { + tmp := t.TempDir() + + createRemoveFile(t, filepath.Join(tmp, "report copy draft.txt")) + createRemoveFile(t, filepath.Join(tmp, "notes draft.txt")) + + root := renamercmd.NewRootCommand() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"remove", " copy", " draft", "--path", tmp, "--yes"}) + + if err := root.Execute(); err != nil { + t.Fatalf("remove command error: %v\noutput: %s", err, out.String()) + } + + ledgerPath := filepath.Join(tmp, ".renamer") + data, err := os.ReadFile(ledgerPath) + if err != nil { + t.Fatalf("read ledger: %v", err) + } + + lines := bytes.Split(bytes.TrimSpace(data), []byte("\n")) + if len(lines) == 0 { + t.Fatalf("expected ledger entry written") + } + + var entry history.Entry + if err := json.Unmarshal(lines[len(lines)-1], &entry); err != nil { + t.Fatalf("decode ledger entry: %v", err) + } + + if entry.Command != "remove" { + t.Fatalf("expected remove command recorded, got %q", entry.Command) + } + + tokensVal, ok := entry.Metadata["tokens"].([]any) + if !ok { + t.Fatalf("expected tokens metadata, got %#v", entry.Metadata) + } + tokens := make([]string, len(tokensVal)) + for i, v := range tokensVal { + s, ok := v.(string) + if !ok { + t.Fatalf("token entry not string: %#v", v) + } + tokens[i] = s + } + if len(tokens) != 2 || tokens[0] != " copy" || tokens[1] != " draft" { + t.Fatalf("unexpected tokens metadata: %#v", tokens) + } + + matchesVal, ok := entry.Metadata["matches"].(map[string]any) + if !ok { + t.Fatalf("expected matches metadata, got %#v", entry.Metadata) + } + if len(matchesVal) != 2 { + t.Fatalf("unexpected matches metadata: %#v", matchesVal) + } + if toFloat(matchesVal[" copy"]) != 1 || toFloat(matchesVal[" draft"]) != 2 { + t.Fatalf("unexpected match counts: %#v", matchesVal) + } + + // Ensure undo restores originals for automation workflows. + undo := renamercmd.NewRootCommand() + undo.SetOut(&bytes.Buffer{}) + undo.SetErr(&bytes.Buffer{}) + undo.SetArgs([]string{"undo", "--path", tmp}) + if err := undo.Execute(); err != nil { + t.Fatalf("undo command error: %v", err) + } + + if _, err := os.Stat(filepath.Join(tmp, "report copy draft.txt")); err != nil { + t.Fatalf("expected original restored after undo: %v", err) + } +} + +func toFloat(value any) float64 { + switch v := value.(type) { + case float64: + return v + case float32: + return float64(v) + case int: + return float64(v) + case int64: + return float64(v) + default: + return -1 + } +} diff --git a/tests/contract/remove_command_preview_test.go b/tests/contract/remove_command_preview_test.go new file mode 100644 index 0000000..b62c6ea --- /dev/null +++ b/tests/contract/remove_command_preview_test.go @@ -0,0 +1,46 @@ +package contract + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + renamercmd "github.com/rogeecn/renamer/cmd" +) + +func TestRemoveCommandDryRunPreview(t *testing.T) { + tmp := t.TempDir() + createRemoveFile(t, filepath.Join(tmp, "report copy draft.txt")) + createRemoveFile(t, filepath.Join(tmp, "notes draft.txt")) + + root := renamercmd.NewRootCommand() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"remove", " copy", " draft", "--path", tmp, "--dry-run"}) + + if err := root.Execute(); err != nil { + t.Fatalf("remove command returned error: %v (output: %s)", err, out.String()) + } + + output := out.String() + if !strings.Contains(output, "report copy draft.txt -> report.txt") { + t.Fatalf("expected preview mapping in output, got: %s", output) + } + + if !strings.Contains(output, "Preview complete") { + t.Fatalf("expected dry-run completion message, got: %s", output) + } +} + +func createRemoveFile(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", path, err) + } + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatalf("write file %s: %v", path, err) + } +} diff --git a/tests/integration/remove_flow_test.go b/tests/integration/remove_flow_test.go new file mode 100644 index 0000000..c2cdd0c --- /dev/null +++ b/tests/integration/remove_flow_test.go @@ -0,0 +1,86 @@ +package integration + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/rogeecn/renamer/internal/history" + "github.com/rogeecn/renamer/internal/remove" +) + +func TestRemoveApplyAndUndo(t *testing.T) { + tmp := t.TempDir() + + createFile(t, filepath.Join(tmp, "report copy draft.txt")) + createFile(t, filepath.Join(tmp, "nested", "notes draft.txt")) + + parsed, err := remove.ParseArgs([]string{" copy", " draft"}) + if err != nil { + t.Fatalf("ParseArgs error: %v", err) + } + + req := &remove.Request{ + WorkingDir: tmp, + Tokens: parsed.Tokens, + Recursive: true, + } + if err := req.Validate(); err != nil { + t.Fatalf("request validation error: %v", err) + } + + summary, planned, err := remove.Preview(context.Background(), req, parsed, nil) + if err != nil { + t.Fatalf("Preview error: %v", err) + } + + if summary.ChangedCount != 2 { + t.Fatalf("expected 2 changes, got %d", summary.ChangedCount) + } + + entry, err := remove.Apply(context.Background(), req, planned, summary, parsed.Tokens) + if err != nil { + t.Fatalf("Apply error: %v", err) + } + + if len(entry.Operations) != 2 { + t.Fatalf("expected 2 operations recorded, got %d", len(entry.Operations)) + } + if entry.Metadata == nil { + t.Fatalf("expected metadata to be recorded") + } + tokens, ok := entry.Metadata["tokens"].([]string) + if !ok { + t.Fatalf("expected ordered tokens metadata, got %#v", entry.Metadata) + } + if len(tokens) != 2 || tokens[0] != " copy" || tokens[1] != " draft" { + t.Fatalf("unexpected tokens metadata: %#v", tokens) + } + matches, ok := entry.Metadata["matches"].(map[string]int) + if !ok { + t.Fatalf("expected matches metadata, got %#v", entry.Metadata) + } + if matches[" copy"] != 1 || matches[" draft"] != 2 { + t.Fatalf("unexpected match counts: %#v", matches) + } + + if _, err := os.Stat(filepath.Join(tmp, "report.txt")); err != nil { + t.Fatalf("expected renamed file exists: %v", err) + } + + if _, err := os.Stat(filepath.Join(tmp, "nested", "notes.txt")); err != nil { + t.Fatalf("expected nested rename exists: %v", err) + } + + if _, err := history.Undo(tmp); err != nil { + t.Fatalf("undo error: %v", err) + } + + if _, err := os.Stat(filepath.Join(tmp, "report copy draft.txt")); err != nil { + t.Fatalf("expected original restored: %v", err) + } + if _, err := os.Stat(filepath.Join(tmp, "nested", "notes draft.txt")); err != nil { + t.Fatalf("expected nested original restored: %v", err) + } +} diff --git a/tests/integration/remove_undo_test.go b/tests/integration/remove_undo_test.go new file mode 100644 index 0000000..46d6d7d --- /dev/null +++ b/tests/integration/remove_undo_test.go @@ -0,0 +1,57 @@ +package integration + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + renamercmd "github.com/rogeecn/renamer/cmd" +) + +func TestRemoveCommandAutomationUndo(t *testing.T) { + tmp := t.TempDir() + + createFile(t, filepath.Join(tmp, "alpha copy.txt")) + createFile(t, filepath.Join(tmp, "nested", "beta draft.txt")) + + preview := renamercmd.NewRootCommand() + var previewOut bytes.Buffer + preview.SetOut(&previewOut) + preview.SetErr(&previewOut) + preview.SetArgs([]string{"remove", " copy", " draft", "--path", tmp, "--recursive", "--dry-run"}) + if err := preview.Execute(); err != nil { + t.Fatalf("preview failed: %v\noutput: %s", err, previewOut.String()) + } + + apply := renamercmd.NewRootCommand() + var applyOut bytes.Buffer + apply.SetOut(&applyOut) + apply.SetErr(&applyOut) + apply.SetArgs([]string{"remove", " copy", " draft", "--path", tmp, "--recursive", "--yes"}) + if err := apply.Execute(); err != nil { + t.Fatalf("apply failed: %v\noutput: %s", err, applyOut.String()) + } + + if !fileExists(filepath.Join(tmp, "alpha.txt")) || !fileExists(filepath.Join(tmp, "nested", "beta.txt")) { + t.Fatalf("expected files renamed after apply") + } + + undo := renamercmd.NewRootCommand() + var undoOut bytes.Buffer + undo.SetOut(&undoOut) + undo.SetErr(&undoOut) + undo.SetArgs([]string{"undo", "--path", tmp}) + if err := undo.Execute(); err != nil { + t.Fatalf("undo failed: %v\noutput: %s", err, undoOut.String()) + } + + if !fileExists(filepath.Join(tmp, "alpha copy.txt")) || !fileExists(filepath.Join(tmp, "nested", "beta draft.txt")) { + t.Fatalf("expected originals restored after undo") + } +} + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/tests/integration/remove_validation_test.go b/tests/integration/remove_validation_test.go new file mode 100644 index 0000000..3803df2 --- /dev/null +++ b/tests/integration/remove_validation_test.go @@ -0,0 +1,62 @@ +package integration + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + renamercmd "github.com/rogeecn/renamer/cmd" +) + +func TestRemoveCommandEmptyBasenameWarning(t *testing.T) { + tmp := t.TempDir() + + createValidationFile(t, filepath.Join(tmp, "draft")) + createValidationFile(t, filepath.Join(tmp, "draft copy.txt")) + + root := renamercmd.NewRootCommand() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"remove", "draft", "--path", tmp, "--dry-run"}) + + if err := root.Execute(); err != nil { + t.Fatalf("remove dry-run failed: %v\noutput: %s", err, out.String()) + } + + if !strings.Contains(out.String(), "Warning: draft would become empty; skipping") { + t.Fatalf("expected empty basename warning, got: %s", out.String()) + } +} + +func TestRemoveCommandDuplicateWarning(t *testing.T) { + tmp := t.TempDir() + + createValidationFile(t, filepath.Join(tmp, "foo draft draft.txt")) + + root := renamercmd.NewRootCommand() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs([]string{"remove", " draft", " draft", "--path", tmp, "--dry-run"}) + + if err := root.Execute(); err != nil { + t.Fatalf("remove dry-run failed: %v\noutput: %s", err, out.String()) + } + + if !strings.Contains(out.String(), "Warning: token \" draft\" provided multiple times") { + t.Fatalf("expected duplicate warning, got: %s", out.String()) + } +} + +func createValidationFile(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", path, err) + } + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatalf("write file %s: %v", path, err) + } +} diff --git a/tests/unit/remove_engine_test.go b/tests/unit/remove_engine_test.go new file mode 100644 index 0000000..b68a79c --- /dev/null +++ b/tests/unit/remove_engine_test.go @@ -0,0 +1,74 @@ +package replace_test + +import ( + "testing" + + "github.com/rogeecn/renamer/internal/remove" +) + +func TestApplyTokensSequentialRemoval(t *testing.T) { + candidate := remove.Candidate{ + BaseName: "report copy draft.txt", + RelativePath: "report copy draft.txt", + } + + result := remove.ApplyTokens(candidate, []string{" copy", " draft"}) + + if !result.Changed { + t.Fatalf("expected result to be marked as changed") + } + + if result.ProposedName != "report.txt" { + t.Fatalf("expected proposed name to be report.txt, got %q", result.ProposedName) + } + + if result.Matches[" copy"] != 1 { + t.Fatalf("expected match count for ' copy' to be 1, got %d", result.Matches[" copy"]) + } + + if result.Matches[" draft"] != 1 { + t.Fatalf("expected match count for ' draft' to be 1, got %d", result.Matches[" draft"]) + } +} + +func TestApplyTokensNoChange(t *testing.T) { + candidate := remove.Candidate{ + BaseName: "notes.txt", + RelativePath: "notes.txt", + } + + result := remove.ApplyTokens(candidate, []string{" copy"}) + + if result.Changed { + t.Fatalf("expected no change for candidate without matches") + } + + if len(result.Matches) != 0 { + t.Fatalf("expected no matches to be recorded, got %#v", result.Matches) + } + + if result.ProposedName != candidate.BaseName { + t.Fatalf("expected proposed name to remain %q, got %q", candidate.BaseName, result.ProposedName) + } +} + +func TestApplyTokensEmptyName(t *testing.T) { + candidate := remove.Candidate{ + BaseName: "draft", + RelativePath: "draft", + } + + result := remove.ApplyTokens(candidate, []string{"draft"}) + + if !result.Changed { + t.Fatalf("expected change when removing the full name") + } + + if result.ProposedName != "" { + t.Fatalf("expected proposed name to be empty, got %q", result.ProposedName) + } + + if result.Matches["draft"] != 1 { + t.Fatalf("expected matches to record removal, got %#v", result.Matches) + } +} diff --git a/tests/unit/remove_parser_test.go b/tests/unit/remove_parser_test.go new file mode 100644 index 0000000..fa67673 --- /dev/null +++ b/tests/unit/remove_parser_test.go @@ -0,0 +1,46 @@ +package replace_test + +import ( + "testing" + + "github.com/rogeecn/renamer/internal/remove" +) + +func TestParseArgsDeduplicatesPreservingOrder(t *testing.T) { + args := []string{" draft", " draft", " copy", " draft"} + + result, err := remove.ParseArgs(args) + if err != nil { + t.Fatalf("ParseArgs returned error: %v", err) + } + + expected := []string{" draft", " copy"} + if len(result.Tokens) != len(expected) { + t.Fatalf("expected %d tokens, got %d", len(expected), len(result.Tokens)) + } + for i, token := range expected { + if result.Tokens[i] != token { + t.Fatalf("token[%d] mismatch: expected %q, got %q", i, token, result.Tokens[i]) + } + } + + if len(result.Duplicates) != 2 { + t.Fatalf("expected 2 duplicates recorded, got %d", len(result.Duplicates)) + } + if result.Duplicates[0] != " draft" || result.Duplicates[1] != " draft" { + t.Fatalf("unexpected duplicates: %#v", result.Duplicates) + } +} + +func TestParseArgsSkipsWhitespaceOnlyTokens(t *testing.T) { + args := []string{" ", "\t", "foo"} + + result, err := remove.ParseArgs(args) + if err != nil { + t.Fatalf("ParseArgs returned error: %v", err) + } + + if len(result.Tokens) != 1 || result.Tokens[0] != "foo" { + t.Fatalf("expected single token 'foo', got %#v", result.Tokens) + } +}