Fix fundamental design issues: circular references, untestable prompts, and implicit init()#122
Merged
Merged
Conversation
Restore() and Remove() had identical inline closures to find the storage backend for a given file. Extract into a shared helper method. Also removes verbose debug logging that was only in Restore but not Remove, making both paths consistent.
File held a reference to its managing Storage, creating a circular dependency (Storage creates Files, Files reference Storage). This field was never consumed — Manager.findStorageForFile() uses TrashPath prefix matching instead. Remove the storage field, SetStorage(), and GetStorage() methods.
Define a Prompter interface (Confirm, ConfirmYes, InputFilename) in the CLI package. restore.go and prune.go now call c.prompter instead of ui package functions directly. The default uiPrompter delegates to the existing ui.Confirm/ui.ConfirmYes/ui.InputFilename functions. This enables mock-based testing of CLI commands that require user confirmation without starting a real BubbleTea program.
The env package used init() to set GOMI_LOG_PATH and CLICOLOR_FORCE as side effects on import. Replace with an explicit Init() call from CLI.Run(), protected by sync.Once. Also remove the unused GOMI_CONFIG_PATH variable and defaultXDGConfigDirname constant.
The history package used init() to auto-rename inventory.json to history.json on import, causing filesystem writes as a side effect. Replace with migrateIfNeeded() called explicitly from New(). Also replace log.Fatalf with slog.Error to avoid process termination on migration failure.
Contributor
Code Metrics Report
Details | | main (d7e0503) | #122 (9d67d02) | +/- |
|---------------------|----------------|----------------|-------|
- | Coverage | 44.2% | 42.1% | -2.1% |
| Files | 24 | 24 | 0 |
| Lines | 1822 | 1839 | +17 |
- | Covered | 806 | 776 | -30 |
+ | Code to Test Ratio | 1:1.5 | 1:1.5 | +0.0 |
| Code | 2772 | 2720 | -52 |
- | Test | 4248 | 4238 | -10 |
+ | Test Execution Time | 4s | 3s | -1s |Code coverage of files in pull request scope (39.2% → 35.2%)
Reported by octocov |
babarot
added
the
kind/cleanup
Merged
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Jun 10, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [babarot/gomi](https://github.com/babarot/gomi) | patch | `v1.6.3` → `v1.6.4` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>babarot/gomi (babarot/gomi)</summary> ### [`v1.6.4`](https://github.com/babarot/gomi/blob/HEAD/CHANGELOG.md#v164---2026-06-07) [Compare Source](babarot/gomi@v1.6.3...v1.6.4) ##### Improvements - fix: keep restore TUI within terminal bounds (closes [#​125](babarot/gomi#125)) by [@​babarot](https://github.com/babarot) in [#​127](babarot/gomi#127) - Allow --prune to skip confirmation with -f by [@​babarot](https://github.com/babarot) in [#​129](babarot/gomi#129) ##### Refactorings - Fix concurrency safety: eliminate global SelectionManager and add mutex to legacy Storage by [@​babarot](https://github.com/babarot) in [#​118](babarot/gomi#118) - Refactor: extract Trash interface, move orphan logic, and DRY up XDG storage by [@​babarot](https://github.com/babarot) in [#​119](babarot/gomi#119) - Refactor: decouple config, remove unused fields, and narrow UI dependencies by [@​babarot](https://github.com/babarot) in [#​120](babarot/gomi#120) - Update architecture docs and rename manager variables to match Trash interface by [@​babarot](https://github.com/babarot) in [#​121](babarot/gomi#121) - Fix fundamental design issues: circular references, untestable prompts, and implicit init() by [@​babarot](https://github.com/babarot) in [#​122](babarot/gomi#122) - Reduce syscalls in gomi -b restore path by [@​babarot](https://github.com/babarot) in [#​123](babarot/gomi#123) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xOTEuMiIsInVwZGF0ZWRJblZlciI6IjQzLjE5MS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6cGF0Y2giXX0=-->
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.
WHAT
Address four fundamental design problems identified during code review: remove the circular File-Storage reference, extract a Prompter interface for testable CLI interactions, deduplicate Manager's storage lookup, and replace two
init()functions with explicit initialization.WHY
File.storage circular reference — File held a reference back to its managing Storage, but Manager never used it (it finds storage via TrashPath prefix matching). The field existed only as dead weight creating a circular dependency.
Untestable CLI prompts —
restore.goandprune.gocalledui.Confirm()/ui.ConfirmYes()/ui.InputFilename()directly, making it impossible to test CLI commands without starting a real BubbleTea terminal program.Duplicated storage lookup —
Restore()andRemove()had identical inline closures to find which storage manages a file.Restorehad verbose debug logging whileRemovehad none.init() side effects — Two packages used
init()to perform filesystem operations on import:envset global path variables, andhistory/migrationrenamed files. Both violated the principle of explicit initialization.