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

Command for Registry_list #50

Open
wants to merge 5 commits into
base: old-main
Choose a base branch
from
Open

Conversation

hannydevelop
Copy link
Contributor

@hannydevelop hannydevelop commented May 19, 2022

This closes #37

status_err!("Can't convert chain info to JSON: {}", err);
std::process::exit(1);
});
print!("{}", info)
Copy link
Member

Choose a reason for hiding this comment

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

Two things: the _IBC directory is included in the list but it's not a chain; we need to filter it out. Also, could we format it like you did with the other list command where it doesn't show up inside of JSON brackets (and without quotes)? currently it looks like:

[
  "_IBC",
  "agoric",
  "akash",
  "arkh",
  "assetmantle",
  ...

vs.

agoric
akash
arkh
assetmantle
...

let mut serialize = serde_json::Serializer::with_formatter(buf, formatter);
info.serialize(&mut serialize).unwrap();

println!("{}", String::from_utf8(serialize.into_inner()).unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the quotes with String.remove()

Suggested change
println!("{}", String::from_utf8(serialize.into_inner()).unwrap());
println!("{}", info.remove("\"", "");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thank you

abscissa_tokio::run(&APP, async {
match ocular::chain::registry::list_chains().await {
Ok(mut info) => {
info.drain(0..1);
Copy link
Member

Choose a reason for hiding this comment

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

Since we can't guarantee the first element will always need to be removed, and that there aren't other folders that should be removed, I think a better approach would be a const ignore-list that can be checked when iterating.

Copy link
Contributor Author

@hannydevelop hannydevelop Jun 1, 2022

Choose a reason for hiding this comment

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

Well, with the format on cosmos/chain-registry, IBC is always the first value.

Comment on lines +17 to +21
let info = info.as_str();
let buf = Vec::new();
let formatter = serde_json::ser::PrettyFormatter::with_indent(b" ");
let mut serialize = serde_json::Serializer::with_formatter(buf, formatter);
info.serialize(&mut serialize).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what this is accomplishing; I don't see any formatting changes when I run the command. I think it can be simplified to an iterator filter and foreach for printing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you remove it before coming to this conclusion?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, I didn't see this part:

I think it can be simplified to an iterator filter and foreach for printing

I thought you meant we should continue with serde_json::to_string_pretty because that doesn't print very good. However, since we already use the above for the show command, I'll make it a function to avoid repetition

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.

Ocular_CLI (Registry_list Chain)
2 participants