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

sync_info format peer list #8978

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maogo
Copy link
Contributor

@maogo maogo commented Aug 21, 2023

2023-08-22_04-26

format peer list
@vtnerd
Copy link
Contributor

vtnerd commented Aug 23, 2023

How does this formatting work with smaller screens (admitting I haven't looked at code too closely)?

@maogo
Copy link
Contributor Author

maogo commented Aug 24, 2023

yeah, that depends upon the screen resolution and visible area of terminal . When the horizontal width is not enough that table won't display correctly.

If i2p/tor address joined:
with resolution 1920 * 1080 works fine , not sure lower resolution, like 1280 * 1024 maybe No.

@vtnerd
Copy link
Contributor

vtnerd commented Aug 24, 2023

It's temping to not allow the patch on that basis, interested to know what others think.

@Gingeropolous
Copy link
Collaborator

could make it an option for the command

sync_info pretty

or if commands can't get options, just a new command (one thats not included in the default output of help because well that thing is already cluttered).

sync_info_pretty

tools::success_msg_writer() << "Downloading at " << current_download << " kB/s";
if (res.next_needed_pruning_seed)
tools::success_msg_writer() << "Next needed pruning seed: " << res.next_needed_pruning_seed;

tools::success_msg_writer() << std::to_string(res.peers.size()) << " peers";
tools::success_msg_writer() << "Remote Host Peer_ID State Prune_Seed Height DL kB/s, Queued Blocks / MB";
std::string table_line = "+" + std::string(longest_host_width - 1,'-')+ "+" + std::string(20 - 1,'-') + "+" + std::string(20 - 1,'-') + "+" + std::string(15 - 1,'-') + "+" + std::string(15 - 1,'-') + "+" + std::string(15 - 1,'-') + "+" + std::string(15 - 1,'-') + "+" + std::string(15 - 1,'-') + "+";

Choose a reason for hiding this comment

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

I find the style here quite brittle: To change a column width, you have to edit three separate lines, and they don't even use the same constant (first two are X, last one is X - 1). It'd be way easier to read, and less error-prone, to use a structured array/vector here (e.g. heading + width, where width defaults to size, and can be adjusted for columns with wide content, like remote host); such an approach would also make it much easier to wrap the table across lines to fit the terminal, if someone wanted.

@maogo
Copy link
Contributor Author

maogo commented Jan 27, 2024

Hard-code here is bad , A well-encapsulated class for reuse is a better way, I am not c++ guy , If someone wants to make a new PR that's better .

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

Successfully merging this pull request may close these issues.

5 participants