Do not output a json array for whoami command when --json is given#1905
Do not output a json array for whoami command when --json is given#1905
Conversation
📝 WalkthroughWalkthroughThe term.Table API is refactored to accept individual data structures directly instead of requiring callers to wrap them in slices. The implementation now handles both single items and slices through internal reflect-based wrapping. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pkg/term/table.go`:
- Around line 28-34: The code in Term.table currently calls
reflect.ValueOf(data) and then uses val.Type(), which will panic if data is nil
(reflect.Value is invalid); add a nil/invalid check after val :=
reflect.ValueOf(data) and before using val.Type(): if data is nil (or
!val.IsValid()) construct a slice of the desired element type (e.g., []any or a
slice of the zero value type) or return an informative error, otherwise proceed
with the existing reflect.MakeSlice logic; update Term.table to handle the nil
case safely (check val.IsValid(), handle nil by creating a fallback slice or
returning an error) so val.Type() is never called on an invalid reflect.Value.
| func (t *Term) table(data any, attributes ...string) error { | ||
| // Ensure data is a slice | ||
| val := reflect.ValueOf(data) | ||
| if val.Kind() != reflect.Slice { | ||
| return errors.New("Table: input is not a slice") | ||
| val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1) | ||
| val.Index(0).Set(reflect.ValueOf(data)) | ||
| } |
There was a problem hiding this comment.
Potential panic if nil is passed.
If data is nil, reflect.ValueOf(nil) returns a zero reflect.Value, and calling val.Type() on line 32 will panic. Consider adding a nil check if this is a realistic scenario.
🛡️ Optional defensive nil check
func (t *Term) table(data any, attributes ...string) error {
// Ensure data is a slice
val := reflect.ValueOf(data)
+ if !val.IsValid() {
+ return nil // or return an error for nil input
+ }
if val.Kind() != reflect.Slice {
val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1)
val.Index(0).Set(reflect.ValueOf(data))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (t *Term) table(data any, attributes ...string) error { | |
| // Ensure data is a slice | |
| val := reflect.ValueOf(data) | |
| if val.Kind() != reflect.Slice { | |
| return errors.New("Table: input is not a slice") | |
| val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1) | |
| val.Index(0).Set(reflect.ValueOf(data)) | |
| } | |
| func (t *Term) table(data any, attributes ...string) error { | |
| // Ensure data is a slice | |
| val := reflect.ValueOf(data) | |
| if !val.IsValid() { | |
| return nil // or return an error for nil input | |
| } | |
| if val.Kind() != reflect.Slice { | |
| val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1) | |
| val.Index(0).Set(reflect.ValueOf(data)) | |
| } |
🤖 Prompt for AI Agents
In `@src/pkg/term/table.go` around lines 28 - 34, The code in Term.table currently
calls reflect.ValueOf(data) and then uses val.Type(), which will panic if data
is nil (reflect.Value is invalid); add a nil/invalid check after val :=
reflect.ValueOf(data) and before using val.Type(): if data is nil (or
!val.IsValid()) construct a slice of the desired element type (e.g., []any or a
slice of the zero value type) or return an informative error, otherwise proceed
with the existing reflect.MakeSlice logic; update Term.table to handle the nil
case safely (check val.IsValid(), handle nil by creating a fallback slice or
returning an error) so val.Type() is never called on an invalid reflect.Value.
| val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1) | ||
| val.Index(0).Set(reflect.ValueOf(data)) |
There was a problem hiding this comment.
What am I missing?
| val = reflect.MakeSlice(reflect.SliceOf(val.Type()), 1, 1) | |
| val.Index(0).Set(reflect.ValueOf(data)) | |
| val = reflect.ValueOf([]any{data}) |
There was a problem hiding this comment.
That was my first instinct too, but this results in a slice of any, not slice of the type, which causes panic later when looping below. So maybe I should improve the code below to allow slice of interfaces to be passed in right now only slice of solid types can be used.
4 participants
Description
Also allow json table to correctly handle non-slice input
Linked Issues
Checklist
Summary by CodeRabbit