Add file path to progress output #4764
Conversation
|
This needs more work. As said, I didn't test on Windows before submitting this, so I didn't notice the test failure (the space character I was talking about), but there are other issues as well. To no surprise, there are reflowing issues on Windows when changing the width of the terminal application(s), because lines don't get cleared properly (not an issue with the code - there's always the carriage return character), but there also appears to be an issue with how the path gets truncated. I will have to check later. |
- Remove `streamlink_cli.utils.terminal` module again and move logic
to the `Progress` and `ProgressFormatter` classes
- Replace `output` and `formatter` arguments of the `Progress` class
with a simple `stream` argument
- Rewrite `ProgressFormatter.format()`
- Pre-parse format strings via `string.Formatter.parse()`
and store the parsing results
- Add support for substitutions with dynamic/variable lengths
- Update tests
bastimeyer
force-pushed
the
cli/progress/refactor-and-add-path
branch
from
7b39396 to
8b50c7b
Compare
- Add `path` argument to the `Progress` class - Update formats and add path variable with a min-width of 15 - Truncate paths according to the available space - Add and update tests
bastimeyer
force-pushed
the
cli/progress/refactor-and-add-path
branch
from
8b50c7b to
89b439f
Compare
|
Everything should be working as intended now. If there are any text duplication issues when resizing the terminal application, then it's because of the terminal application not respecting carriage return characters on the line that's getting updated by the progress output. All standard terminals on Windows as well as the one on macOS have this issue, which is quite ridiculous. Konsole for example works beautifully. Not an issue with this implementation or the ones before though. streamlink.mp4 |
|
Thanks for the helpful comments, it made reviewing this easier. |
Resolves #4752
This refactors the
progressmodule, removes theterminalmodule that was added in #4656, adds support for substitutions in format strings with dynamic/variable lengths, and finally adds a new format string that includes the file path. Paths now get truncated in a proper way, and the whole available space is used. Once not enough space is available for the path to be shown in a proper/usable way (at least 15 characters), then it'll fall back to the next format.Didn't test on Windows. There's a special case when adding space characters to the output, where it removes one character. Since the non-space characters don't get covered by this special case, it's possible that the line will overflow. Need to check this later.
Also didn't test with paths that include CJK characters with widths larger than 1. After the changes of #4656, the whole character width calculation was unused, as there was no user-generated content in the progress output. Now that's obviously changed with the addition of the path, but the
cut()method should still do its job according to the tests, so I don't think there will be an issue here.