Skip to content

Refactor list command to support json format #388

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

roulpriya
Copy link
Contributor

Fixes Issue #376
Refactor the List command to support JSON output format.

List.self, ["list", "--format", "json"], format: .json
)

let result = try JSONSerialization.jsonObject(
Copy link
Member

@cmcgee1024 cmcgee1024 Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Instead of decoding this JSON data into a dictionary, why not decode into a InstalledToolchainsListInfo instead? The checks below would just be simple field accesses instead of dictionary lookups.

Copy link
Contributor Author

@roulpriya roulpriya Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing that forces us to implement Decodable in all the structs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be much effort to make them Decodable?

List.self, ["list", "5", "--format", "json"], format: .json
)

var result = try JSONSerialization.jsonObject(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: same as above. Why not decode into a InstalledToolchainsListInfo here?

List.self, ["list", "--format", "json"], format: .json
)

let result = try JSONSerialization.jsonObject(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: see above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants