Copilot commented on code in PR #165:
URL:
https://github.com/apache/cloudstack-cloudmonkey/pull/165#discussion_r2253947298
##########
cli/completer.go:
##########
@@ -421,12 +421,14 @@ func (t *autoCompleter) Do(line []rune, pos int) (options
[][]rune, offset int)
}
for _, item := range filteredOptions {
option := item.Value + " "
- if len(filteredOptions) > 1 && len(item.Detail)
> 0 {
+ if len(filteredOptions) > 0 && len(item.Detail)
> 0 {
Review Comment:
[nitpick] The condition change from `> 1` to `> 0` means details will now
always be shown when they exist, even for single options. This could clutter
the display unnecessarily. Consider if this change aligns with the intended UX
behavior.
```suggestion
if len(filteredOptions) > 1 && len(item.Detail)
> 0 {
```
##########
cli/completer.go:
##########
@@ -421,12 +421,14 @@ func (t *autoCompleter) Do(line []rune, pos int) (options
[][]rune, offset int)
}
for _, item := range filteredOptions {
option := item.Value + " "
- if len(filteredOptions) > 1 && len(item.Detail)
> 0 {
+ if len(filteredOptions) > 0 && len(item.Detail)
> 0 {
option += fmt.Sprintf("(%v)",
item.Detail)
}
if strings.HasPrefix(option, argInput) {
options = append(options,
[]rune(option[len(argInput):]))
offset = len(argInput)
+ } else if len(item.Detail) > 0 &&
strings.HasPrefix(item.Detail, argInput) {
Review Comment:
[nitpick] The logic for handling detail-based matches could be simplified.
Consider extracting the condition `len(item.Detail) > 0 &&
strings.HasPrefix(item.Detail, argInput)` into a variable to avoid duplication
and improve readability.
```suggestion
detailMatchesInput := len(item.Detail) > 0 &&
strings.HasPrefix(item.Detail, argInput)
if strings.HasPrefix(option, argInput) {
options = append(options,
[]rune(option[len(argInput):]))
offset = len(argInput)
} else if detailMatchesInput {
```
##########
cli/completer.go:
##########
@@ -421,12 +421,14 @@ func (t *autoCompleter) Do(line []rune, pos int) (options
[][]rune, offset int)
}
for _, item := range filteredOptions {
option := item.Value + " "
- if len(filteredOptions) > 1 && len(item.Detail)
> 0 {
+ if len(filteredOptions) > 0 && len(item.Detail)
> 0 {
option += fmt.Sprintf("(%v)",
item.Detail)
}
if strings.HasPrefix(option, argInput) {
options = append(options,
[]rune(option[len(argInput):]))
offset = len(argInput)
+ } else if len(item.Detail) > 0 &&
strings.HasPrefix(item.Detail, argInput) {
+ options = append(options,
[]rune(option))
Review Comment:
When matching on detail, the full option string is returned without
adjusting the offset. This means the completion will include the entire option
text, potentially overwriting the user's input. The offset should be set to 0
when returning the full option.
```suggestion
options = append(options,
[]rune(option))
offset = 0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
