noe.trevino/paso
1
0
Fork 0
Code Issues 17 Pull requests 1 Projects 1 Releases Packages Wiki Activity Actions

Feature 98/move search filterbar #102

Merged
noe.trevino merged 6 commits from feature-98/move-search-filterbar into main 2026-02-18 00:50:21 -06:00
Conversation 7 Commits 6 Files changed 28 +344 -266
noe.trevino commented 2026-02-16 23:36:47 -06:00
Owner
Copy link

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 #98

## 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 <!-- What does this PR change and/or fix? Examples: Closes: 54 Fixes: 67 --> Closes #98
github-actions[bot] commented 2026-02-16 23:36:57 -06:00 (Migrated from github.com)
Copy link

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 (11 files)
  • internal/testing/snapshots/cli/testdata/e2e/task-show.golden
  • internal/testing/snapshots/tui/testdata/board_with_labels.golden
  • internal/testing/snapshots/tui/testdata/board_with_multiple_tasks.golden
  • internal/testing/snapshots/tui/testdata/board_with_tasks.golden
  • internal/testing/snapshots/tui/testdata/connection_disconnected.golden
  • internal/testing/snapshots/tui/testdata/connection_reconnecting.golden
  • internal/testing/snapshots/tui/testdata/empty_project_board.golden
  • internal/testing/snapshots/tui/testdata/no_projects.golden
  • internal/testing/snapshots/tui/testdata/notification_error.golden
  • internal/testing/snapshots/tui/testdata/notification_warning.golden
  • internal/testing/snapshots/tui/testdata/project_no_columns.golden
<!-- snapshot-warning --> **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 (11 files)</summary> - `internal/testing/snapshots/cli/testdata/e2e/task-show.golden` - `internal/testing/snapshots/tui/testdata/board_with_labels.golden` - `internal/testing/snapshots/tui/testdata/board_with_multiple_tasks.golden` - `internal/testing/snapshots/tui/testdata/board_with_tasks.golden` - `internal/testing/snapshots/tui/testdata/connection_disconnected.golden` - `internal/testing/snapshots/tui/testdata/connection_reconnecting.golden` - `internal/testing/snapshots/tui/testdata/empty_project_board.golden` - `internal/testing/snapshots/tui/testdata/no_projects.golden` - `internal/testing/snapshots/tui/testdata/notification_error.golden` - `internal/testing/snapshots/tui/testdata/notification_warning.golden` - `internal/testing/snapshots/tui/testdata/project_no_columns.golden` </details>
👍 1
noe.trevino commented 2026-02-16 23:38:03 -06:00
Author
Owner
Copy link
2026-02-16-230121_hyprshot
<img width="3810" height="2043" alt="2026-02-16-230121_hyprshot" src="https://github.com/user-attachments/assets/632880e9-cf0f-492f-9ab2-e489174adf16" />
claude[bot] (Migrated from github.com) reviewed 2026-02-16 23:40:51 -06:00
internal/tui/state/filter_state.go
claude[bot] (Migrated from github.com) commented 2026-02-16 23:40:51 -06:00
Copy link

SearchBackspace slices by bytes, not runes:

f.SearchQuery = f.SearchQuery[:len(f.SearchQuery)-1]

For a query ending in a multi-byte UTF-8 character (e.g. any non-ASCII input) this will produce a malformed string. Use []rune round-tripping:

	runes := []rune(f.SearchQuery)
	f.SearchQuery = string(runes[:len(runes)-1])
`SearchBackspace` slices by bytes, not runes: ```go f.SearchQuery = f.SearchQuery[:len(f.SearchQuery)-1] ``` For a query ending in a multi-byte UTF-8 character (e.g. any non-ASCII input) this will produce a malformed string. Use `[]rune` round-tripping: ```suggestion runes := []rune(f.SearchQuery) f.SearchQuery = string(runes[:len(runes)-1]) ```
claude[bot] (Migrated from github.com) reviewed 2026-02-16 23:40:55 -06:00
internal/tui/state/filter_state.go
claude[bot] (Migrated from github.com) commented 2026-02-16 23:40:55 -06:00
Copy link

AppendSearchChar enforces the cap with len(f.SearchQuery) which counts bytes, not characters. A query of 50 two-byte runes already hits the 100-byte limit while only being 50 characters long. Use utf8.RuneCountInString (or len([]rune(f.SearchQuery))) for a consistent character limit.

`AppendSearchChar` enforces the cap with `len(f.SearchQuery)` which counts bytes, not characters. A query of 50 two-byte runes already hits the 100-byte limit while only being 50 characters long. Use `utf8.RuneCountInString` (or `len([]rune(f.SearchQuery))`) for a consistent character limit.
claude[bot] (Migrated from github.com) reviewed 2026-02-16 23:41:11 -06:00
internal/tui/update_normal.go
@ -129,0 +132,4 @@
m.UI.Filter.FocusedChip = state.FilterChipSearch
m.UI.Filter.SearchInputActive = true
m.UI.Filter.SearchQuery = ""
m.UIState.Mode = state.FilterBarMode
claude[bot] (Migrated from github.com) commented 2026-02-16 23:41:11 -06:00
Copy link

SearchQuery is unconditionally cleared here. Since search now persists across project switches, pressing / to re-open search wipes the active query instead of letting the user edit it. Either pre-populate the input with the existing query, or only clear it if there is no active query.

`SearchQuery` is unconditionally cleared here. Since search now persists across project switches, pressing `/` to re-open search wipes the active query instead of letting the user edit it. Either pre-populate the input with the existing query, or only clear it if there is no active query.
claude[bot] (Migrated from github.com) reviewed 2026-02-16 23:41:15 -06:00
internal/tui/view_board.go
claude[bot] (Migrated from github.com) commented 2026-02-16 23:41:15 -06:00
Copy link

SearchQuery and SearchInputActive are already accessible through props.Filter (the *state.FilterState pointer passed on the line above). Duplicating them as separate FilterBarProps fields creates two sources of truth for the same state. RenderFilterBar should read them directly from props.Filter instead.

`SearchQuery` and `SearchInputActive` are already accessible through `props.Filter` (the `*state.FilterState` pointer passed on the line above). Duplicating them as separate `FilterBarProps` fields creates two sources of truth for the same state. `RenderFilterBar` should read them directly from `props.Filter` instead.
claude[bot] commented 2026-02-16 23:41:27 -06:00 (Migrated from github.com)
Copy link

Review Summary

The refactor is coherent and the consolidation of SearchState into FilterState is the right direction. Four concrete issues found:

1. Byte-level backspace (correctness bug)
SearchBackspace truncates via f.SearchQuery[:len(f.SearchQuery)-1]. For any non-ASCII character (multi-byte UTF-8 rune) this produces a malformed string or panics. Must use rune-level slicing. See inline comment on filter_state.go.

2. Byte-based max-length check (correctness bug)
AppendSearchChar guards with len(f.SearchQuery) >= maxSearchQueryLength which counts bytes. The effective character limit is inconsistent for non-ASCII input (50 two-byte characters = 100 bytes = limit hit). Use utf8.RuneCountInString. See inline comment on filter_state.go.

3. Pressing / discards an active search query (behaviour regression)
handleEnterSearchChip unconditionally sets SearchQuery = "". Search now persists across project switches by design, but pressing / to refine an existing search destroys it. The query should either be preserved for editing, or only cleared when there is no active query. See inline comment on update_normal.go.

4. Duplicate state fields in FilterBarProps / view_board.go (design issue)
SearchQuery and SearchInputActive are set redundantly in buildFilterBarProps even though the same values are already accessible via props.Filter (the *state.FilterState pointer). This creates two sources of truth for the same state and unnecessary indirection. See inline comment on view_board.go.

Score: 6/10

## Review Summary The refactor is coherent and the consolidation of `SearchState` into `FilterState` is the right direction. Four concrete issues found: **1. Byte-level backspace (correctness bug)** `SearchBackspace` truncates via `f.SearchQuery[:len(f.SearchQuery)-1]`. For any non-ASCII character (multi-byte UTF-8 rune) this produces a malformed string or panics. Must use rune-level slicing. See inline comment on `filter_state.go`. **2. Byte-based max-length check (correctness bug)** `AppendSearchChar` guards with `len(f.SearchQuery) >= maxSearchQueryLength` which counts bytes. The effective character limit is inconsistent for non-ASCII input (50 two-byte characters = 100 bytes = limit hit). Use `utf8.RuneCountInString`. See inline comment on `filter_state.go`. **3. Pressing `/` discards an active search query (behaviour regression)** `handleEnterSearchChip` unconditionally sets `SearchQuery = ""`. Search now persists across project switches by design, but pressing `/` to refine an existing search destroys it. The query should either be preserved for editing, or only cleared when there is no active query. See inline comment on `update_normal.go`. **4. Duplicate state fields in `FilterBarProps` / `view_board.go` (design issue)** `SearchQuery` and `SearchInputActive` are set redundantly in `buildFilterBarProps` even though the same values are already accessible via `props.Filter` (the `*state.FilterState` pointer). This creates two sources of truth for the same state and unnecessary indirection. See inline comment on `view_board.go`. **Score: 6/10**
😕 1
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something isn't working

  
ci
Pipelines, runners, CI/CD, any devops stuff

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
feature
New feature request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
performance
Improvement or issue

  
priority
P0
 Priority, carried over from the GitHub project board

  
priority
P1
 Priority, carried over from the GitHub project board

  
priority
P2
 Priority, carried over from the GitHub project board

  
question
Further information is requested

  
refactor
Changing the architecture of the codebase

  
size
L
 Size, carried over from the GitHub project board

  
size
M
 Size, carried over from the GitHub project board

  
size
S
 Size, carried over from the GitHub project board

  
size
XL
 Size, carried over from the GitHub project board

  
size
XS
 Size, carried over from the GitHub project board

  
tests

   
trivial
This issue is not a priority at the moment

  
wontfix
This will not be worked on

No labels
bug
ci
documentation
duplicate
enhancement
feature
good first issue
help wanted
invalid
performance
priority
P0
priority
P1
priority
P2
question
refactor
size
L
size
M
size
S
size
XL
size
XS
tests
trivial
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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!102
No description provided.