fix(tui): Fixed a bug that caused tui to be stuck in filepath picker box#395
Merged
junaid2005p merged 1 commit intomainfrom Apr 20, 2026
Merged
fix(tui): Fixed a bug that caused tui to be stuck in filepath picker box#395junaid2005p merged 1 commit intomainfrom
junaid2005p merged 1 commit intomainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+92
to
98
| default: | ||
| m.inputs[2].SetValue(path) | ||
| m.focusInput(2) | ||
| m.filepickerOrigin = FilePickerOriginNone | ||
| m.state = InputState | ||
| return m, nil | ||
| } |
Contributor
There was a problem hiding this comment.
FilePickerOriginAdd silently handled by default
FilePickerOriginAdd is defined in the enum but never explicitly cased in this switch (or in the cancel switch in update_filepicker.go). It currently falls through to default, which is correct, but it means adding a new origin in the future could accidentally inherit the "go to InputState" behaviour without any compile-time signal. An explicit case makes the intent clear and makes the switch exhaustive for all declared origins.
Suggested change
| default: | |
| m.inputs[2].SetValue(path) | |
| m.focusInput(2) | |
| m.filepickerOrigin = FilePickerOriginNone | |
| m.state = InputState | |
| return m, nil | |
| } | |
| case FilePickerOriginAdd: | |
| m.inputs[2].SetValue(path) | |
| m.focusInput(2) | |
| m.filepickerOrigin = FilePickerOriginNone | |
| m.state = InputState | |
| return m, nil | |
| default: | |
| m.inputs[2].SetValue(path) | |
| m.focusInput(2) | |
| m.filepickerOrigin = FilePickerOriginNone | |
| m.state = InputState | |
| return m, nil | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/tui/helpers.go
Line: 92-98
Comment:
**`FilePickerOriginAdd` silently handled by `default`**
`FilePickerOriginAdd` is defined in the enum but never explicitly cased in this switch (or in the cancel switch in `update_filepicker.go`). It currently falls through to `default`, which is correct, but it means adding a new origin in the future could accidentally inherit the "go to InputState" behaviour without any compile-time signal. An explicit case makes the intent clear and makes the switch exhaustive for all declared origins.
```suggestion
case FilePickerOriginAdd:
m.inputs[2].SetValue(path)
m.focusInput(2)
m.filepickerOrigin = FilePickerOriginNone
m.state = InputState
return m, nil
default:
m.inputs[2].SetValue(path)
m.focusInput(2)
m.filepickerOrigin = FilePickerOriginNone
m.state = InputState
return m, nil
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+956
to
+990
| updated, _ = m2.Update(tea.KeyPressMsg{Code: tea.KeyEscape}) | ||
| m3 := unwrapRootModel(t, updated) | ||
|
|
||
| if got, want := m3.state, InputState; got != want { | ||
| t.Fatalf("state after esc = %v, want %v", got, want) | ||
| } | ||
| if got := m3.filepickerOrigin; got != FilePickerOriginNone { | ||
| t.Fatalf("filepickerOrigin after esc = %v, want none", got) | ||
| } | ||
| if got, want := m3.focusedInput, 2; got != want { | ||
| t.Fatalf("focusedInput after esc = %d, want %d", got, want) | ||
| } | ||
| if got, want := m3.inputs[2].Value(), browseDir; got != want { | ||
| t.Fatalf("path input after esc = %q, want %q", got, want) | ||
| } | ||
| } | ||
|
|
||
| func TestUpdate_FilePickerUseDirReturnsToAddInput(t *testing.T) { | ||
| browseDir := t.TempDir() | ||
|
|
||
| m := RootModel{ | ||
| state: FilePickerState, | ||
| focusedInput: 2, | ||
| inputs: newInputModels(), | ||
| keys: Keys, | ||
| Settings: config.DefaultSettings(), | ||
| filepicker: newFilepicker(browseDir), | ||
| filepickerOrigin: FilePickerOriginAdd, | ||
| } | ||
| m.filepicker.CurrentDirectory = browseDir | ||
|
|
||
| updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEnter}) | ||
| m2 := unwrapRootModel(t, updated) | ||
|
|
||
| if got, want := m2.state, InputState; got != want { |
Contributor
There was a problem hiding this comment.
Enter, not the UseDir (.) key
The UseDir key binding maps to . (period) and explicitly selects the current directory via handleFilePickerSelection. Enter is passed to the underlying filepicker component and relies on DidSelectFile returning true — a different code path. Naming the test UseDirReturnsToAddInput while sending KeyEnter is misleading; consider renaming it to TestUpdate_FilePickerEnterConfirmsDirReturnsToAddInput or testing the actual . key press to match the name.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/tui/update_test.go
Line: 956-990
Comment:
**Test name says "UseDir" but tests `Enter`, not the `UseDir` (`.`) key**
The `UseDir` key binding maps to `.` (period) and explicitly selects the current directory via `handleFilePickerSelection`. `Enter` is passed to the underlying filepicker component and relies on `DidSelectFile` returning true — a different code path. Naming the test `UseDirReturnsToAddInput` while sending `KeyEnter` is misleading; consider renaming it to `TestUpdate_FilePickerEnterConfirmsDirReturnsToAddInput` or testing the actual `.` key press to match the name.
How can I resolve this? If you propose a fix, please make it concise.
SuperCoolPencil
approved these changes
Apr 20, 2026
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
2 participants
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.
Greptile Summary
This PR replaces three independent boolean browsing flags (
SettingsFileBrowsing,ExtensionFileBrowsing,catMgrFileBrowsing) with a singleFilePickerOriginenum, adds a centralisedopenDirectoryPickerhelper, and ensuresfocusInput(2)is called on picker return — fixing the root cause of the "stuck in file picker" bug. New tests cover open, cancel, confirm, and root-directory edge cases.Confidence Score: 5/5
Safe to merge; all remaining findings are minor style suggestions with no correctness impact.
The bug fix is well-targeted: the enum refactor eliminates the flag-management scatter, and adding focusInput(2) on picker dismissal directly addresses the stuck-input regression. Both open code paths and cancel paths are handled symmetrically. The only findings are a P2 implicit default case and a P2 test naming inconsistency — neither affects runtime behaviour.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[InputState / SettingsState / ExtensionState / CategoryState] -->|Tab / Browse key| B[openDirectoryPicker\nsets filepickerOrigin + state=FilePickerState] B --> C{FilePickerState} C -->|UseDir / Enter selects path| D[handleFilePickerSelection] C -->|Escape / Cancel| E[updateFilePicker cancel switch] D --> F{filepickerOrigin} E --> F F -->|OriginSettings| G[SettingsState\nset DefaultDownloadDir] F -->|OriginExtension| H[ExtensionConfirmationState\nset inputs2 + focusInput 2] F -->|OriginCategory| I[CategoryManagerState\nset catMgrInputs3 + focus] F -->|OriginAdd / default| J[InputState\nset inputs2 + focusInput 2]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(tui): Fixed a bug that caused tui to..." | Re-trigger Greptile