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

Substituting plain headed box when show_header=False on Table #2330

Merged
merged 3 commits into from Jun 8, 2022

Conversation

darrenburns
Copy link
Member

@darrenburns darrenburns commented Jun 7, 2022

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

Ensure that if the user passes show_header=False to Table, then we don't use a Box which assumes the first row is a header row.

(Note the examples below aren't of the exact same table, only the first row of the box is relevant anyway)

After

The first line is smooth because show_header=False, so the box used was updated to one that doesn't use different header edges.

image

Before

The first line uses thicker lines, because the HEAVY_HEAD box is used by default, even though there's no header.

image

Fixes #2312

@darrenburns darrenburns marked this pull request as ready for review June 7, 2022 11:21
@@ -14,7 +14,6 @@ repos:
- id: check-yaml
- id: end-of-file-fixer
- id: mixed-line-ending
- id: trailing-whitespace
Copy link
Member Author

Choose a reason for hiding this comment

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

This incorrectly removed some whitespace inside a multiline string (inside a Box), which led to errors.

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022

Codecov Report

Merging #2330 (17c6aa1) into master (14d47c9) will decrease coverage by 0.12%.
The diff coverage is 95.67%.

@@            Coverage Diff             @@
##           master    #2330      +/-   ##
==========================================
- Coverage   98.88%   98.76%   -0.13%     
==========================================
  Files          73       73              
  Lines        7629     7690      +61     
==========================================
+ Hits         7544     7595      +51     
- Misses         85       95      +10     
Flag Coverage Δ
unittests 98.76% <95.67%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rich/_export_format.py 100.00% <ø> (ø)
rich/cells.py 100.00% <ø> (ø)
rich/live.py 97.90% <40.00%> (-2.10%) ⬇️
rich/console.py 98.29% <96.03%> (-0.49%) ⬇️
rich/_inspect.py 100.00% <100.00%> (ø)
rich/_wrap.py 100.00% <100.00%> (ø)
rich/box.py 100.00% <100.00%> (ø)
rich/control.py 100.00% <100.00%> (ø)
rich/syntax.py 99.30% <100.00%> (+0.02%) ⬆️
rich/table.py 100.00% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8a9466...17c6aa1. Read the comment docs.

Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable solution. A small request and feel free to merge.

rich/table.py Outdated Show resolved Hide resolved
@darrenburns darrenburns merged commit 51889e2 into master Jun 8, 2022
@darrenburns darrenburns deleted the table-header-border-issue branch June 8, 2022 15:22
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.

[BUG] If show_header=False the next row will have the header border style
3 participants