feat(search-content): add non-interactive output modes#94
feat(search-content): add non-interactive output modes#94jogi47 wants to merge 1 commit intoYakitrak:mainfrom
Conversation
|
Thank you for the PR, I will take a look as soon as I can! |
| searchContentCmd.Flags().StringVarP(&vaultName, "vault", "v", "", "vault name") | ||
| searchContentCmd.Flags().BoolP("editor", "e", false, "open in editor instead of Obsidian") | ||
| searchContentCmd.Flags().Bool("list", false, "print matching results to stdout") | ||
| searchContentCmd.Flags().Bool("no-interactive", false, "disable interactive selection (alias for --list)") |
There was a problem hiding this comment.
Redundant flags: --list and --no-interactive do exactly the same thing — the description even calls --no-interactive "an alias for --list". This doubles the API surface and the flag description on this line makes the relationship confusing.
--no-interactive is the more conventional name (used by apt, pip, git, etc.) and is self-explanatory. --list is ambiguous — you could want a list interactively. Consider dropping --list and keeping only --no-interactive.
| for _, match := range matches { | ||
| fmt.Fprintln(output, formatMatchForList(match)) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Silent empty output in text non-interactive mode: when matches is empty the loop produces no output and returns nil. In contrast, interactive mode prints "No notes found containing '...'" (line 92). This inconsistency can surprise scripts — a caller can't distinguish between zero results and something going wrong.
JSON mode handles this correctly by always outputting []. For text mode, consider printing a message to stderr (keeping stdout clean for piping), or at least matching the interactive "no results" message.
| NoInteractive bool | ||
| Format string | ||
| InteractiveTerminal bool | ||
| Output io.Writer |
There was a problem hiding this comment.
Output io.Writer inside the options struct mixes user intent with infrastructure. SearchContentOptions currently carries both flag state (List, NoInteractive, Format, …) and a runtime dependency (Output). A more conventional Go approach is to pass the writer as a separate parameter:
func SearchNotesContentWithOptions(..., options SearchContentOptions, output io.Writer) errorMinor point — the current approach is testable and coherent — but worth considering for a cleaner separation.
|
|
||
| test-search-content: | ||
| go test ./pkg/actions -run TestSearchNotesContent -v | ||
| @help_output="$$(go run . search-content --help)"; \ |
There was a problem hiding this comment.
Fragile shell-based flag validation: grepping Cobra's help text output is brittle — it breaks if Cobra changes its help formatting, flag column width, or indentation. The same guarantee is already covered more reliably by TestSearchContentCommandFlagsWired in cmd/search_content_test.go (which uses Flags().Lookup(...)).
Consider removing lines 19–22 and relying on the unit test instead, leaving this target as just:
test-search-content:
go test ./pkg/actions -run TestSearchNotesContent -v
Summary
search-contentvia--listand--no-interactive--formatwithtext|json(jsonimplies non-interactive)--editorin non-interactive mode and preserve configured editor fallback behavior safelySearchNotesContent(..., useEditor bool)and introducingSearchNotesContentWithOptions(...)make test-search-contentValidation
go test ./...make test-search-contentCloses #75