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

Inject width via pylib to argparse formatter #5056

Merged
merged 1 commit into from Aug 17, 2019

Conversation

@blueyed
Copy link
Contributor

commented Apr 5, 2019

argparse.HelpFormatter looks at $COLUMNS only, falling back to a default of 80.

py.io.get_terminal_width() is smarter there, and could even work better with pytest-dev/py#219.

This ensures to use a consistent value for formatting the ini values etc.

1: pytest-dev/py#219

TODO:

  • changelog
  • coverage
@codecov

This comment has been minimized.

Copy link

commented Apr 5, 2019

Codecov Report

Merging #5056 into features will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5056      +/-   ##
============================================
+ Coverage     96.25%   96.25%   +<.01%     
============================================
  Files           117      117              
  Lines         25948    25962      +14     
  Branches       2497     2499       +2     
============================================
+ Hits          24977    24991      +14     
  Misses          667      667              
  Partials        304      304
Impacted Files Coverage Δ
src/_pytest/config/argparsing.py 88.41% <100%> (+0.2%) ⬆️
testing/test_config.py 99.85% <100%> (ø) ⬆️

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 0f11a7a...2a6a1ca. Read the comment docs.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

Together with the py patch, it makes pytest --help | less look much nicer now (I have a HH shell suffix alias, which I usually use to look at --help output).

@blueyed blueyed requested a review from RonnyPfannschmidt Apr 9, 2019
@blueyed blueyed force-pushed the blueyed:argparsing-width branch 2 times, most recently from 4b49ef1 to 3052602 Aug 17, 2019
`argparse.HelpFormatter` looks at `$COLUMNS` only, falling back to a
default of 80.

`py.io.get_terminal_width()` is smarter there, and could even work
better with pytest-dev/py#219.

This ensures to use a consistent value for formatting the ini values etc.
@blueyed blueyed force-pushed the blueyed:argparsing-width branch from 3052602 to 2a6a1ca Aug 17, 2019
@blueyed blueyed merged commit f05ca74 into pytest-dev:features Aug 17, 2019
5 checks passed
5 checks passed
WIP Ready for review
Details
codecov/patch 100% of diff hit (target 96.25%)
Details
codecov/project 96.25% (+<.01%) compared to 0f11a7a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
pytest-CI #20190817.4 succeeded
Details
@blueyed blueyed deleted the blueyed:argparsing-width branch Aug 17, 2019
def __init__(self, *args, **kwargs):
"""Use more accurate terminal width via pylib."""
if "width" not in kwargs:
kwargs["width"] = py.io.get_terminal_width()

This comment has been minimized.

Copy link
@asottile

asottile Oct 3, 2019

Member

should we use setdefault(...) and shutil.get_terminal_width?

This comment has been minimized.

Copy link
@blueyed

blueyed Oct 16, 2019

Author Contributor

setdefault maybe, but otherwise it is meant to use an improved version (pytest-dev/py#219).

This comment has been minimized.

Copy link
@asottile

asottile Oct 16, 2019

Member

hmmm I'm firmly in the camp that we should stop adding things to pylib

This comment has been minimized.

Copy link
@blueyed

blueyed Oct 16, 2019

Author Contributor

sure, but better then to pytest - assuming that Python does not fix it itself. IIRC there's an old issue / PR idling there.
Anyway, this is merged, and pylib's PR is also idling. So no worries.. ;)

This comment has been minimized.

Copy link
@asottile

asottile Oct 16, 2019

Member

I'd rather put it in pytest, I've been trying to factor out pylib and this does the opposite!

This comment has been minimized.

Copy link
@blueyed

blueyed Oct 16, 2019

Author Contributor

ok, maybe later.
Feel free to fix this one up like you suggested initially then if it bothers you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.