Skip to content

Conversation

wimglenn
Copy link
Member

@wimglenn wimglenn commented Aug 25, 2018

For example taking into account full width asian characters, combining accents in latin characters, etc. - it's not entirely trivial because Python module filenames can for example have accents.

This is going to be extremely difficult to handle correctly 100% of the time, without a big performance hit, and this is just a start (plus some testing) for trying to provide a number that may hopefully be more reliable than the string length.

pytest without this stuff:

short_misaligned

pytest with this stuff:

short

…idth asian characters, accented latin characters
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Awesome work @wimglenn!

CHANGELOG Outdated
@@ -1,3 +1,11 @@
1.5.5 (unreleased)
Copy link
Member

Choose a reason for hiding this comment

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

This will need to be 1.6.0 as adds a new functionality. 👍

def width_of_current_line(self):
"""Return an estimate of the width so far in the current line.
.. versionadded:: 1.5.5
Copy link
Member

Choose a reason for hiding this comment

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

Update version here as well please.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Just need to update the version in the places requested and we are good to go! 👍

@wimglenn
Copy link
Member Author

@nicoddemus any idea what's going on in the appveyor build failure? doesn't look like any module I've touched?

@nicoddemus
Copy link
Member

Hmmm seems like a flaky test, let me trigger the build again to confirm

@nicoddemus nicoddemus merged commit d97f092 into pytest-dev:master Aug 27, 2018
@wimglenn wimglenn deleted the better_line_width_estimate branch August 27, 2018 15:42
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.

2 participants