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

remove the one leading space for all lines in show(); fixes line wrapping problem #167

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

ahbarnett
Copy link
Contributor

Dear Kristoffer,

This PR is a simple fix to #166. It removes all the leading spaces (" ") that you had in each line of output in print_timer in show.jl. This makes my output table fill the exact width available, without spilling over by 1 char as in your current master.

Note that the 1-char spillover can be predicted, eg for the 1st line of header here:

printstyled(io, " ", topbottomrule^total_table_width, "\n"; bold=true)

since it prints one space then total_table_width additional chars, giving a width of total_table_width+1.
You have otherwise done a perfect job of insuring that total_table_width <= available_width, I think.

Maybe no-one else has this problem, in which case, please ignore the PR.
But I'd be surprised if that were the case, since I'm in a very standard set-up: GNOME terminal, 80-char width.

Thanks again! Alex

…ine wrapping by exactly 1 char too many for terminal widths around 80, the standard
@ahbarnett ahbarnett changed the title remove leading spaces for all lines in show(); fixes line wrapping problem remove the one leading space for all lines in show(); fixes line wrapping problem Jun 27, 2023
@fredrikekre
Copy link
Contributor

Fixes #154 too? I thought the space was intentional, but perhaps it is missed in the width computation?

@ahbarnett
Copy link
Contributor Author

looks like #154 is an overflow-by-1 error too, so, yes, would fix. Meanwhile I'm happily using my fixed version until this gets merged...

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