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.

Sign up for GitHub

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

Jump to bottom

call length less in _wrap_chunks #274

Closed
wants to merge 1 commit into from
Closed

call length less in _wrap_chunks #274

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion blessed/sequences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def _wrap_chunks(self, chunks):
break
cur_line.append(chunks.pop())
cur_len += chunk_len
if chunks and Sequence(chunks[-1], term).length() > width:
if chunks and chunk_len > width:
Copy link
Owner

@jquast jquast Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

at this time chunk_len represents the length of the last chunk that was "popped" and not the last chunk in chunks, but who knows, maybe that's a good thing? just need to add more tests with #273 and if it works then that's fine

on closer look, this is fine, because of if chunks predicate that this only tests true on the break statement in the while loop, looks like a fine optimization, thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was expecting the linter to complain since chunk_len is defined within the while loop. I'm not sure if it's smart enough to know it's guarded or what, but it's not complaining.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think the linter will catch it. mypy will complain about it - but that's because mypy doesn't understand the if chunks aspect

Copy link
Collaborator

Choose a reason for hiding this comment

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

mypy ran fine

Copy link
Owner

Choose a reason for hiding this comment

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

As for the dubious use of chunk_len outside of while loop, I've opted to incorporate the performance enhancement but move it to the inside of while loop in PR #275, thanks!

self._handle_long_word(chunks, cur_line, cur_len, width)
if drop_whitespace and (
cur_line and Sequence(cur_line[-1], term).strip() == ''):
Expand Down
Loading