Skip to content

Add support for pagination in the CLI wallet #1894

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

Merged
merged 1 commit into from
Apr 12, 2025

Conversation

OBorce
Copy link
Contributor

@OBorce OBorce commented Mar 18, 2025

No description provided.

Comment on lines +236 to +238
header: String,
body: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO "paginate" should accept a more structured data, where "header" is a list of column names and "body" is a table.

A TODO would be fine too for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added TODO

Comment on lines 269 to 271
crossterm::terminal::enable_raw_mode().expect("Should not fail normally");
let event = crossterm::event::read().expect("Should not fail normally");
crossterm::terminal::disable_raw_mode().expect("Should not fail normally");
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Probably the more correct way is to switch to the raw mode once at the beginning and switch back via OnceDestructor.
    (and probably we should first check that the terminal is not in the raw mode already).

    The problem is that println! (and therefore ConsoleOutput::print_line) won't work properly, because "\n" won't return the cursor to the beginning of the line ("\r" is needed for that).
    But we can just add a separate function ConsoleOutput::print (so that the line separator can be chosen by the caller) and mention in comments that the existing print_line and print_error are not suitable for a terminal in a raw mode.

    (though I'm not sure if the current implementation has any drawbacks, besides not looking nice).

  2. We probably need a way of pausing logging while in the pagination mode, because when the loop is waiting for a key press, the terminal is in the raw mode either way. So if a logging event happens, it won't be printed correctly (due to "\n" not returning the cursor).

Let's maybe at least add some TODOs for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added OnceDestructor for the inner loop, and added TODO for the outer loop

@OBorce OBorce force-pushed the feature/wallet-cli-pagination branch from 9d4a512 to b4c0c50 Compare March 31, 2025 04:05
@OBorce OBorce force-pushed the feature/wallet-cli-pagination branch from b4c0c50 to 6184588 Compare April 1, 2025 04:57
@OBorce OBorce merged commit 27d5946 into master Apr 12, 2025
18 checks passed
@OBorce OBorce deleted the feature/wallet-cli-pagination branch April 12, 2025 19:36
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.

3 participants