Fixed issue #433 - CLI eats cursor #598
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The issue is that underlying iterator is not fully consumed within the body of the `with file_progress()` block. Instead, that block creates generator expressions like `docs = (dict(zip(headers, row)) for row in reader)` These iterables are consumed later, outside the `with file_progress()` block, which consumes the underlying iterator, and in turn updates the progress bar. This means that the `ProgressBar.__exit__` method gets called before the last time the `ProgressBar.update` method gets called. The result is that the code to make the cursor invisible (inside the `update()` method) is called after the cleanup code to make it visible (in the `__exit__` method). The fix is to move consumption of the `docs` iterators within the progress bar block. (An additional fix, to make ProgressBar more robust against this kind of misuse, would to make it refusing to update after its `__exit__` method had been called, just like files cannot be `read()` after they are closed. That requires a in the click library).
|
Thanks! |
Closed
|
Manually tested. Before:
After:
|
simonw
added a commit
that referenced
this pull request
Nov 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue is that underlying iterator is not fully consumed within the body of the
with file_progress()block. Instead, that block creates generator expressions likedocs = (dict(zip(headers, row)) for row in reader)These iterables are consumed later, outside the
with file_progress()block, which consumes the underlying iterator, and in turn updates the progress bar.This means that the
ProgressBar.__exit__method gets called before the last time theProgressBar.updatemethod gets called. The result is that the code to make the cursor invisible (inside theupdate()method) is called after the cleanup code to make it visible (in the__exit__method).The fix is to move consumption of the
docsiterators within the progress bar block. ((An additional fix, to make ProgressBar more robust against this kind of misuse, would to make it refusing to update after its
__exit__method had been called, just like files cannot beread()after they are closed. That requires a in the click library).Note that Github diff obscures the simplicity of this diff, it's just indenting a block of code.
馃摎 Documentation preview 馃摎: https://sqlite-utils--598.org.readthedocs.build/en/598/