Feature 103/fzf native support #120

noe.trevino · 2026-02-21T23:24:54-06:00

noe.trevino commented

2026-02-21 23:24:54 -06:00

Checklist

Read CONTRIBUTING.md
Ran tests (go test ./... -race) locally
Formatted and linted with gofmt -w . and golangci-lint run ./... respectively
Added tests if necessary (significant changes/complex logic)
Updated documentation if applicable
Updated README.md if applicable

Summary

closes: #103

## Checklist - [x] Read `CONTRIBUTING.md` - [x] Ran tests (`go test ./... -race`) locally - [x] Formatted and linted with `gofmt -w .` and `golangci-lint run ./...` respectively - [x] Added tests if necessary (significant changes/complex logic) - [x] Updated documentation if applicable - [x] Updated `README.md` if applicable ## Summary  closes: #103

github-actions[bot] commented

2026-02-21 23:25:03 -06:00

(Migrated from github.com)

Informational Warning:

The snapshot tests have been changed.
Please add a screenshot showing what has been changed, and a description of why it was changed.

If you do not provide an explanation and screenshot in 3 days, this PR will be closed.

@TheNoeTrevino

Changed snapshot files (10 files)

internal/testing/snapshots/cli/testdata/help/paso-assignee-pick.golden
internal/testing/snapshots/cli/testdata/help/paso-assignee.golden
internal/testing/snapshots/cli/testdata/help/paso-column-pick.golden
internal/testing/snapshots/cli/testdata/help/paso-column.golden
internal/testing/snapshots/cli/testdata/help/paso-label-pick.golden
internal/testing/snapshots/cli/testdata/help/paso-label.golden
internal/testing/snapshots/cli/testdata/help/paso-project-pick.golden
internal/testing/snapshots/cli/testdata/help/paso-project.golden
internal/testing/snapshots/cli/testdata/help/paso-task-pick.golden
internal/testing/snapshots/cli/testdata/help/paso-task.golden

**Informational Warning:** The snapshot tests have been changed. Please add a screenshot showing what has been changed, and a description of why it was changed. If you do not provide an explanation and screenshot in 3 days, this PR will be closed. @TheNoeTrevino <details> <summary>Changed snapshot files (10 files)</summary> - `internal/testing/snapshots/cli/testdata/help/paso-assignee-pick.golden` - `internal/testing/snapshots/cli/testdata/help/paso-assignee.golden` - `internal/testing/snapshots/cli/testdata/help/paso-column-pick.golden` - `internal/testing/snapshots/cli/testdata/help/paso-column.golden` - `internal/testing/snapshots/cli/testdata/help/paso-label-pick.golden` - `internal/testing/snapshots/cli/testdata/help/paso-label.golden` - `internal/testing/snapshots/cli/testdata/help/paso-project-pick.golden` - `internal/testing/snapshots/cli/testdata/help/paso-project.golden` - `internal/testing/snapshots/cli/testdata/help/paso-task-pick.golden` - `internal/testing/snapshots/cli/testdata/help/paso-task.golden` </details>

noe.trevino commented

2026-02-21 23:26:32 -06:00

pick addition is the same for the other entities. diff is actually readable, no ansi

<img width="1236" height="784" alt="image" src="https://github.com/user-attachments/assets/d0579266-7b41-420f-ac53-7716fd051d97" /> pick addition is the same for the other entities. diff is actually readable, no ansi

claude[bot] (Migrated from github.com) reviewed

2026-02-21 23:28:56 -06:00

internal/cli/assignee/pick.go

					
				@ -0,0 +30,4 @@

					return cmd

				}

				func runPick(cmd *cobra.Command, args []string) error {

Clear DRY violation: runPick is copy-pasted nearly verbatim across all five pick.go files (assignee, column, label, project, task). The boilerplate — CLI init, defer close, empty-slice check, cli.RunPick call, fmt.Println(selected) — is identical in every file. Only the service call and title string differ.

The project-scoped variant (column, label, task) additionally duplicates the project-ID resolution block. A shared helper — e.g. a generic runPickWith[T cli.Pickable] function — would eliminate this repetition and give a single place to fix any future changes to the pattern.

Clear DRY violation: `runPick` is copy-pasted nearly verbatim across all five `pick.go` files (`assignee`, `column`, `label`, `project`, `task`). The boilerplate — CLI init, defer close, empty-slice check, `cli.RunPick` call, `fmt.Println(selected)` — is identical in every file. Only the service call and title string differ. The project-scoped variant (column, label, task) additionally duplicates the project-ID resolution block. A shared helper — e.g. a generic `runPickWith[T cli.Pickable]` function — would eliminate this repetition and give a single place to fix any future changes to the pattern.

claude[bot] (Migrated from github.com) reviewed

2026-02-21 23:29:02 -06:00

internal/cli/assignee/pick.go

					
				@ -0,0 +1,66 @@

				package assignee

cli.RunPick can return errors other than user cancellation (e.g., terminal/rendering failures from the underlying Bubble Tea runtime). Mapping every error to "PICK_CANCELLED" / "Selection cancelled" masks those failures, making them impossible to distinguish from a user pressing Escape.

At minimum, huh.ErrUserAborted should be checked explicitly and handled separately from unexpected errors:

if errors.Is(err, huh.ErrUserAborted) {
    return formatter.Error(cli.ExitUsage, "PICK_CANCELLED", "Selection cancelled")
}
return formatter.Error(cli.ExitError, "PICK_ERROR", err.Error())

The same pattern applies to the other four pick.go files.

`cli.RunPick` can return errors other than user cancellation (e.g., terminal/rendering failures from the underlying Bubble Tea runtime). Mapping every error to `"PICK_CANCELLED"` / `"Selection cancelled"` masks those failures, making them impossible to distinguish from a user pressing Escape. At minimum, `huh.ErrUserAborted` should be checked explicitly and handled separately from unexpected errors: ```go if errors.Is(err, huh.ErrUserAborted) { return formatter.Error(cli.ExitUsage, "PICK_CANCELLED", "Selection cancelled") } return formatter.Error(cli.ExitError, "PICK_ERROR", err.Error()) ``` The same pattern applies to the other four `pick.go` files.

claude[bot] commented

2026-02-21 23:29:36 -06:00

(Migrated from github.com)

Review summary

Two substantive issues found.

DRY violation (assignee, column, label, project, task pick.go files): The runPick function body is duplicated five times. The only variation across files is the service call, the picker title, and (for project-scoped entities) the project-ID resolution block. All other logic — CLI init, defer close, empty-slice guard, RunPick invocation, fmt.Println — is identical. This should be factored into a shared helper.

Error masking (all five runPick functions): Every error returned by cli.RunPick is reported as "PICK_CANCELLED" / "Selection cancelled", including errors that have nothing to do with the user cancelling. Terminal or rendering errors would produce a misleading message and the wrong exit code classification.

Score: 7/10

## Review summary Two substantive issues found. **DRY violation** (assignee, column, label, project, task pick.go files): The runPick function body is duplicated five times. The only variation across files is the service call, the picker title, and (for project-scoped entities) the project-ID resolution block. All other logic — CLI init, defer close, empty-slice guard, RunPick invocation, fmt.Println — is identical. This should be factored into a shared helper. **Error masking** (all five runPick functions): Every error returned by cli.RunPick is reported as "PICK_CANCELLED" / "Selection cancelled", including errors that have nothing to do with the user cancelling. Terminal or rendering errors would produce a misleading message and the wrong exit code classification. **Score: 7/10**

noe.trevino reviewed

2026-02-21 23:44:54 -06:00

internal/cli/assignee/pick.go

					
				@ -0,0 +30,4 @@

					return cmd

				}

				func runPick(cmd *cobra.Command, args []string) error {

skipping this

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

noe.trevino/paso!120

				`@ -0,0 +1,66 @@`
				`package assignee`

Rows
Columns