Repository: pallets/click
Pull request #1354 (2019-07-13 02:23:01)

Fix ProgressBar item_show_func so it displays the current item
Per @davidism's request this PR contains my suggested fix for `ProgressBar.generator()`. In short, this change makes the `item_show_func` show the item currently being processed as opposed to the item previously processed (like it currently does). I've explained the bug more fully in #1353.

You can use this example to see the difference when run against current master vs this PR.
```python
from time import sleep
from click import progressbar


def format_item(item):
    if item:
        return "processing {}".format(item)


with progressbar(range(5), item_show_func=format_item) as pb:
    for item in pb:
        print(" actually on", item)
        sleep(1)
```

Thanks in advance for taking the time to look into this.

davidism: This seems incompatible the changes from #1698 which added an update interval. That PR relies on `update()`, but this one bypasses that and unconditionally calls `make_step()` and `render_progress()`. However, moving `update()` before `yield` means that the ETA calculation is wrong. Incidentally, that PR is when I decided that the progress bar was getting too complex without enough design oversight and started recommending tqdm.
davidism: #1803 to stop hiding fast progress bars seems to have inadvertently mostly fixed this too. "processing" matches "actual" now, although it's not displayed for the first item (0).
davidism: Regarding the ETA issue with this fix, this fix doesn't match the docs:

> Note that progressbar() updates the bar after each iteration of the loop. So code like this will render correctly:
> 
> ```python
> import time
> 
> with click.progressbar([1, 2, 3]) as bar:
>     for x in bar:
>         print('sleep({})...'.format(x))
>         time.sleep(x)
> ```

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 6d596b5..c1649ce 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -178,6 +178,8 @@ Unreleased
     is not a TTY. :issue:`1138`
 -   Progress bar output is shown even if execution time is less than 0.5
     seconds. :issue:`1648`
+-   Progress bar ``item_show_func`` shows the current item, not the
+    previous item. :issue:`1353`
 
 
 Version 7.1.2
diff --git a/src/click/_termui_impl.py b/src/click/_termui_impl.py
index c1cc936..3c7d57f 100644
--- a/src/click/_termui_impl.py
+++ b/src/click/_termui_impl.py
@@ -302,14 +302,13 @@ class ProgressBar:
             Only render when the number of steps meets the
             ``update_min_steps`` threshold.
         """
+        if current_item is not None:
+            self.current_item = current_item
+
         self._completed_intervals += n_steps
 
         if self._completed_intervals >= self.update_min_steps:
             self.make_step(self._completed_intervals)
-
-            if current_item is not None:
-                self.current_item = current_item
-
             self.render_progress()
             self._completed_intervals = 0
 
@@ -338,8 +337,16 @@ class ProgressBar:
         else:
             for rv in self.iter:
                 self.current_item = rv
+
+                # This allows show_item_func to be updated before the
+                # item is processed. Only trigger at the beginning of
+                # the update interval.
+                if self._completed_intervals == 0:
+                    self.render_progress()
+
                 yield rv
                 self.update(1)
+
             self.finish()
             self.render_progress()
 
diff --git a/src/click/termui.py b/src/click/termui.py
index 4fc7229..4ea76d8 100644
--- a/src/click/termui.py
+++ b/src/click/termui.py
@@ -366,10 +366,10 @@ def progressbar(
                          `False` if not.
     :param show_pos: enables or disables the absolute position display.  The
                      default is `False`.
-    :param item_show_func: a function called with the current item which
-                           can return a string to show the current item
-                           next to the progress bar.  Note that the current
-                           item can be `None`!
+    :param item_show_func: A function called with the current item which
+        can return a string to show next to the progress bar. If the
+        function returns ``None`` nothing is shown. The current item can
+        be ``None``, such as when entering and exiting the bar.
     :param fill_char: the character to use to show the filled part of the
                       progress bar.
     :param empty_char: the character to use to show the non-filled part of
@@ -393,6 +393,9 @@ def progressbar(
     .. versionchanged:: 8.0
         Output is shown even if execution time is less than 0.5 seconds.
 
+    .. versionchanged:: 8.0
+        ``item_show_func`` shows the current item, not the previous one.
+
     .. versionchanged:: 8.0
         Labels are echoed if the output is not a TTY. Reverts a change
         in 7.0 that removed all output.


Test patch:
diff --git a/tests/test_termui.py b/tests/test_termui.py
index da586af..05e3bf8 100644
--- a/tests/test_termui.py
+++ b/tests/test_termui.py
@@ -284,22 +284,19 @@ def test_progressbar_update(runner, monkeypatch):
 
 
 def test_progressbar_item_show_func(runner, monkeypatch):
+    """item_show_func should show the current item being yielded."""
+
     @click.command()
     def cli():
-        with click.progressbar(
-            range(3), item_show_func=lambda x: f"Custom {x}"
-        ) as progress:
-            for _ in progress:
-                click.echo()
+        with click.progressbar(range(3), item_show_func=lambda x: str(x)) as progress:
+            for item in progress:
+                click.echo(f" item {item}")
 
     monkeypatch.setattr(click._termui_impl, "isatty", lambda _: True)
-    output = runner.invoke(cli, []).output
-    lines = [line for line in output.splitlines() if "[" in line]
-    assert "Custom None" in lines[0]
-    assert "Custom 0" in lines[1]
-    assert "Custom 1" in lines[2]
-    assert "Custom 2" in lines[3]
-    assert "Custom None" in lines[4]
+    lines = runner.invoke(cli).output.splitlines()
+
+    for i, line in enumerate(x for x in lines if "item" in x):
+        assert f"{i}    item {i}" in line
 
 
 def test_progressbar_update_with_item_show_func(runner, monkeypatch):