-
Notifications
You must be signed in to change notification settings - Fork 13
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
Support outputting a list of databases as JSON #112
Support outputting a list of databases as JSON #112
Conversation
@itowlson I noticed that in |
@kate-goldenring Just to reduce clutter in the help text. In that case, the JSON is primarily expected to be consumed by other programs, not be interactive users, so I felt it was not worth making interactive users look at the flag. |
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.
Looks good (style nit aside) but I think it needs to either respect the --app
filter or (less desirably) be incompatible with it.
Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>
71ba7bd
to
fbc40f5
Compare
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.
🎉 Looks good!
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.
Seems fine though there does seem to be unnecessary code duplication between the json and table formatting code paths.
src/commands/sqlite.rs
Outdated
if let Some(app) = &self.app { | ||
databases.retain(|d| { | ||
d.links | ||
.iter() | ||
.any(|l| l.app_name.as_deref().unwrap_or("UNKNOWN") == app) | ||
}); | ||
} | ||
if let Some(db) = &self.database { | ||
databases.retain(|d| d.name == *db); | ||
} |
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.
Seems we're repeating a lot of the same logic here between json and table printing. Why not delay the formatting decision until we've munged all the data?
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.
One is iterating over Links
while the other is over Databases
since the tabular form does one row per link while the json form does one array entry per database. If we want to do links as teh output format for json, we could share code but i think it makes more sense in json to represent a database and then nest all its links
Signed-off-by: Kate Goldenring <kate.goldenring@fermyon.com>
fbc40f5
to
79664e3
Compare
Takes a similar approach as
spin templates list
and provides an option of listing databases as JSONExample output: