-
Notifications
You must be signed in to change notification settings - Fork 712
add filter option to list command #4187
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,11 @@ | |
import ( | ||
"bufio" | ||
"bytes" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"reflect" | ||
"regexp" | ||
"sort" | ||
"strings" | ||
|
||
|
@@ -57,6 +59,14 @@ | |
--format yaml - Output in YAML format | ||
--format table - Output in table format | ||
--format '{{ <go template> }}' - If the format begins and ends with '{{ }}', then it is used as a go template. | ||
Filtering instances: | ||
--filter EXPR - Filter instances using yq expression (this is equivalent to --yq 'select(EXPR)') | ||
Can be specified multiple times and it works with all output formats. | ||
Examples: | ||
--filter '.status == "Running"' | ||
--filter '.vmType == "vz"' | ||
--filter '.status == "Running"' --filter '.vmType == "vz"' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this AND-matching or OR-matching? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume you want the description updated to clarify. It is AND so you can do this: ❯ cat ~/bin/limactl-ps
#!/bin/sh
limactl ls --filter 'select(.status == "Running")' "$@"
❯ l ls
NAME STATUS SSH VMTYPE ARCH CPUS MEMORY DISK DIR
alpine-iso Running 127.0.0.1:50496 vz aarch64 4 4GiB 100GiB ~/.lima/alpine-iso
alpine-lima-3.22.2 Stopped 127.0.0.1:0 vz aarch64 4 4GiB 100GiB ~/.lima/alpine-lima-3.22.2
default Running 127.0.0.1:54512 qemu aarch64 4 4GiB 100GiB ~/.lima/default
foo Stopped 127.0.0.1:0 vz aarch64 4 1.177MiB 100GiB ~/.lima/foo
❯ l ps
NAME STATUS SSH VMTYPE ARCH CPUS MEMORY DISK DIR
alpine-iso Running 127.0.0.1:50496 vz aarch64 4 4GiB 100GiB ~/.lima/alpine-iso
default Running 127.0.0.1:54512 qemu aarch64 4 4GiB 100GiB ~/.lima/default
❯ l ps -l '.vmType == "vz"'
NAME STATUS SSH VMTYPE ARCH CPUS MEMORY DISK DIR
alpine-iso Running 127.0.0.1:50496 vz aarch64 4 4GiB 100GiB ~/.lima/alpine-iso |
||
` + store.FormatHelp + ` | ||
The following legacy flags continue to function: | ||
--json - equal to '--format json'`, | ||
|
@@ -72,6 +82,7 @@ | |
listCommand.Flags().BoolP("quiet", "q", false, "Only show names") | ||
listCommand.Flags().Bool("all-fields", false, "Show all fields") | ||
listCommand.Flags().StringArray("yq", nil, "Apply yq expression to each instance") | ||
listCommand.Flags().StringArrayP("filter", "l", nil, "Filter instances using yq expression (equivalent to --yq 'select(EXPR)')") | ||
|
||
return listCommand | ||
} | ||
|
@@ -121,6 +132,10 @@ | |
if err != nil { | ||
return err | ||
} | ||
filter, err := cmd.Flags().GetStringArray("filter") | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if jsonFormat { | ||
format = "json" | ||
|
@@ -141,6 +156,11 @@ | |
return errors.New("option --list-fields conflicts with option --yq") | ||
} | ||
} | ||
if len(filter) != 0 { | ||
if listFields { | ||
return errors.New("option --list-fields conflicts with option --filter") | ||
} | ||
} | ||
|
||
if quiet && format != "table" { | ||
return errors.New("option --quiet can only be used with '--format table'") | ||
|
@@ -220,15 +240,35 @@ | |
options.TerminalWidth = w | ||
} | ||
} | ||
// --yq implies --format json unless --format yaml has been explicitly specified | ||
|
||
// --yq implies --format json unless --format has been explicitly specified | ||
if len(yq) != 0 && !cmd.Flags().Changed("format") { | ||
format = "json" | ||
} | ||
|
||
// Always pipe JSON and YAML through yq to colorize it if isTTY | ||
if len(yq) == 0 && (format == "json" || format == "yaml") { | ||
yq = append(yq, ".") | ||
} | ||
|
||
for _, f := range filter { | ||
// only allow fields, ==, !=, and literals. | ||
valid := regexp.MustCompile(`^[a-zA-Z0-9_.\s"'-=><!]+$`) | ||
if !valid.MatchString(f) { | ||
return fmt.Errorf("unsafe characters in filter expression: %q", f) | ||
} | ||
Comment on lines
+257
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is useful. The whole point of And we already get an error when the YQ expression is invalid, e.g. ❯ l ls -l '.vmType =='
FATA[0000] failed to apply filter select(.vmType ==): '==' expects 2 args but there is 1 So I don't understand the purpose of this "validation", especially since it might make legitimate use cases invalid (e.g. you are restricting which characters are allowed inside literal strings). |
||
|
||
yq = append(yq, "select("+f+")") | ||
} | ||
|
||
if len(filter) != 0 && (format != "json" && format != "yaml") { | ||
instances, err = filterInstances(instances, yq) | ||
if err != nil { | ||
return err | ||
} | ||
yq = nil | ||
} | ||
|
||
if len(yq) == 0 { | ||
err = store.PrintInstances(cmd.OutOrStdout(), instances, format, &options) | ||
if err == nil && unmatchedInstances { | ||
|
@@ -320,3 +360,31 @@ | |
func listBashComplete(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { | ||
return bashCompleteInstanceNames(cmd) | ||
} | ||
|
||
// filterInstances applies yq expressions to instances and returns the filtered results. | ||
func filterInstances(instances []*limatype.Instance, yqExprs []string) ([]*limatype.Instance, error) { | ||
if len(yqExprs) == 0 { | ||
return instances, nil | ||
} | ||
|
||
yqExpr := strings.Join(yqExprs, " | ") | ||
|
||
var filteredInstances []*limatype.Instance | ||
for _, instance := range instances { | ||
jsonBytes, err := json.Marshal(instance) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to marshal instance %q: %w", instance.Name, err) | ||
} | ||
|
||
result, err := yqutil.EvaluateExpression(yqExpr, jsonBytes) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to apply filter %q: %w", yqExpr, err) | ||
} | ||
|
||
if len(result) > 0 { | ||
filteredInstances = append(filteredInstances, instance) | ||
} | ||
} | ||
|
||
return filteredInstances, nil | ||
} |
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.
Maybe we do not need this flag. We can just add --yq select() examples to --help and call it a day
Uh oh!
There was an error while loading. Please reload this page.
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.
That's why I wrote in #4007:
So you can do this (which isn't possible with
--yq
):I've not had time yet to review this PR to see if this is properly implemented.
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.
What if we just support
--format table
with--yq
?