Skip to content

Use width aware slice #684

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

Merged
merged 2 commits into from
May 13, 2017
Merged

Use width aware slice #684

merged 2 commits into from
May 13, 2017

Conversation

ata2001
Copy link
Contributor

@ata2001 ata2001 commented May 10, 2017

Fixes #670.

May not work with the last curtsies release, tested with the git version.

Although, I'm not sure if converting to FmtStr is the most efficient way of splitting lines, but curtsies.formatstring.width_aware_slice was not working as I've expected.

@ata2001
Copy link
Contributor Author

ata2001 commented May 10, 2017

Sorry, my bad: Some of the tests fail with the git version of curtsies too. Going to fix it tomorrow.

range(0, msg.width, columns),
range(columns, msg.width + columns, columns))]
if msg else ([''] if blank_line else []))
except ValueError:
Copy link
Contributor

Choose a reason for hiding this comment

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

In which case do we get a ValueError here?

Copy link
Contributor Author

@ata2001 ata2001 May 12, 2017

Choose a reason for hiding this comment

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

When attempting to get the width of a FmtStr, curtsies raises a ValueError, in case the length of the string is greater than 0, but the width of the FmtStr is less than 1.

https://raw.githubusercontent.com/thomasballinger/curtsies/master/curtsies/formatstring.py

At line 85.

@sebastinas sebastinas merged commit f0ca805 into bpython:master May 13, 2017
@ata2001 ata2001 deleted the width_aware_slice branch May 13, 2017 12:29
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