-
Notifications
You must be signed in to change notification settings - Fork 0
feat(cli): add rg/grep command for searching synced commands #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add new `shelltime rg` command (with `grep` alias) to search server-synced commands via GraphQL API. Supports table/JSON output and multiple filters: - shell, hostname, username - exit code, main command, session ID - time range (since/until) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new rg command (with a grep alias) for searching synced commands, which is a great addition to the CLI's functionality. The implementation is solid, covering filtering, pagination, and multiple output formats (table and JSON).
I've left a couple of comments for improvement:
- A medium-severity comment regarding platform-independent path construction, to align with the repository's general rules and improve portability.
- A high-severity comment about error handling for time-based filters, which currently fails silently on invalid input. Addressing this will improve user experience and prevent confusion.
Overall, this is a well-implemented feature. Nice work!
commands/rg.go
Outdated
| if since := c.String("since"); since != "" { | ||
| t, err := time.Parse(time.RFC3339, since) | ||
| if err == nil { | ||
| timeFilters = append(timeFilters, float64(t.UnixMilli())) | ||
| } | ||
| } | ||
| if until := c.String("until"); until != "" { | ||
| t, err := time.Parse(time.RFC3339, until) | ||
| if err == nil { | ||
| timeFilters = append(timeFilters, float64(t.UnixMilli())) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code for parsing since and until time filters silently ignores any parsing errors. If a user provides an invalid time format (e.g., not RFC3339), the filter is simply not applied, and the user receives no feedback. This can lead to confusing search results.
I recommend refactoring buildCommandFilter to return an error. This way, you can propagate the parsing error up to commandRg and inform the user about the invalid input.
For example:
func buildCommandFilter(c *cli.Context, searchText string) (map[string]interface{}, error) {
// ...
if since := c.String("since"); since != "" {
t, err := time.Parse(time.RFC3339, since)
if err != nil {
return nil, fmt.Errorf("invalid 'since' time format: %w. Use RFC3339 format (e.g. '2006-01-02T15:04:05Z07:00')", err)
}
timeFilters = append(timeFilters, float64(t.UnixMilli()))
}
// ... similar for 'until'
// ...
return filter, nil
}And then in commandRg, you would handle the error:
filter, err := buildCommandFilter(c, searchText)
if err != nil {
return err
}
commands/rg.go
Outdated
| ctx, span := commandTracer.Start(c.Context, "rg", trace.WithSpanKind(trace.SpanKindClient)) | ||
| defer span.End() | ||
|
|
||
| SetupLogger(os.ExpandEnv("$HOME/" + model.COMMAND_BASE_STORAGE_FOLDER)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path for the logger is constructed using os.ExpandEnv with a hardcoded path separator. This is not platform-independent. You should use os.UserHomeDir() to get the user's home directory and filepath.Join to construct paths. This ensures the code works correctly on different operating systems (e.g., Windows).
You'll also need to import the path/filepath package.
homeDir, err := os.UserHomeDir()
if err != nil {
return fmt.Errorf("could not get user home directory: %w", err)
}
SetupLogger(filepath.Join(homeDir, model.COMMAND_BASE_STORAGE_FOLDER))References
- For platform-independent paths, use
filepath.Jointo combine segments andos.UserHomeDir()to get the home directory, rather than hardcoding path separators or environment variables like$HOME.
- Move API types and FetchCommandsFromServer to model/command_search.go - Rename commands/rg.go to commands/grep.go - Remove sessionId from filter and output - Support flexible date formats for --since/--until (2024, 2024-01, 2024-01-15) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show a spinner with "Searching commands..." while fetching data from the server, then hide it when results are received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change from offset-based pagination to cursor-based pagination using lastId as required by the server's InputPagination type. Also add ID column to table output for pagination reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show errors as formatted output instead of returning them:
- JSON format: output {"error": "message"}
- Table format: print error in red
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
shelltime rgcommand withgrepalias to search server-synced commands via GraphQL API--limitand--offsetflagsTest plan
shelltime rg --helpto verify command registrationshelltime rg "docker"shelltime rg "git" --shell zsh --result 0shelltime rg "npm" -f jsonshelltime rg "test" --limit 10 --offset 5🤖 Generated with Claude Code