Skip to content

Commit f8def41

Browse files
authored
Comment flags on all show commands + bug fixes (#394)
* Fix renderer comments suppression causing data loss renderObject unconditionally skipped the "comments" key, then topLevelComments tried to reconstruct it as []map[string]any. When comments was a different shape (string, int, []string) — as happens with `basecamp api get` — reconstruction returned nil and data was silently dropped. Only skip "comments" in the field loop when topLevelComments returns non-nil. Both styled and markdown renderers. * Fix false-positive attachment regression tests TestStyledRenderObjectPreservesUnknownAttachmentFields asserted custom_attachments_report survived — but that key doesn't end with _attachments, so it passed under the old code too. previewable_attachments was in the test data but never asserted. Assert previewable_attachments values appear in output (both styled and markdown). Remove the misleading assertion. * Redesign commentFlags for three-flag model Three mutually exclusive flags: --comments / --no-comments / --all-comments. addCommentFlags(cmd, defaultOn) controls whether comments are fetched by default (show) or require opt-in (typed show commands). New fetchCommentsForRecording derives count from API response metadata, not the parent's comments_count. fetchRecordingComments becomes a thin wrapper that falls back to the parent's count when Meta.TotalCount is unavailable. * Always fetch comments in show; gate non-commentable types Drop the comments_count gate — basecamp show now always attempts comment fetching for commentable types. Skip the fetch for people and boosts which don't support comments. Update tests: expect the extra comments request, add X-Total-Count header support to mock transport, rename TestShowCommentsMissingField to TestShowSkipsCommentsForNonCommentableTypes. * Add --comments / --no-comments / --all-comments to typed show commands Wire fetchCommentsForRecording into all commentable show commands: todos, messages, cards, files, todolists, schedule (both paths), checkins (question + answer), forwards, chat line. defaultOn=false means typed commands require --comments or --all-comments to opt in. Attachment notices are merged via applyNotices. Comments on comments are excluded (flat model). * Update .surface and SKILL.md for comment flags Regenerate CLI surface snapshot with new --comments, --no-comments, --all-comments flags on typed show commands. Acknowledge comments show flag removal in .surface-breaking. Update skill with typed show command examples. * Fix empty comments array leaking as field; rename tests Use isCommentsArray to decide whether to suppress the raw comments field, not len(comments) > 0. topLevelComments returns nil for empty arrays (toMapSlice on []any{}), so empty comments arrays were rendered as regular fields instead of being suppressed. Rename attachment tests to reflect updated fixture focus. * Exclude comment type from automatic comment fetch Add comment/comments CLI aliases and Comment API type to isCommentableShowType's non-commentable list. Comments are flat — attempting to fetch comments-on-comments wastes a request and can produce misleading diagnostics. * Wire comment flags into checkins parent shortcut commands Register --comments/--no-comments/--all-comments on the parent question and answer commands so both entry points (shortcut and explicit show) accept the same flags. * Regenerate .surface; remove stale .surface-breaking entries Fresh surface snapshot reflects comment type exclusion and checkins parent flag wiring. Remove the comments show flag removal entries from .surface-breaking — the flags were never in the regenerated baseline. * Extract commentEnrichment.apply helper; collapse enrichment tail Every typed show command repeated the same withComments / applyNotices / breadcrumbs sequence. The new apply() method on commentEnrichment returns enriched data and composed opts in one call. All 11 call sites now use it. Includes tests for apply() (nil, populated, breadcrumb composition) and isCommentableShowType (CLI aliases, generic lookups, API types). * Render comments after entity presenter in styled and markdown output The schema-driven presenter only renders fields declared in YAML schemas, so comments injected via withComments were silently dropped in styled and markdown output. Extract comments from NormalizeData(resp.Data) — never from DisplayData — and append a comments section after the presenter body. Includes tests for entity+comments rendering (plain entity and DisplayData split path, both formats) and isCommentsArray coverage. * Address review feedback: rename test, add truncation notice fallback Rename TestShowNoCommentsWhenCountZero to TestShowFetchesCommentsEvenWhenCountZero to match the actual assertion (comments ARE fetched even with count 0). When Meta.TotalCount is missing from the API response but the parent's comments_count indicates truncation occurred, recompute the truncation notice so the user sees a warning and the --all-comments breadcrumb.
1 parent 87432e3 commit f8def41

19 files changed

Lines changed: 672 additions & 125 deletions

.surface

Lines changed: 85 additions & 0 deletions
Large diffs are not rendered by default.

.surface-breaking

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,3 +761,4 @@ FLAG basecamp chat messages --campfire type=string
761761
FLAG basecamp chat post --campfire type=string
762762
FLAG basecamp chat show --campfire type=string
763763
FLAG basecamp chat upload --campfire type=string
764+

internal/commands/cards.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ You can pass either a card ID or a Basecamp URL:
267267
}
268268

269269
dlDir := addDownloadAttachmentsFlag(cmd)
270+
cf := addCommentFlags(cmd, false)
270271

271272
cmd.RunE = func(cmd *cobra.Command, args []string) error {
272273
app := appctx.FromContext(cmd.Context())
@@ -288,6 +289,8 @@ You can pass either a card ID or a Basecamp URL:
288289
return convertSDKError(err)
289290
}
290291

292+
enrichment := fetchCommentsForRecording(cmd.Context(), app, cardIDStr, cf)
293+
291294
opts := []output.ResponseOption{
292295
output.WithSummary(fmt.Sprintf("Card #%s: %s", cardIDStr, card.Title)),
293296
output.WithBreadcrumbs(
@@ -300,6 +303,7 @@ You can pass either a card ID or a Basecamp URL:
300303
}
301304

302305
data := any(card)
306+
attachmentNotice := ""
303307
contentAtts := downloadableAttachments(richtext.ParseAttachments(card.Content))
304308
descAtts := downloadableAttachments(richtext.ParseAttachments(card.Description))
305309
total := len(contentAtts) + len(descAtts)
@@ -317,17 +321,19 @@ You can pass either a card ID or a Basecamp URL:
317321
if len(descAtts) > 0 {
318322
data = withAttachmentMeta(data, "description", descAtts, descDL)
319323
}
320-
notice := fmt.Sprintf("%d attachment(s) — download: basecamp attachments download %s",
324+
attachmentNotice = fmt.Sprintf("%d attachment(s) — download: basecamp attachments download %s",
321325
total, cardIDStr)
322326
if dl != nil && dl.Notice != "" {
323-
notice += "; " + dl.Notice
327+
attachmentNotice += "; " + dl.Notice
324328
}
325329
opts = append(opts,
326-
output.WithNotice(notice),
327330
output.WithBreadcrumbs(attachmentBreadcrumb(cardIDStr, total)),
328331
)
329332
}
330333

334+
data, extraOpts := enrichment.apply(data, attachmentNotice)
335+
opts = append(opts, extraOpts...)
336+
331337
return app.OK(data, opts...)
332338
}
333339

internal/commands/chat.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,8 @@ func runChatUpload(cmd *cobra.Command, app *appctx.App, chatID, project, filePat
627627
}
628628

629629
func newChatLineShowCmd(project, chatID *string) *cobra.Command {
630+
var cf *commentFlags
631+
630632
cmd := &cobra.Command{
631633
Use: "line <id|url>",
632634
Aliases: []string{"show"},
@@ -697,9 +699,13 @@ You can pass either a line ID or a Basecamp line URL:
697699
if line.Creator != nil {
698700
creatorName = line.Creator.Name
699701
}
702+
703+
enrichment := fetchCommentsForRecording(cmd.Context(), app, lineID, cf)
704+
data, commentOpts := enrichment.apply(line, "")
700705
summary := fmt.Sprintf("Line #%s by %s", lineID, creatorName)
701706

702-
return app.OK(line,
707+
opts := make([]output.ResponseOption, 0, 4+len(commentOpts))
708+
opts = append(opts,
703709
output.WithSummary(summary),
704710
output.WithEntity("chat_line"),
705711
output.WithDisplayData(chatLineDisplayData(line)),
@@ -716,8 +722,14 @@ You can pass either a line ID or a Basecamp line URL:
716722
},
717723
),
718724
)
725+
opts = append(opts, commentOpts...)
726+
727+
return app.OK(data, opts...)
719728
},
720729
}
730+
731+
cf = addCommentFlags(cmd, false)
732+
721733
return cmd
722734
}
723735

internal/commands/checkins.go

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,16 @@ func newCheckinsQuestionCmd(project *string) *cobra.Command {
153153
Use: "question <id|url>",
154154
Short: "Show or manage a question",
155155
Args: cobra.MaximumNArgs(1),
156-
RunE: func(cmd *cobra.Command, args []string) error {
157-
// Show help when invoked with no arguments
158-
if len(args) == 0 {
159-
return missingArg(cmd, "<id|url>")
160-
}
161-
return runCheckinsQuestionShow(cmd, *project, args[0])
162-
},
156+
}
157+
158+
cf := addCommentFlags(cmd, false)
159+
160+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
161+
// Show help when invoked with no arguments
162+
if len(args) == 0 {
163+
return missingArg(cmd, "<id|url>")
164+
}
165+
return runCheckinsQuestionShow(cmd, *project, args[0], cf)
163166
}
164167

165168
cmd.AddCommand(
@@ -172,7 +175,7 @@ func newCheckinsQuestionCmd(project *string) *cobra.Command {
172175
}
173176

174177
func newCheckinsQuestionShowCmd(project *string) *cobra.Command {
175-
return &cobra.Command{
178+
cmd := &cobra.Command{
176179
Use: "show <id|url>",
177180
Short: "Show question details",
178181
Long: `Display details of a check-in question.
@@ -181,13 +184,18 @@ You can pass either a question ID or a Basecamp URL:
181184
basecamp checkins question show 789 --in my-project
182185
basecamp checkins question show https://3.basecamp.com/123/buckets/456/questionnaires/questions/789`,
183186
Args: cobra.ExactArgs(1),
184-
RunE: func(cmd *cobra.Command, args []string) error {
185-
return runCheckinsQuestionShow(cmd, *project, args[0])
186-
},
187187
}
188+
189+
cf := addCommentFlags(cmd, false)
190+
191+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
192+
return runCheckinsQuestionShow(cmd, *project, args[0], cf)
193+
}
194+
195+
return cmd
188196
}
189197

190-
func runCheckinsQuestionShow(cmd *cobra.Command, project, questionIDStr string) error {
198+
func runCheckinsQuestionShow(cmd *cobra.Command, project, questionIDStr string, cf *commentFlags) error {
191199
app := appctx.FromContext(cmd.Context())
192200

193201
if err := ensureAccount(cmd, app); err != nil {
@@ -230,9 +238,12 @@ func runCheckinsQuestionShow(cmd *cobra.Command, project, questionIDStr string)
230238
return convertSDKError(err)
231239
}
232240

241+
enrichment := fetchCommentsForRecording(cmd.Context(), app, questionIDStr, cf)
242+
data, commentOpts := enrichment.apply(question, "")
233243
summary := fmt.Sprintf("%s (%d answers)", question.Title, question.AnswersCount)
234244

235-
return app.OK(question,
245+
opts := make([]output.ResponseOption, 0, 2+len(commentOpts))
246+
opts = append(opts,
236247
output.WithSummary(summary),
237248
output.WithBreadcrumbs(
238249
output.Breadcrumb{
@@ -247,6 +258,9 @@ func runCheckinsQuestionShow(cmd *cobra.Command, project, questionIDStr string)
247258
},
248259
),
249260
)
261+
opts = append(opts, commentOpts...)
262+
263+
return app.OK(data, opts...)
250264
}
251265

252266
func newCheckinsQuestionCreateCmd(project *string) *cobra.Command {
@@ -620,13 +634,16 @@ func newCheckinsAnswerCmd(project *string) *cobra.Command {
620634
Use: "answer <id|url>",
621635
Short: "Show or manage an answer",
622636
Args: cobra.MaximumNArgs(1),
623-
RunE: func(cmd *cobra.Command, args []string) error {
624-
// Show help when invoked with no arguments
625-
if len(args) == 0 {
626-
return missingArg(cmd, "<id|url>")
627-
}
628-
return runCheckinsAnswerShow(cmd, *project, args[0])
629-
},
637+
}
638+
639+
cf := addCommentFlags(cmd, false)
640+
641+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
642+
// Show help when invoked with no arguments
643+
if len(args) == 0 {
644+
return missingArg(cmd, "<id|url>")
645+
}
646+
return runCheckinsAnswerShow(cmd, *project, args[0], cf)
630647
}
631648

632649
cmd.AddCommand(
@@ -639,7 +656,7 @@ func newCheckinsAnswerCmd(project *string) *cobra.Command {
639656
}
640657

641658
func newCheckinsAnswerShowCmd(project *string) *cobra.Command {
642-
return &cobra.Command{
659+
cmd := &cobra.Command{
643660
Use: "show <id|url>",
644661
Short: "Show answer details",
645662
Long: `Display details of a check-in answer.
@@ -648,13 +665,18 @@ You can pass either an answer ID or a Basecamp URL:
648665
basecamp checkins answer show 789 --in my-project
649666
basecamp checkins answer show https://3.basecamp.com/123/buckets/456/question_answers/789`,
650667
Args: cobra.ExactArgs(1),
651-
RunE: func(cmd *cobra.Command, args []string) error {
652-
return runCheckinsAnswerShow(cmd, *project, args[0])
653-
},
654668
}
669+
670+
cf := addCommentFlags(cmd, false)
671+
672+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
673+
return runCheckinsAnswerShow(cmd, *project, args[0], cf)
674+
}
675+
676+
return cmd
655677
}
656678

657-
func runCheckinsAnswerShow(cmd *cobra.Command, project, answerIDStr string) error {
679+
func runCheckinsAnswerShow(cmd *cobra.Command, project, answerIDStr string, cf *commentFlags) error {
658680
app := appctx.FromContext(cmd.Context())
659681

660682
if err := ensureAccount(cmd, app); err != nil {
@@ -705,14 +727,18 @@ func runCheckinsAnswerShow(cmd *cobra.Command, project, answerIDStr string) erro
705727
if len(date) > 10 {
706728
date = date[:10]
707729
}
730+
731+
enrichment := fetchCommentsForRecording(cmd.Context(), app, answerIDStr, cf)
732+
data, commentOpts := enrichment.apply(answer, "")
708733
summary := fmt.Sprintf("Answer by %s on %s", author, date)
709734

710735
questionID := ""
711736
if answer.Parent != nil {
712737
questionID = strconv.FormatInt(answer.Parent.ID, 10)
713738
}
714739

715-
return app.OK(answer,
740+
opts := make([]output.ResponseOption, 0, 2+len(commentOpts))
741+
opts = append(opts,
716742
output.WithSummary(summary),
717743
output.WithBreadcrumbs(
718744
output.Breadcrumb{
@@ -727,6 +753,9 @@ func runCheckinsAnswerShow(cmd *cobra.Command, project, answerIDStr string) erro
727753
},
728754
),
729755
)
756+
opts = append(opts, commentOpts...)
757+
758+
return app.OK(data, opts...)
730759
}
731760

732761
func newCheckinsAnswerCreateCmd(project *string) *cobra.Command {

0 commit comments

Comments
 (0)