fix: plain output formatting across all command modules#19
Open
itsjeremyjohnson wants to merge 2 commits intomainfrom
Open
fix: plain output formatting across all command modules#19itsjeremyjohnson wants to merge 2 commits intomainfrom
itsjeremyjohnson wants to merge 2 commits intomainfrom
Conversation
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+116
to
119
| if !cmd.Yes { | ||
| fmt.Fprintf(os.Stderr, "Warning: This will permanently delete comment %s\n", cmd.CommentID) | ||
| fmt.Fprint(os.Stderr, "Use --force to confirm deletion\n") | ||
| return nil |
There was a problem hiding this comment.
CommentsDeleteCmd returns nil (exit 0) when the user hasn't supplied --yes, which is indistinguishable from a successful deletion. Every other delete command introduced in this PR — ChecklistsDeleteCmd, GoalsDeleteCmd, TagsDeleteCmd, TimeDeleteCmd, ViewsDeleteCmd, WebhooksDeleteCmd — returns fmt.Errorf("operation cancelled: use --force to confirm") (exit 1). The same return nil pattern also appears in internal/cmd/chat.go:324 (ChatDeleteChannelCmd), chat.go:454 (ChatDeleteMessageCmd), and internal/cmd/users.go:151 (UsersRemoveCmd).
Scripts that check exit codes to detect skipped deletions will silently miss the cancellation for these four commands.
Suggested change
| if !cmd.Yes { | |
| fmt.Fprintf(os.Stderr, "Warning: This will permanently delete comment %s\n", cmd.CommentID) | |
| fmt.Fprint(os.Stderr, "Use --force to confirm deletion\n") | |
| return nil | |
| if !cmd.Yes { | |
| fmt.Fprintf(os.Stderr, "Warning: This will permanently delete comment %s\n", cmd.CommentID) | |
| fmt.Fprint(os.Stderr, "Use --yes to confirm deletion\n") | |
| return fmt.Errorf("operation cancelled: use --yes to confirm") | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/cmd/comments.go
Line: 116-119
Comment:
**Inconsistent exit code when confirmation is skipped**
`CommentsDeleteCmd` returns `nil` (exit 0) when the user hasn't supplied `--yes`, which is indistinguishable from a successful deletion. Every other delete command introduced in this PR — `ChecklistsDeleteCmd`, `GoalsDeleteCmd`, `TagsDeleteCmd`, `TimeDeleteCmd`, `ViewsDeleteCmd`, `WebhooksDeleteCmd` — returns `fmt.Errorf("operation cancelled: use --force to confirm")` (exit 1). The same `return nil` pattern also appears in `internal/cmd/chat.go:324` (`ChatDeleteChannelCmd`), `chat.go:454` (`ChatDeleteMessageCmd`), and `internal/cmd/users.go:151` (`UsersRemoveCmd`).
Scripts that check exit codes to detect skipped deletions will silently miss the cancellation for these four commands.
```suggestion
if !cmd.Yes {
fmt.Fprintf(os.Stderr, "Warning: This will permanently delete comment %s\n", cmd.CommentID)
fmt.Fprint(os.Stderr, "Use --yes to confirm deletion\n")
return fmt.Errorf("operation cancelled: use --yes to confirm")
}
```
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standardizes plain output formatting across all ClickUp CLI command modules and consolidates progress tracking.
Greptile Summary
This PR standardizes
--plainoutput formatting across all 12 ClickUp CLI command modules by addingoutfmt.IsPlain(ctx)/outfmt.WritePlain(os.Stdout, headers, rows)branches to everyRun()method. The plain-output additions are well-structured and follow the project's stdout/stderr discipline from AGENTS.md.However, two consistent defects are introduced:
--forcevs--yesflag mismatch in all delete/remove warning messages — Every destructive command defines a flag named--yes(short-y) but prints "Use --force to confirm deletion." Any user or script following the on-screen prompt will receive a"flag provided but not defined: --force"error.ChatDeleteChannelCmd,ChatDeleteMessageCmd,CommentsDeleteCmd, andUsersRemoveCmdreturnnil(exit 0) when the user omits--yes, while every other delete command correctly returns a non-zero error. This makes scripted usage unreliable.Confidence Score: 2/5
Not safe to merge — every delete/remove command displays an incorrect --force flag name; users following the on-screen prompt will receive a runtime flag error.
The plain-output additions are correct and well-structured, matching the project's AGENTS.md patterns. However, the --force vs --yes mismatch is a widespread usability defect that affects every destructive operation in the CLI and will block real users from confirming deletions. The additional inconsistency in exit codes (nil vs error) further complicates reliable scripted use.
All files containing Delete or Remove commands require warning message corrections: chat.go, checklists.go, comments.go, folders.go, goals.go, lists.go, spaces.go, tags.go, time.go, users.go, views.go, webhooks.go
Important Files Changed
--forceflag name mismatch in both delete warning messages;ChatDeleteChannelCmdandChatDeleteMessageCmdreturn nil (exit 0) on cancellation--forcemismatch in delete warning;CommentsDeleteCmdreturns nil (exit 0) on cancellation instead of an error--forcemismatch in remove warning;UsersRemoveCmdreturns nil (exit 0) on cancellation instead of an error--forceflag name mismatch in both delete warning messages--forceflag name mismatch in both delete warning messages--forceflag name mismatch inTimeDeleteCmdwarning--forceflag name mismatch inViewsDeleteCmdwarning--forceflag name mismatch inWebhooksDeleteCmdwarning--forceflag name mismatch inFoldersDeleteCmdwarning--forceflag name mismatch inListsDeleteCmdwarning--forceflag name mismatch inSpacesDeleteCmdwarning--forceflag name mismatch inTagsDeleteCmdwarningSequence Diagram
sequenceDiagram participant User as User (Terminal) participant Main as main.go participant Execute as cmd.Execute() participant Run as Cmd.Run(ctx) participant OutFmt as outfmt participant Stdout as stdout participant Stderr as stderr User->>Main: clickup <command> [--json|--plain] Main->>Execute: Execute(os.Args[1:]) Execute->>Execute: Parse flags, build ctx with output mode Execute->>Run: Run(ctx) Run->>Run: Call ClickUp API alt --json flag Run->>OutFmt: IsJSON(ctx) → true Run->>OutFmt: WriteJSON(stdout, result) OutFmt->>Stdout: JSON payload else --plain flag Run->>OutFmt: IsPlain(ctx) → true Run->>OutFmt: WritePlain(stdout, headers, rows) OutFmt->>Stdout: Tab-separated rows else default (human-readable) Run->>Stderr: Status / progress messages Run->>Stdout: Formatted data end Run-->>Execute: nil / error Execute-->>Main: nil / error Main->>User: exit 0 / exit 1Comments Outside Diff (1)
internal/cmd/chat.go, line 319-324 (link)--forceflag referenced but--yesis the actual flagThe warning tells users
"Use --force to confirm deletion."but the struct field isYes boolwithname:"yes" short:"y". Runningclickup chat delete-channel <id> --forcewill produce a "flag provided but not defined: --force" error at runtime. Users following the on-screen instruction will never be able to complete the deletion.The same mismatch exists in every other delete/remove command across the PR:
internal/cmd/chat.go:452-453(ChatDeleteMessageCmd)internal/cmd/checklists.go:108-109(ChecklistsDeleteCmd) andchecklists.go:256-257(ChecklistsDeleteItemCmd)internal/cmd/comments.go:117-118(CommentsDeleteCmd)internal/cmd/folders.go:142-143(FoldersDeleteCmd)internal/cmd/goals.go:337-338(GoalsDeleteCmd) andgoals.go:499-500(GoalsDeleteKeyResultCmd)internal/cmd/lists.go:305-307(ListsDeleteCmd)internal/cmd/spaces.go:242-243(SpacesDeleteCmd)internal/cmd/tags.go:204-205(TagsDeleteCmd)internal/cmd/time.go:472-473(TimeDeleteCmd)internal/cmd/users.go:148-149(UsersRemoveCmd)internal/cmd/views.go:355-356(ViewsDeleteCmd)internal/cmd/webhooks.go:181-182(WebhooksDeleteCmd)All warning messages should reference
--yesto match the defined flag:Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore: update build prompt and consolida..." | Re-trigger Greptile