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

feat: Improved usability of NEAR CLI in scripts #445

Open
wants to merge 42 commits into
base: main
Choose a base branch
from

Conversation

FroVolod
Copy link
Collaborator

@FroVolod FroVolod commented Feb 4, 2025

Resolves #437

Running NEAR CLI in the --quiet case:
Image

Running NEAR CLI in the "interactive" case:
Screenshot 2025-02-04 at 19 16 42
Screenshot 2025-02-04 at 19 17 18

.with(env_filter)
.init();
}
Verbosity::Quiet => {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, that's a good use of the tracing crate 👍

@FroVolod FroVolod force-pushed the update-for-quiet-option branch from 145b7a9 to 65180f7 Compare February 24, 2025 15:51
@FroVolod FroVolod marked this pull request as ready for review March 5, 2025 07:40
akorchyn
akorchyn previously approved these changes Mar 6, 2025
Copy link
Collaborator

@akorchyn akorchyn left a comment

Choose a reason for hiding this comment

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

Big effort. But I think it can be improved and reduce repeat-ness.

I think it would be beneficial to introduce targets.
e.g.: teach-me, interactive, output.

And depending on the parameter, suppress interactive.

@@ -138,6 +145,10 @@ fn save_access_key(
account_id.as_ref(),
)
.wrap_err_with(|| format!("Failed to save a file with access key: {}", public_key_str))?;
eprintln!("{}", storage_message);
tracing::info!(
parent: &tracing::Span::none(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to repeat that parent: none()?

if let crate::Verbosity::Quiet = verbosity {
println!("{}", info_str);
};
info_str.push_str("\n------------------------------------");
Copy link
Collaborator

Choose a reason for hiding this comment

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

else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

println!("Contract state (values):\n{}\n", serde_json::to_string_pretty(&result.values)?);
println!("Contract state (proof):\n{:#?}\n", result.proof)
}
tracing::info!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

else?

To be honest, I believe that tracing could be configured with using target to display output and to remove noise

this will help to reduce that repeateness. I would ask you to take a look a bit into that direction.
I don't mind if it will come in the next PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok

src/common.rs Outdated
Comment on lines 1340 to 1343
if let crate::Verbosity::Quiet = verbosity {
std::io::stdout().write_all(bytes_result)?;
return Ok(());
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this block before let result_output, so the program does not perform useless work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

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

Successfully merging this pull request may close these issues.

Usability of the NEAR CLI in scripting?
4 participants