Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Cli: Update OutputFormat method to return a String to restore consistency #9904

Merged
merged 2 commits into from
May 7, 2020

Conversation

CriesofCarrots
Copy link
Contributor

Problem

The Cli process_command() method returns a String, which is printed in main.rs. In times past, every cli method populated this String, but as users wanted more complex and informative prints, they implemented those prints in the respective process_ method and returned Ok("".to_string()) to main.rs.

However, #9478 implemented Display for most of these complex prints, providing the opportunity to re-populate those empty Strings.

Summary of Changes

  • Update OutputFormat to return a formatted String, instead of doing the print itself.
  • Return formatted Strings from process_command()

Enables #9650 without the duplicated signature-prints

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

Nice! These have bothered me for awhile

@mergify mergify bot dismissed t-nelson’s stale review May 6, 2020 23:23

Pull request has been modified.

t-nelson
t-nelson previously approved these changes May 6, 2020
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks!

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label May 6, 2020
@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label May 6, 2020
@solana-grimes
Copy link
Contributor

💔 Unable to automerge due to CI failure

@mergify mergify bot dismissed t-nelson’s stale review May 7, 2020 00:54

Pull request has been modified.

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label May 7, 2020
@solana-grimes
Copy link
Contributor

💔 Unable to automerge due to CI failure

@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label May 7, 2020
@codecov
Copy link

codecov bot commented May 7, 2020

Codecov Report

Merging #9904 into master will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@           Coverage Diff            @@
##           master   #9904     +/-   ##
========================================
- Coverage    80.5%   80.3%   -0.2%     
========================================
  Files         283     283             
  Lines       65654   65642     -12     
========================================
- Hits        52858   52772     -86     
- Misses      12796   12870     +74     

@CriesofCarrots CriesofCarrots merged commit 65a52a4 into solana-labs:master May 7, 2020
mergify bot pushed a commit that referenced this pull request May 7, 2020
…ency (#9904)

* Update OutputFormat method to return a String to restore consistency

* Remove process_show_account special case

(cherry picked from commit 65a52a4)
solana-grimes pushed a commit that referenced this pull request May 7, 2020
@CriesofCarrots CriesofCarrots deleted the cli-output branch May 7, 2020 20:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants