Skip to content
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

[cli] format output of sui client gas to a well formatted table #13852

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

stefan-mysten
Copy link
Contributor

@stefan-mysten stefan-mysten commented Sep 19, 2023

Description

This PR displays the output of sui client gas [address] as a well formatted table. If there are more than 10 coins, it will add an extra header and footer with the number of coins.

Test Plan

image image

Found an address with lots of coins on devnet
image
image


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for the sui client gas command's JSON output
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

This PR changes the output of sui client gas [address] to a well-formatted table. This breaks the previous JSON format, when using --json flag, due to changing the GasCoin output type to a simpler one. For example, you might have previously accessed the gas coin ID using id.id and the value using balance.value from the original JSON, as in the following example:

{
    "id": {
      "id": "0x20365c5cee12091faf31e3ea5f3586a4ea5f1ae49d71cda99c104b5ae8325f8b"
    },
    "balance": {
      "value": 997250075972
    }
}

With this change, now you must use gasCoinId to access the ID and gasBalance for the balance vlaue. The JSON output now resembles the following:

{
   "gasCoinId": "0x20365c5cee12091faf31e3ea5f3586a4ea5f1ae49d71cda99c104b5ae8325f8b",
   "gasBalance": 997250075972
}

@vercel
Copy link

vercel bot commented Sep 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mysten-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 8:52pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2023 8:52pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Sep 20, 2023 8:52pm
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Sep 20, 2023 8:52pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Sep 20, 2023 8:52pm

Copy link
Contributor

@wlmyng wlmyng left a comment

Choose a reason for hiding this comment

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

Is the well-formatted table purely for aesthetic reasons or are there other benefits to this?

crates/sui/src/client_commands.rs Show resolved Hide resolved
table.with(TableStyle::rounded().horizontals([
HorizontalLine::new(1, TableStyle::modern().get_horizontal()),
HorizontalLine::new(2, TableStyle::modern().get_horizontal()),
HorizontalLine::new(gases.len() + 2, TableStyle::modern().get_horizontal()),
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the gases.len() + 2 for?

Copy link
Contributor Author

@stefan-mysten stefan-mysten Sep 20, 2023

Choose a reason for hiding this comment

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

This will draw a line between the last item (that has a coin value) in the table and the footer, similarly as it is done for the header. the +2 is due to the fact that there are two rows in the beginning: header text + header with column names. If you take a look at the screenshots, you can see these three horizontal lines that are defined there.

@stefan-mysten
Copy link
Contributor Author

stefan-mysten commented Sep 20, 2023

Is the well-formatted table purely for aesthetic reasons or are there other benefits to this?

Thanks Will for the review and for your thoughtful questions! I'll try to explain my reasoning here. In my view, there are two reasons:

  1. Makes the whole look-and-feel of the CLI the same, as most of the outputs are slowly becoming tabulated.
  2. Most of these outputs are new streamlined structs, which also gives us better JSON output. Before if a Sui type was used to display output, when requesting the JSON format there might be a few nested types without offering much added value. This way we get rid of that. This will make it easier to consume things from the CLI.
    For example

Before change

{
    "id": {
      "id": "0x20365c5cee12091faf31e3ea5f3586a4ea5f1ae49d71cda99c104b5ae8325f8b"
    },
    "balance": {
      "value": 997250075972
    }
  },

After this change

{
   "gasCoinId": "0x20365c5cee12091faf31e3ea5f3586a4ea5f1ae49d71cda99c104b5ae8325f8b",
   "coinValue": 997250075972
 }

Copy link
Contributor

@wlmyng wlmyng left a comment

Choose a reason for hiding this comment

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

Neat!

@stefan-mysten stefan-mysten changed the title [cli] show gas coins as a well formatted table [cli] format output of sui client gas to a well formatted table Sep 20, 2023
@amnn
Copy link
Contributor

amnn commented Sep 20, 2023

Output looks great @stefan-mysten , thanks! Before you land, can you update the Release Notes to mention the breaking change to the JSON format? (i.e. if someone was using id.id before to access the gas coin ID, they would now need to use gasCoinId -- similar for balance.value).

@stefan-mysten
Copy link
Contributor Author

stefan-mysten commented Sep 20, 2023

Output looks great @stefan-mysten , thanks! Before you land, can you update the Release Notes to mention the breaking change to the JSON format? (i.e. if someone was using id.id before to access the gas coin ID, they would now need to use gasCoinId -- similar for balance.value).

Good point! I revisited the naming and settled on gasCoinId and gasBalance. I fixed the release notes and rebased the commits. Should be good to go once the CI is green.

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.

None yet

3 participants