From d7cb0c145d7aa9992535bb9ad7e1be0abc1b9c9e Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:06:15 +0200 Subject: [PATCH 01/15] fix(bar): populate widgets before start() flips _started MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProgressBar.start() called super().start() (which sets _started=True via ProgressBarMixinBase.start) before building default_widgets(), so a concurrent reader — e.g. MultiBar's render thread reading bar.started() then asserting bar.widgets in _label_bar — could observe started() True with an empty widget list and crash. Move the cooperative super().start() dispatch to run after all widget/prefix/suffix/poll/gate/max_value setup completes. The 0% draw still happens at the same point (stream redirect and console mode are set up in super().start(), before the draw). Add a deterministic regression test capturing the widget list at the exact _started flip. --- progressbar/bar.py | 16 +++++++++++----- tests/test_multibar.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/progressbar/bar.py b/progressbar/bar.py index a80501ce..df637e60 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -1295,11 +1295,6 @@ def start( if self.max_value is None: self.max_value = self._DEFAULT_MAXVAL - # Cooperative dispatch through the MRO - # (StdRedirectMixin -> DefaultFdMixin -> ProgressBarMixinBase); - # ResizableMixin/ProgressBarBase define no `start` and are skipped. - super().start(max_value=max_value) - # Constructing the default widgets is only done when we know max_value if not self.widgets: self.widgets = self.default_widgets() @@ -1314,6 +1309,17 @@ def start( self._gate_enabled = False self._verify_max_value() + # Cooperative dispatch through the MRO + # (StdRedirectMixin -> DefaultFdMixin -> ProgressBarMixinBase); + # ResizableMixin/ProgressBarBase define no `start` and are skipped. + # This runs *after* all widget/state setup so that `_started` (set by + # ProgressBarMixinBase.start) only becomes observable once `widgets` + # is fully populated. Otherwise a concurrent reader (e.g. MultiBar's + # render thread) could see `started()` True with an empty widget list + # crash in `_label_bar`'s `assert bar.widgets`. The 0% draw below + # still happens at the same point, after stream/console setup. + super().start(max_value=max_value) + now = datetime.now() self.start_time = self.initial_start_time or now self.last_update_time = now diff --git a/tests/test_multibar.py b/tests/test_multibar.py index e960ebad..b70902a1 100644 --- a/tests/test_multibar.py +++ b/tests/test_multibar.py @@ -163,6 +163,43 @@ def test_multibar_empty_key() -> None: multibar.render(force=True) +def test_started_flag_not_observable_before_widgets(monkeypatch) -> None: + """Regression: ``_started`` must not flip True before widgets are built. + + ``MultiBar.render()`` (potentially from a background thread) reads + ``bar.started()`` and then ``_label_bar`` asserts ``bar.widgets``. If + ``start()`` sets ``_started`` before populating ``default_widgets()`` there + is a window where a concurrent reader observes ``started() is True`` with + an empty ``widgets`` list and crashes on that assertion. Reproduced + deterministically by capturing the widget list at the exact ``_started`` + flip. + """ + import progressbar.bar as bar_module + + original_start = bar_module.ProgressBarMixinBase.start + observed: dict[str, bool] = {} + + def recording_start(self, **kwargs): + result = original_start(self, **kwargs) + # `_started` has just flipped True here; capture whether the widget + # list is already populated at this exact moment. + observed['widgets_at_flip'] = bool(self.widgets) + observed['started_at_flip'] = self.started() + return result + + monkeypatch.setattr( + bar_module.ProgressBarMixinBase, 'start', recording_start + ) + + bar = progressbar.ProgressBar(max_value=N, fd=io.StringIO()) + bar.start() + + assert observed.get('started_at_flip') is True + assert observed.get('widgets_at_flip') is True, ( + 'widgets must be populated before started() can observe _started' + ) + + def test_multibar_print() -> None: bars = 5 n = 10 From 04d57e803bd5b68c40f3becceb6820e715e51af6 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:11:25 +0200 Subject: [PATCH 02/15] refactor(bar): make data() a pure snapshot via _mark_update() data() secretly reset _last_update_time/_last_update_timer on every call, so a getter mutated timing state. Extract the reset into a private _mark_update() and call it from _update_parents() (the redraw path), where the gate calibration in _draw_and_recalibrate already expects the timer to be refreshed during the draw. data() is now a pure read with no timing side effects; the public last_update_time and gate cadence are unchanged because the stamp still happens once per redraw at the same point. Add a regression test (deterministic advancing clock) asserting two consecutive data() calls leave the timing fields untouched. --- progressbar/bar.py | 27 +++++++++++++++++++++++--- tests/test_progressbar.py | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/progressbar/bar.py b/progressbar/bar.py index df637e60..90d4eedd 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -874,9 +874,10 @@ def data(self) -> types.Dict[str, types.Any]: - `variables`: Dictionary of user-defined variables for the :py:class:`~progressbar.widgets.Variable`'s. + This is a pure snapshot of the current state: it performs no timing + side effects. The redraw path stamps the update timestamps via + :py:meth:`_mark_update` before the widgets read them. """ - self._last_update_time = time.time() - self._last_update_timer = timeit.default_timer() elapsed = self.last_update_time - self.start_time # type: ignore # For Python 2.7 and higher we have _`timedelta.total_seconds`, but we # want to support older versions as well @@ -1165,7 +1166,7 @@ def _draw_and_recalibrate( prev_value = self._last_drawn_value prev_timer = self._last_update_timer try: - self._update_parents(value) # data() refreshes the timer + self._update_parents(value) # _mark_update refreshes timer finally: # `_last_drawn_value` is the value at the last *redraw* (the # pixel reference for `_needs_update`); set in finally so it @@ -1248,8 +1249,28 @@ def _update_variables(self, kwargs): variables_changed = True return variables_changed + def _mark_update(self) -> None: + """Stamp the wall-clock and perf-counter time of the current redraw. + + Called from the draw path (:py:meth:`_update_parents`) before the + widgets read ``last_update_time``. ``_last_update_timer`` feeds the + poll-interval gate in :py:meth:`_needs_update` and the cadence + calibration in :py:meth:`_draw_and_recalibrate`; ``_last_update_time`` + backs the public ``last_update_time`` property used by + elapsed-time/ETA widgets. Kept out of :py:meth:`data` so that method is + a pure snapshot with no timing side effects. + """ + self._last_update_time = time.time() + self._last_update_timer = timeit.default_timer() + def _update_parents(self, value: ValueT): self.updates += 1 + # Stamp the redraw timestamps before formatting widgets so that + # `data()`/`last_update_time` reflect this redraw and the gate + # calibration in `_draw_and_recalibrate` measures the interval up to + # this draw (it snapshots `_last_update_timer` before this call and + # reads it again afterwards). + self._mark_update() # Cooperative dispatch through the MRO # (StdRedirectMixin -> DefaultFdMixin -> ProgressBarMixinBase). The # `value` is passed by keyword so the intermediate `*args, **kwargs` diff --git a/tests/test_progressbar.py b/tests/test_progressbar.py index 2267b59d..376cbd60 100644 --- a/tests/test_progressbar.py +++ b/tests/test_progressbar.py @@ -99,6 +99,47 @@ def test_elapsed_data_spans_days() -> None: assert data['days_elapsed'] == pytest.approx(expected_days, abs=0.01) +@pytest.mark.no_freezegun +def test_data_is_a_pure_snapshot(monkeypatch) -> None: + # `data()` must be a pure read of the current state: calling it must not + # mutate the timing fields (`_last_update_time` / `_last_update_timer`). + # The redraw path refreshes those via `_mark_update()`, not the getter. + # + # A strictly-increasing clock makes any hidden mutation observable: on the + # old code each data() call re-stamped the fields with a fresh (larger) + # value, so two calls would disagree. + import timeit as _timeit + + import progressbar.bar as bar_module + + ticks = iter(range(1_700_000_000, 1_700_001_000)) + + def fake_clock() -> float: + return float(next(ticks)) + + bar = progressbar.ProgressBar( + max_value=10, fd=io.StringIO(), term_width=60 + ) + bar.start() + + monkeypatch.setattr(bar_module.time, 'time', fake_clock) + monkeypatch.setattr(_timeit, 'default_timer', fake_clock) + + time_before = bar._last_update_time + timer_before = bar._last_update_timer + + first = bar.data() + second = bar.data() + + # Neither the wall-clock nor the perf-counter timing state may change. + assert bar._last_update_time == time_before + assert bar._last_update_timer == timer_before + # And the two snapshots agree on the timing-derived fields. + assert first['last_update_time'] == second['last_update_time'] + assert first['total_seconds_elapsed'] == second['total_seconds_elapsed'] + assert first['time_elapsed'] == second['time_elapsed'] + + def test_restart_after_finish_writes_final_newline() -> None: # Regression: A2 - init() did not reset _finished, so a reused bar # never wrote its final newline (and never flushed) again. From 8c822886c785852443781d9158bcaab9511c9bed Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:12:50 +0200 Subject: [PATCH 03/15] perf(bar): cache _load_widgets() with functools.cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _load_widgets() called importlib.import_module on every invocation — once per full-bar render site (_format_widgets, default_widgets, prefix/suffix init). The module is immutable after first import, so wrap the helper in functools.cache: importlib resolves progressbar.widgets once on first use and reuses the object thereafter. Name/signature unchanged; the fast path still never calls _load_widgets() so it stays widgets-free (lazy-import tests remain green). --- progressbar/bar.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/progressbar/bar.py b/progressbar/bar.py index 90d4eedd..06fd148c 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -2,6 +2,7 @@ import abc import contextlib +import functools import importlib import itertools import logging @@ -42,12 +43,17 @@ _FastBarIterator = None +@functools.cache def _load_widgets() -> typing.Any: - """Import the widgets module lazily. + """Import the widgets module lazily (and once). The full-bar code needs ``widgets``, but the lean fast path must not pull it in (it drags the terminal/colour tables). Imported via importlib so the deferred load doesn't read as a static ``bar -> widgets`` import cycle. + + Cached with ``functools.cache`` so full-bar render sites don't pay the + ``import_module`` lookup on every call; the module object is resolved once + on first use and reused thereafter. """ return importlib.import_module('progressbar.widgets') From 007f5f1656db5ba6b32fd8a80d8314dc0e11e464 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:14:37 +0200 Subject: [PATCH 04/15] docs(fast): document _format_fast_line as an extension-point hook The '_format_fast_line ... Wired in a later task' comment described work that never happened and read as dead indirection. It is in fact a tested, supported extension point (test_fast_format_line_uses_native_hook monkeypatches it, and _format_line prefers it over the pure-Python formatter). Keep the hook and the fallback; replace the misleading comment with an accurate description of the optional native/custom formatter hook. Also annotate default_widgets(self) -> list[typing.Any]. --- progressbar/fast.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/progressbar/fast.py b/progressbar/fast.py index 65d029be..46e42aa5 100644 --- a/progressbar/fast.py +++ b/progressbar/fast.py @@ -8,8 +8,12 @@ base, ) -#: Optional native line formatter, provided by the `speedups` package. When -#: present it replaces the pure-Python formatter below. Wired in a later task. +#: Optional native line-formatter hook. Left as ``None`` so the pure-Python +#: ``_pure_format_fast_line`` below is used by default. When set to a callable +#: it takes precedence in :py:meth:`FastProgressBar._format_line`, letting the +#: ``speedups`` package — or any caller — swap in a faster/custom formatter. +#: This is a supported extension point, exercised by +#: ``test_fast_format_line_uses_native_hook``. _format_fast_line: typing.Callable[[FastProgressBar], str] | None = None #: Spinner frames cycled for unknown-length bars: bar, forward slash, dash, @@ -71,7 +75,7 @@ class FastProgressBar(bar_module.ProgressBar): render-cheap. Output stays close to the default look without the gradient. """ - def default_widgets(self) -> list: + def default_widgets(self) -> list[typing.Any]: # No widgets: the fixed formatter renders everything. return [] From 842b6d406959b20f4f9218c36d6803681bfdc301 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:16:51 +0200 Subject: [PATCH 05/15] refactor(bar): split ProgressBar.__init__ into focused helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The constructor mixed alias deprecation, widget copying, poll-interval setup, and variable seeding — enough to warrant a 'sourcery skip: low-code-quality' marker. Extract four private, single-purpose helpers and reduce __init__ to orchestration: - _apply_deprecated_aliases(): resolve maxval/poll DeprecationWarnings, returns the updated (max_value, poll_interval) - _copy_widgets(): deepcopy the copy-safe widgets into a fresh list - _setup_poll_intervals(): timedelta->seconds conversion and clamping - _seed_variables(): variables dict + VariableMixin widget-name scan Order of operations is unchanged, so construction behavior is identical. __init__'s signature is byte-identical (only the sourcery comment is dropped, no longer needed). --- progressbar/bar.py | 111 ++++++++++++++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 32 deletions(-) diff --git a/progressbar/bar.py b/progressbar/bar.py index 06fd148c..9d425400 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -685,26 +685,13 @@ def __init__( variables=None, min_poll_interval=None, **kwargs, - ): # sourcery skip: low-code-quality + ): """Initializes a progress bar with sane defaults.""" super().__init__(**kwargs) - if not max_value and kwargs.get('maxval') is not None: - warnings.warn( - 'The usage of `maxval` is deprecated, please use ' - '`max_value` instead', - DeprecationWarning, - stacklevel=1, - ) - max_value = kwargs.get('maxval') - if not poll_interval and kwargs.get('poll'): - warnings.warn( - 'The usage of `poll` is deprecated, please use ' - '`poll_interval` instead', - DeprecationWarning, - stacklevel=1, - ) - poll_interval = kwargs.get('poll') + max_value, poll_interval = self._apply_deprecated_aliases( + max_value, poll_interval, kwargs + ) if max_value and min_value > types.cast(NumberT, max_value): raise ValueError( @@ -716,13 +703,7 @@ def __init__( self.max_value = max_value # type: ignore self.max_error = max_error - # Only copy the widget if it's safe to copy. Most widgets are so we - # assume this to be true - self.widgets = [] - for widget in widgets or []: - if getattr(widget, 'copy', True): - widget = deepcopy(widget) - self.widgets.append(widget) + self.widgets = self._copy_widgets(widgets) self.prefix = prefix self.suffix = suffix @@ -734,13 +715,71 @@ def __init__( self.initial_start_time = kwargs.get('start_time') self.init() - # Convert a given timedelta to a floating point number as internal - # interval. We're not using timedelta's internally for two reasons: - # 1. Backwards compatibility (most important one) - # 2. Performance. Even though the amount of time it takes to compare a - # timedelta with a float versus a float directly is negligible, this - # comparison is run for _every_ update. With billions of updates - # (downloading a 1GiB file for example) this adds up. + self._setup_poll_intervals(poll_interval, min_poll_interval) + self._seed_variables(variables) + + def _apply_deprecated_aliases( + self, + max_value: ValueT, + poll_interval: types.Optional[float], + kwargs: types.Dict[str, typing.Any], + ) -> tuple[ValueT, types.Optional[float]]: + """Resolve the deprecated ``maxval``/``poll`` keyword aliases. + + Emits a :py:class:`DeprecationWarning` for each legacy name that is + used without its modern counterpart and returns the (possibly updated) + ``(max_value, poll_interval)`` pair. + """ + if not max_value and kwargs.get('maxval') is not None: + warnings.warn( + 'The usage of `maxval` is deprecated, please use ' + '`max_value` instead', + DeprecationWarning, + stacklevel=1, + ) + max_value = kwargs.get('maxval') + + if not poll_interval and kwargs.get('poll'): + warnings.warn( + 'The usage of `poll` is deprecated, please use ' + '`poll_interval` instead', + DeprecationWarning, + stacklevel=1, + ) + poll_interval = kwargs.get('poll') + + return max_value, poll_interval + + def _copy_widgets( + self, widgets: types.Optional[types.Sequence[typing.Any]] + ) -> list[typing.Any]: + """Return a fresh widget list, deep-copying the copy-safe widgets. + + Only copy a widget if it's safe to copy. Most widgets are, so that is + assumed to be true unless a widget opts out with ``copy = False``. + """ + result: list[typing.Any] = [] + for widget in widgets or []: + if getattr(widget, 'copy', True): + widget = deepcopy(widget) + result.append(widget) + return result + + def _setup_poll_intervals( + self, + poll_interval: types.Optional[float], + min_poll_interval: types.Optional[float], + ) -> None: + """Convert the poll intervals to seconds and clamp the minimum. + + Convert a given timedelta to a floating point number as the internal + interval. We're not using timedelta's internally for two reasons: + 1. Backwards compatibility (most important one) + 2. Performance. Even though the amount of time it takes to compare a + timedelta with a float versus a float directly is negligible, this + comparison is run for _every_ update. With billions of updates + (downloading a 1GiB file for example) this adds up. + """ poll_interval = utils.deltas_to_seconds(poll_interval, default=None) min_poll_interval = utils.deltas_to_seconds( min_poll_interval, @@ -760,7 +799,15 @@ def __init__( float(os.environ.get('PROGRESSBAR_MINIMUM_UPDATE_INTERVAL', 0)), ) # type: ignore - # A dictionary of names that can be used by Variable and FormatWidget + def _seed_variables( + self, variables: types.Optional[types.Dict[str, typing.Any]] + ) -> None: + """Seed the user-defined variables dict and scan widgets for names. + + Builds the ``variables`` mapping used by ``Variable``/``FormatWidget`` + and registers a ``None`` placeholder for every ``VariableMixin`` widget + whose name isn't already supplied. + """ self.variables = utils.AttributeDict(variables or {}) if self.widgets: widgets_module = _load_widgets() From 960657ece99412aee1683b15343abe8bfb47a432 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:22:23 +0200 Subject: [PATCH 06/15] test: make API snapshot order-independent under freezegun If an earlier test imports a module while freezegun is active, module constants like widgets.MAX_DATE are Fake* instances for the rest of the process; describe instances by their first non-freezegun MRO class so the snapshot no longer depends on test collection order. --- tests/test_api_surface.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_api_surface.py b/tests/test_api_surface.py index 24bd27b8..e7a775cc 100644 --- a/tests/test_api_surface.py +++ b/tests/test_api_surface.py @@ -104,7 +104,15 @@ def _describe(obj: typing.Any) -> str: return f'class{_describe_signature(obj)}' if callable(obj): return f'callable{_describe_signature(obj)}' - return type(obj).__name__ + # Describe instances by their first non-freezegun class: if an earlier + # test imported a module under freezegun, module constants like + # widgets.MAX_DATE are Fake* instances forever, which would make this + # snapshot order-dependent (FakeDate vs date). + return next( + cls.__name__ + for cls in type(obj).__mro__ + if 'freezegun' not in cls.__module__ + ) def _public_names(module: types.ModuleType) -> list[str]: From ca6553f0d6fb7dea92b5a0f87cb695a901745848 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:27:07 +0200 Subject: [PATCH 07/15] refactor(__main__): split main() into focused helpers Extract the input resolution, widget selection and copy loop out of the 125-line main() god function into three private single-purpose helpers: - _resolve_inputs(args, parser): stdin/file resolution + total-size detection - _build_widgets(args, filesize_available): widget-set selection - _transfer(bar, input_paths, output_stream, args, stack): the copy loop main() is now pure orchestration and passes flake8 C901 without the '# noqa: C901' suppression, which is removed. Behavior-preserving: CLI output is byte-identical and the pty-based command tests pass unchanged. --- progressbar/__main__.py | 246 ++++++++++++++++++++++++---------------- 1 file changed, 146 insertions(+), 100 deletions(-) diff --git a/progressbar/__main__.py b/progressbar/__main__.py index 59e4117c..a0cb6a3d 100644 --- a/progressbar/__main__.py +++ b/progressbar/__main__.py @@ -271,7 +271,7 @@ def create_argument_parser() -> argparse.ArgumentParser: return parser -def main(argv: list[str] | None = None) -> None: # noqa: C901 +def main(argv: list[str] | None = None) -> None: """ Main function for the `progressbar` command. @@ -289,56 +289,10 @@ def main(argv: list[str] | None = None) -> None: # noqa: C901 args.output, args.line_mode, stack ) - input_paths: list[BinaryIO | TextIO | Path | IO[typing.Any]] = [] - total_size: int = 0 - filesize_available: bool = True - for filename in args.input: - input_path: typing.IO[typing.Any] | pathlib.Path - if filename == '-': - if args.line_mode: - input_path = sys.stdin - else: - input_path = sys.stdin.buffer - - filesize_available = False - else: - input_path = pathlib.Path(filename) - if not input_path.exists(): - parser.error(f'File not found: {filename}') - - if not args.size: - total_size += input_path.stat().st_size - - input_paths.append(input_path) - - # Determine the size for the progress bar (if provided) - if args.size: - total_size = size_to_bytes(args.size) - filesize_available = True - - if filesize_available: - # Create the progress bar components - widgets = [ - progressbar.Percentage(), - ' ', - progressbar.Bar(), - ' ', - progressbar.Timer(), - ' ', - progressbar.FileTransferSpeed(), - ] - else: - widgets = [ - progressbar.SimpleProgress(), - ' ', - progressbar.DataSize(), - ' ', - progressbar.Timer(), - ] - - if args.eta: - widgets.append(' ') - widgets.append(progressbar.AdaptiveETA()) + input_paths, total_size, filesize_available = _resolve_inputs( + args, parser + ) + widgets = _build_widgets(args, filesize_available) # Initialize the progress bar bar = progressbar.ProgressBar( @@ -347,56 +301,148 @@ def main(argv: list[str] | None = None) -> None: # noqa: C901 max_error=False, ) - # Data processing and updating the progress bar - buffer_size = ( - size_to_bytes(args.buffer_size) if args.buffer_size else 1024 - ) - total_transferred = 0 - - bar.start() - with contextlib.suppress(KeyboardInterrupt, BrokenPipeError): - for input_path in input_paths: - if isinstance(input_path, pathlib.Path): - if args.line_mode: - # newline='' disables universal-newline - # translation so the byte count matches the file - # size for CRLF files as well - input_stream = stack.enter_context( - input_path.open('r', newline=''), - ) - else: - input_stream = stack.enter_context( - input_path.open('rb'), - ) + _transfer(bar, input_paths, output_stream, args, stack) + + +def _resolve_inputs( + args: argparse.Namespace, + parser: argparse.ArgumentParser, +) -> tuple[list[BinaryIO | TextIO | Path | IO[typing.Any]], int, bool]: + """ + Resolve the input arguments into concrete streams/paths and the total size. + + Returns the list of inputs (stdin streams or file paths), the total size in + bytes and whether that size is known (``filesize_available``). + """ + input_paths: list[BinaryIO | TextIO | Path | IO[typing.Any]] = [] + total_size: int = 0 + filesize_available: bool = True + for filename in args.input: + input_path: typing.IO[typing.Any] | pathlib.Path + if filename == '-': + if args.line_mode: + input_path = sys.stdin + else: + input_path = sys.stdin.buffer + + filesize_available = False + else: + input_path = pathlib.Path(filename) + if not input_path.exists(): + parser.error(f'File not found: {filename}') + + if not args.size: + total_size += input_path.stat().st_size + + input_paths.append(input_path) + + # An explicit ``--size`` overrides the detected file sizes entirely. + if args.size: + total_size = size_to_bytes(args.size) + filesize_available = True + + return input_paths, total_size, filesize_available + + +def _build_widgets( + args: argparse.Namespace, + filesize_available: bool, +) -> list[typing.Any]: + """ + Select the widget set for the progress bar. + + When the total size is known a percentage/bar layout is used, otherwise a + size-based layout is used. An adaptive ETA is appended when requested. + """ + widgets: list[typing.Any] + if filesize_available: + # Create the progress bar components + widgets = [ + progressbar.Percentage(), + ' ', + progressbar.Bar(), + ' ', + progressbar.Timer(), + ' ', + progressbar.FileTransferSpeed(), + ] + else: + widgets = [ + progressbar.SimpleProgress(), + ' ', + progressbar.DataSize(), + ' ', + progressbar.Timer(), + ] + + if args.eta: + widgets.append(' ') + widgets.append(progressbar.AdaptiveETA()) + + return widgets + + +def _transfer( + bar: progressbar.ProgressBar, + input_paths: list[BinaryIO | TextIO | Path | IO[typing.Any]], + output_stream: typing.IO[typing.Any], + args: argparse.Namespace, + stack: contextlib.ExitStack, +) -> None: + """ + Copy every input through the progress bar into ``output_stream``. + + Opened files are registered on ``stack`` so they are closed when the caller + exits its ``ExitStack`` context. + """ + # Data processing and updating the progress bar + buffer_size = size_to_bytes(args.buffer_size) if args.buffer_size else 1024 + total_transferred = 0 + + bar.start() + with contextlib.suppress(KeyboardInterrupt, BrokenPipeError): + for input_path in input_paths: + if isinstance(input_path, pathlib.Path): + if args.line_mode: + # newline='' disables universal-newline + # translation so the byte count matches the file + # size for CRLF files as well + input_stream = stack.enter_context( + input_path.open('r', newline=''), + ) else: - input_stream = input_path - - while True: - data: str | bytes - if args.line_mode: - data = input_stream.readline(buffer_size) - else: - data = input_stream.read(buffer_size) - - if not data: - break - - output_stream.write(data) - if isinstance(data, str): - # The total size is measured in bytes, so progress - # must be tracked in bytes as well - encoding = ( - getattr(input_stream, 'encoding', None) or 'utf-8' - ) - total_transferred += len( - data.encode(encoding, errors='replace'), - ) - else: - total_transferred += len(data) - - bar.update(total_transferred) - - bar.finish(dirty=True) + input_stream = stack.enter_context( + input_path.open('rb'), + ) + else: + input_stream = input_path + + while True: + data: str | bytes + if args.line_mode: + data = input_stream.readline(buffer_size) + else: + data = input_stream.read(buffer_size) + + if not data: + break + + output_stream.write(data) + if isinstance(data, str): + # The total size is measured in bytes, so progress + # must be tracked in bytes as well + encoding = ( + getattr(input_stream, 'encoding', None) or 'utf-8' + ) + total_transferred += len( + data.encode(encoding, errors='replace'), + ) + else: + total_transferred += len(data) + + bar.update(total_transferred) + + bar.finish(dirty=True) def _get_output_stream( From 3aaf4616fcd3e6ece07f047f9b9f50867d6782cd Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:32:07 +0200 Subject: [PATCH 08/15] refactor(env): split ColorSupport.from_env into per-source helpers from_env() mixed three unrelated detection strategies inline. Extract each into a focused classmethod, leaving from_env() as a precedence-ordered dispatcher: - _from_jupyter(): interactive-kernel true-color shortcut - _from_windows(): Windows console-mode probe - _from_term_variables(variables): terminal env-var depth scan Behavior-preserving: the JUPYTER -> Windows -> env-scan precedence and the subtle scan semantics (first truecolor/24bit wins via break; otherwise the highest depth wins via max(); a generic truthy FORCE_COLOR=1 returns full color) are unchanged. The 'elif os.name == "nt"' dispatch line keeps the existing coverage exclusion; _from_windows() carries the '# pragma: no cover' that previously sat on the Windows block (unreachable on non-Windows CI). --- progressbar/env.py | 58 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/progressbar/env.py b/progressbar/env.py index 225dc23f..ef6514c9 100644 --- a/progressbar/env.py +++ b/progressbar/env.py @@ -62,23 +62,50 @@ def from_env(cls) -> ColorSupport: 'TERM', ) + # Precedence order is significant: an interactive Jupyter kernel and + # the Windows console probe each take priority over (and short-circuit) + # the env-var scan below. if JUPYTER: - # Jupyter notebook always supports true color. - return cls.XTERM_TRUECOLOR + return cls._from_jupyter() elif os.name == 'nt': - # We can't reliably detect true color support on Windows, so we - # will assume it is supported if the console is configured to - # support it. - from .terminal.os_specific import windows - - if ( - windows.get_console_mode() - & windows.WindowsConsoleModeFlags.ENABLE_PROCESSED_OUTPUT - ): - return cls.XTERM_TRUECOLOR - else: - return cls.WINDOWS # pragma: no cover + return cls._from_windows() + + return cls._from_term_variables(variables) + + @classmethod + def _from_jupyter(cls) -> ColorSupport: + """Jupyter notebooks always support true color.""" + return cls.XTERM_TRUECOLOR + + @classmethod + def _from_windows(cls) -> ColorSupport: # pragma: no cover + """Detect color support from the Windows console mode. + We can't reliably detect true color support on Windows, so we assume + it is supported when the console is configured to support it. + """ + from .terminal.os_specific import windows + + if ( + windows.get_console_mode() + & windows.WindowsConsoleModeFlags.ENABLE_PROCESSED_OUTPUT + ): + return cls.XTERM_TRUECOLOR + else: + return cls.WINDOWS + + @classmethod + def _from_term_variables( + cls, + variables: tuple[str, ...], + ) -> ColorSupport: + """Pick the highest color support advertised by the terminal env vars. + + The first `truecolor`/`24bit` value wins immediately; otherwise the + highest depth seen across all variables is returned. A generic truthy + flag such as `FORCE_COLOR=1` carries no depth and implies full color + support, analogous to the Jupyter handling above. + """ support = cls.NONE for variable in variables: value = os.environ.get(variable) @@ -93,9 +120,6 @@ def from_env(cls) -> ColorSupport: elif value == 'xterm': support = max(cls.XTERM, support) elif env_flag(variable, default=False): - # Generic truthy flags such as `FORCE_COLOR=1` enable - # color support but don't specify the depth; assume full - # color support analogous to the Jupyter handling above. return cls.XTERM_TRUECOLOR return support From 59b3cb24280e34b6c18a61aad1f44a3eeedaae19 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:39:44 +0200 Subject: [PATCH 09/15] refactor(widgets): dedup ETA value/elapsed + make UnknownLength explicit ETA.__call__ and SmoothingETA.__call__ shared an identical value/elapsed default-resolution preamble. Extract it into ETA._resolve_value_elapsed() and call it from both; the shared helper's branches are covered by the ETA paths so SmoothingETA no longer needs its two '# pragma: no branch'. Also replace the TypeError-as-control-flow around _calculate_eta with an explicit 'progress.max_value is base.UnknownLength' check. The only source of that TypeError was subtracting the value from the UnknownLength sentinel inside _calculate_eta's 'if elapsed:' branch, so the explicit check is guarded by 'elapsed' too: this keeps the elapsed==0 case returning 0 (format_zero) instead of N/A, preserving byte-identical output. Nothing else in the ETA math raises TypeError, so the try/except is removed entirely rather than kept as a defensive handler. Behavior-preserving: full suite + render goldens unchanged, widgets.py at 100% branch coverage. --- progressbar/widgets.py | 54 ++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/progressbar/widgets.py b/progressbar/widgets.py index 47feebf2..2b18efd7 100644 --- a/progressbar/widgets.py +++ b/progressbar/widgets.py @@ -569,33 +569,56 @@ def _calculate_eta( else: return 0 - def __call__( + def _resolve_value_elapsed( self, progress: ProgressBarMixinBase, data: Data, - value=None, - elapsed=None, + value, + elapsed, ): - """Updates the widget to show the ETA or total time when finished.""" + """Fill in the value/elapsed defaults shared by the ETA variants. + + When a caller does not supply them, the per-item rate is based on the + progress relative to ``min_value`` (not the raw value) and the elapsed + time is taken from the data snapshot. + """ if value is None: - # The per-item rate must be based on the progress relative to - # min_value, not the raw value value = data['value'] - progress.min_value if elapsed is None: elapsed = data['time_elapsed'] + return value, elapsed + + def __call__( + self, + progress: ProgressBarMixinBase, + data: Data, + value=None, + elapsed=None, + ): + """Updates the widget to show the ETA or total time when finished.""" + value, elapsed = self._resolve_value_elapsed( + progress, data, value, elapsed + ) + + # ``max_value`` is ``UnknownLength`` for indeterminate bars. The + # remaining-count subtraction in ``_calculate_eta`` only runs (and + # only then fails) once ``elapsed`` is truthy, so guard on both to + # keep the ``elapsed == 0`` case rendering ``format_zero`` as before. + # Nothing else in the ETA math raises ``TypeError``, so the previous + # try/except-as-control-flow is intentionally removed. eta_na = False - try: + if elapsed and progress.max_value is base.UnknownLength: + data['eta_seconds'] = None + eta_na = True + else: data['eta_seconds'] = self._calculate_eta( progress, data, value=value, elapsed=elapsed, ) - except TypeError: - data['eta_seconds'] = None - eta_na = True data['eta'] = None if data['eta_seconds']: @@ -722,14 +745,9 @@ def __call__( value=None, elapsed=None, ): - if value is None: # pragma: no branch - # The per-item rate must be based on the progress relative to - # min_value, not the raw value - value = data['value'] - progress.min_value - - if elapsed is None: # pragma: no branch - elapsed = data['time_elapsed'] - + value, elapsed = self._resolve_value_elapsed( + progress, data, value, elapsed + ) value = self.smoothing_algorithm.update(value, elapsed) return ETA.__call__(self, progress, data, value=value, elapsed=elapsed) From 5927aae5dd266585e1fba06f7b76eb64d9e92055 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 04:41:36 +0200 Subject: [PATCH 10/15] refactor(widgets): extract Bar._render_borders for the shared border preamble The identical left/right border resolution (resolve callables, subtract their visible length from width) was copy-pasted across five bar __call__ methods. Extract it into Bar._render_borders(progress, data, width) -> (left, right, remaining_width) and call it from Bar, BouncingBar, MultiRangeBar and JobStatusBar (all Bar subclasses). GranularBar descends from AutoWidthWidgetBase (not Bar) so it can't reach the helper; rather than hoisting border concerns onto a shared width-widget base (a responsibility smell affecting widgets that have no borders) or changing any MRO, its copy is kept with an explanatory comment. Behavior-preserving: widget render output byte-identical, full suite + render goldens pass, widgets.py at 100% branch coverage. --- progressbar/widgets.py | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/progressbar/widgets.py b/progressbar/widgets.py index 2b18efd7..5f7bc98a 100644 --- a/progressbar/widgets.py +++ b/progressbar/widgets.py @@ -1114,6 +1114,23 @@ def __init__( super().__init__(**kwargs) + def _render_borders( + self, + progress: ProgressBarMixinBase, + data: Data, + width: int, + ) -> tuple[str, str, int]: + """Resolve the left/right borders and the width left for the body. + + The borders may be callables, so they are resolved against + ``progress``/``data`` and their visible length subtracted from + ``width``. Shared by every :class:`Bar` subclass' ``__call__``. + """ + left = converters.to_unicode(self.left(progress, data, width)) + right = converters.to_unicode(self.right(progress, data, width)) + width -= progress.custom_len(left) + progress.custom_len(right) + return left, right, width + def __call__( self, progress: ProgressBarMixinBase, @@ -1122,9 +1139,7 @@ def __call__( color=True, ): """Updates the progress bar and its subcomponents.""" - left = converters.to_unicode(self.left(progress, data, width)) - right = converters.to_unicode(self.right(progress, data, width)) - width -= progress.custom_len(left) + progress.custom_len(right) + left, right, width = self._render_borders(progress, data, width) marker = converters.to_unicode(self.marker(progress, data, width)) fill = converters.to_unicode(self.fill(progress, data, width)) @@ -1185,9 +1200,7 @@ def __call__( color=True, ): """Updates the progress bar and its subcomponents.""" - left = converters.to_unicode(self.left(progress, data, width)) - right = converters.to_unicode(self.right(progress, data, width)) - width -= progress.custom_len(left) + progress.custom_len(right) + left, right, width = self._render_borders(progress, data, width) marker = converters.to_unicode(self.marker(progress, data, width)) fill = converters.to_unicode(self.fill(progress, data, width)) @@ -1285,9 +1298,7 @@ def __call__( color=True, ): """Updates the progress bar and its subcomponents.""" - left = converters.to_unicode(self.left(progress, data, width)) - right = converters.to_unicode(self.right(progress, data, width)) - width -= progress.custom_len(left) + progress.custom_len(right) + left, right, width = self._render_borders(progress, data, width) values = self.get_values(progress, data) values_sum = sum(values) @@ -1414,6 +1425,10 @@ def __call__( data: Data, width: int = 0, ): + # GranularBar descends from AutoWidthWidgetBase, not Bar, so it can't + # reach Bar._render_borders. The border preamble is intentionally + # duplicated here rather than hoisting the helper onto a shared base + # (which would put border concerns on width widgets that have none). left = converters.to_unicode(self.left(progress, data, width)) right = converters.to_unicode(self.right(progress, data, width)) width -= progress.custom_len(left) + progress.custom_len(right) @@ -1682,9 +1697,7 @@ def __call__( width: int = 0, color=True, ): - left = converters.to_unicode(self.left(progress, data, width)) - right = converters.to_unicode(self.right(progress, data, width)) - width -= progress.custom_len(left) + progress.custom_len(right) + left, right, width = self._render_borders(progress, data, width) status: str | bool | None = data['variables'].get(self.name) From ee38c2fc16def7f9740b1a46624d6f1d6ee09a0d Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 05:04:05 +0200 Subject: [PATCH 11/15] fix(__main__): keep finish(dirty=True) inside the suppress block The _transfer extraction moved it outside, so BrokenPipeError from a piped stderr could crash shutdown (flagged by review on #321); restore the historical placement. --- progressbar/__main__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/progressbar/__main__.py b/progressbar/__main__.py index a0cb6a3d..a59f7d53 100644 --- a/progressbar/__main__.py +++ b/progressbar/__main__.py @@ -442,7 +442,10 @@ def _transfer( bar.update(total_transferred) - bar.finish(dirty=True) + # Inside the suppress block on purpose (matching the historical + # behavior): on interrupt/broken pipe the finish is skipped and a + # BrokenPipeError from a closed stderr cannot crash shutdown. + bar.finish(dirty=True) def _get_output_stream( From 0b7cf5a32e9c0a4b26a405afe20226083c18e96d Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 05:04:06 +0200 Subject: [PATCH 12/15] fix(bar): set timing state before _started becomes observable A MultiBar render thread that sees started() True calls update(force=True), and update() re-enters start() while start_time is None -- double-running the stream-capturing path. Populate start_time/last_update_time before the cooperative start() dispatch flips _started (flagged by review on #321); flip-observation test extended to assert it. --- progressbar/bar.py | 14 ++++++++++---- tests/test_multibar.py | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/progressbar/bar.py b/progressbar/bar.py index 9d425400..ab996f7b 100644 --- a/progressbar/bar.py +++ b/progressbar/bar.py @@ -1383,6 +1383,16 @@ def start( self._gate_enabled = False self._verify_max_value() + # Timing state must be populated before `_started` becomes + # observable: a concurrent reader (MultiBar's render thread) that + # sees `started()` True calls `update(force=True)`, and `update()` + # re-enters `start()` whenever `start_time` is still None -- running + # the stream-capturing path twice. + now = datetime.now() + self.start_time = self.initial_start_time or now + self.last_update_time = now + self._last_update_timer = timeit.default_timer() + # Cooperative dispatch through the MRO # (StdRedirectMixin -> DefaultFdMixin -> ProgressBarMixinBase); # ResizableMixin/ProgressBarBase define no `start` and are skipped. @@ -1394,10 +1404,6 @@ def start( # still happens at the same point, after stream/console setup. super().start(max_value=max_value) - now = datetime.now() - self.start_time = self.initial_start_time or now - self.last_update_time = now - self._last_update_timer = timeit.default_timer() self.update(self.min_value, force=True) return self diff --git a/tests/test_multibar.py b/tests/test_multibar.py index b70902a1..08ca1ea1 100644 --- a/tests/test_multibar.py +++ b/tests/test_multibar.py @@ -185,6 +185,9 @@ def recording_start(self, **kwargs): # list is already populated at this exact moment. observed['widgets_at_flip'] = bool(self.widgets) observed['started_at_flip'] = self.started() + # update() re-enters start() while start_time is None, so a + # concurrent update(force=True) would double-run the start path. + observed['start_time_at_flip'] = self.start_time is not None return result monkeypatch.setattr( @@ -198,6 +201,9 @@ def recording_start(self, **kwargs): assert observed.get('widgets_at_flip') is True, ( 'widgets must be populated before started() can observe _started' ) + assert observed.get('start_time_at_flip') is True, ( + 'start_time must be set before started() can observe _started' + ) def test_multibar_print() -> None: From 1fa6bd72adc96cf033d2d6728841d8b3377ca845 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 05:04:06 +0200 Subject: [PATCH 13/15] fix(widgets): treat max_value=None as unknown in ETA guard The explicit UnknownLength check replaced a broad try/except TypeError; None is also a legitimate indeterminate max_value and must take the N/A path (flagged by review on #321). --- progressbar/widgets.py | 17 ++++++++++------- tests/test_timed.py | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/progressbar/widgets.py b/progressbar/widgets.py index 5f7bc98a..147f6cf7 100644 --- a/progressbar/widgets.py +++ b/progressbar/widgets.py @@ -602,14 +602,17 @@ def __call__( progress, data, value, elapsed ) - # ``max_value`` is ``UnknownLength`` for indeterminate bars. The - # remaining-count subtraction in ``_calculate_eta`` only runs (and - # only then fails) once ``elapsed`` is truthy, so guard on both to - # keep the ``elapsed == 0`` case rendering ``format_zero`` as before. - # Nothing else in the ETA math raises ``TypeError``, so the previous - # try/except-as-control-flow is intentionally removed. + # ``max_value`` is ``UnknownLength`` (or ``None``) for indeterminate + # bars. The remaining-count subtraction in ``_calculate_eta`` only + # runs (and only then fails) once ``elapsed`` is truthy, so guard on + # both to keep the ``elapsed == 0`` case rendering ``format_zero`` as + # before. Nothing else in the ETA math raises ``TypeError``, so the + # previous try/except-as-control-flow is intentionally removed. eta_na = False - if elapsed and progress.max_value is base.UnknownLength: + if elapsed and ( + progress.max_value is None + or progress.max_value is base.UnknownLength + ): data['eta_seconds'] = None eta_na = True else: diff --git a/tests/test_timed.py b/tests/test_timed.py index ee19ab95..0eb17745 100644 --- a/tests/test_timed.py +++ b/tests/test_timed.py @@ -185,3 +185,26 @@ def gen(): bar = progressbar.ProgressBar(widgets=widgets) for _i in bar(gen()): pass + + +def test_eta_with_none_max_value() -> None: + # Regression (#321 review): the explicit UnknownLength guard replaced a + # broad try/except TypeError; ``max_value`` may also legitimately be + # ``None`` for indeterminate bars and must take the N/A path rather + # than crashing on the remaining-count subtraction. + import io + + widget = progressbar.ETA() + bar = progressbar.ProgressBar( + widgets=[widget], + max_value=progressbar.UnknownLength, + fd=io.StringIO(), + term_width=60, + ) + bar.start() + bar.update(1) + data = bar.data() + data['time_elapsed'] = datetime.timedelta(seconds=5) + bar.max_value = None + assert 'N/A' in widget(bar, data) + bar.finish(dirty=True) From c6f75b195a39754368be25a6c740e02f7dc7c82f Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 05:04:06 +0200 Subject: [PATCH 14/15] test: harden MRO module check against non-string __module__ --- tests/test_api_surface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api_surface.py b/tests/test_api_surface.py index e7a775cc..18df63b9 100644 --- a/tests/test_api_surface.py +++ b/tests/test_api_surface.py @@ -111,7 +111,7 @@ def _describe(obj: typing.Any) -> str: return next( cls.__name__ for cls in type(obj).__mro__ - if 'freezegun' not in cls.__module__ + if 'freezegun' not in (getattr(cls, '__module__', '') or '') ) From 39c09cf66f8e85ee4ad2bb227b08f79a55d5eee8 Mon Sep 17 00:00:00 2001 From: Rick van Hattem Date: Mon, 6 Jul 2026 05:04:06 +0200 Subject: [PATCH 15/15] test: single import style for progressbar.bar (CodeQL) --- tests/test_progressbar.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_progressbar.py b/tests/test_progressbar.py index 376cbd60..e8de2c09 100644 --- a/tests/test_progressbar.py +++ b/tests/test_progressbar.py @@ -200,15 +200,15 @@ def write(self, value: str) -> int: def test_sigwinch_restored_with_overlapping_bars() -> None: # Regression: A5 - with two live bars, finishing them in creation # order left a dangling handler installed. - from progressbar.bar import _ResizeRegistry + import progressbar.bar as bar_module saved_handler = signal.getsignal(signal.SIGWINCH) # Isolate the global registry so the assertions don't depend on bars # left registered (and a handler left installed) by other tests - saved_bars = list(_ResizeRegistry.bars) - saved_prev = _ResizeRegistry.previous_handler - _ResizeRegistry.bars.clear() - _ResizeRegistry.previous_handler = None + saved_bars = list(bar_module._ResizeRegistry.bars) + saved_prev = bar_module._ResizeRegistry.previous_handler + bar_module._ResizeRegistry.bars.clear() + bar_module._ResizeRegistry.previous_handler = None # Start from a known sentinel handler so we can tell apart "still # installed" from "restored" without depending on global state @@ -238,6 +238,6 @@ def test_sigwinch_restored_with_overlapping_bars() -> None: assert signal.getsignal(signal.SIGWINCH) is signal.SIG_IGN finally: for restored_bar in saved_bars: - _ResizeRegistry.bars.add(restored_bar) - _ResizeRegistry.previous_handler = saved_prev + bar_module._ResizeRegistry.bars.add(restored_bar) + bar_module._ResizeRegistry.previous_handler = saved_prev signal.signal(signal.SIGWINCH, saved_handler)