bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]


When evaluating this form

        (dolist-with-progress-reporter (i (number-sequence 1 4))
            "Doing stuff"
          (sleep-for 1))

I would expect to get the messages

Doing stuff...
Doing stuff...25%
Doing stuff...50%
Doing stuff...75%
Doing stuff...Done

each update after one argument of the list has been handled. Instead, it
seems I get

Doing stuff...
Doing stuff...33%
Doing stuff...66%
Doing stuff...100%
Doing stuff...Done

This confuses me, because after "processing" one out of four elements, I
wouldn't expect the message to say I already finished a third. And on
the other side, I am still not done after reaching 100%, and in this
case, am still waiting for the last element to be processed.

I am not sure if this is intentional, but in case it is no, the
following patch should fix it:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-dolist-with-progress-reporter-behaviour.patch --]
[-- Type: text/x-diff, Size: 945 bytes --]

From f7da2585886bd4fd795713a605ff6bd17a4c337a Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Mon, 12 Jul 2021 17:26:43 +0200
Subject: [PATCH] Fix dolist-with-progress-reporter behaviour

* subr.el (dolist-with-progress-reporter): Use the length of list
  argument as maximal value the reporter with reach
---
 lisp/subr.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index e49c277335..20f40be40d 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -5880,7 +5880,7 @@ dolist-with-progress-reporter
            (,count 0)
            (,list ,(cadr spec)))
        (when (stringp ,prep)
-         (setq ,prep (make-progress-reporter ,prep 0 (1- (length ,list)))))
+         (setq ,prep (make-progress-reporter ,prep 0 (length ,list))))
        (dolist (,(car spec) ,list)
          ,@body
          (progress-reporter-update ,prep (setq ,count (1+ ,count))))
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 8010 bytes --]




In GNU Emacs 28.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-06-20 built on icterid
Repository revision: 869db473cbd510270faec2ba43dd4a5ba10b0020
Repository branch: feature/rcirc-update
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $EMACSLOADPATH: 
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  TeX-PDF-mode: t
  paredit-mode: t
  flymake-mode: t
  flyspell-mode: t
  outline-minor-mode: t
  rcirc-track-minor-mode: t
  display-time-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  winner-mode: t
  electric-pair-mode: t
  recentf-mode: t
  save-place-mode: t
  savehist-mode: t
  show-paren-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/philip/.config/emacs/elpa/transient-0.3.6/transient hides /home/philip/Code/src/emacs/lisp/transient

Features:
(shadow emacsbug sh-script executable ps-mode descr-text allout locate
grep windmove magit-ediff ediff ediff-merg ediff-mult ediff-wind
ediff-diff ediff-help ediff-init ediff-util preview reporter desktop
frameset tex-buf tex-fold reftex-dcr reftex-auc reftex reftex-loaddefs
reftex-vars font-latex latex latex-flymake tex-ispell tex-style tex
texmathp tex-mode latexenc org-goto tramp-sshfs tramp-smb tramp-rclone
tramp-fuse tramp-gvfs zeroconf tramp-crypt tramp-adb ibuffer
ibuffer-loaddefs git-rebase tramp-cmds gnus-draft vc-annotate em-alias
em-basic em-dirs esh-var em-hist em-ls em-script em-tramp em-unix
em-cmpl em-glob em-pred em-prompt em-rebind em-smart esh-mode em-term
two-column debug ps-mule mhtml-mode css-mode smie js sgml-mode htmlize
cl url-http url-gw url-cache url-auth display-line-numbers cal-move
calc-forms calc-bin calc-frac calc-lang calc-mode calc-undo calc-math
calcalg2 calccomp calc-arith calc-misc calc-alg calc-ext calc-aent
calc-menu calc calc-loaddefs calc-macs whitespace hi-lock ffap apropos
facemenu modus-vivendi-theme help-at-pt tabify man gdb-mi gud cl-print
edebug backtrace shortdoc yasnippet-snippets yasnippet help-fns avy
tramp-cache tramp-sh pulse view reposition flymake-cc macrostep-c
cmacexp macrostep cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-langs cc-vars cc-defs cc-bytecomp org-clock
cus-edit cus-start finder-inf char-fold misearch multi-isearch
shell-command+ markdown-mode color mailalias bbdb-pgp bbdb-message
autocrypt-message flow-fill mm-archive qp magit-extras magit-patch
goto-addr bug-reference dired-aux pp vc-mtn vc-hg vc-bzr vc-src vc-sccs
vc-cvs vc-rcs mule-util sort smiley gnus-cite mail-extr gnus-bcklg
org-duration diary-lib diary-loaddefs cal-iso ol-eww eww xdg url-queue
mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search
eieio-opt speedbar ezimage dframe ol-docview ol-bibtex bibtex ol-bbdb
ol-w3m face-remap org-agenda org-refile gnus-async gnus-ml
autocrypt-gnus autocrypt nndraft nnmh epa-file gnutls network-stream nsm
nnmaildir nnfolder bbdb-gnus bbdb-mua bbdb-com nnnil gnus-agent
gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu
mml2015 mm-view mml-smime smime dig nntp gnus-cache gnus-sum shr kinsoku
svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud
nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range
gnus-win time-stamp quail vc-backup log-view vc-fossil vc make-mode
sp-tutor doc-view jka-compr image-mode exif ps-print ps-print-loaddefs
ps-def lpr vc-svn ox-odt rng-loc rng-uri rng-parse rng-match rng-dt
rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util ox-latex
ox-icalendar ox-html table ox-ascii ox-publish ox org-element org ob
ob-tangle ob-ref ob-lob ob-table org-macro org-footnote org-src
ob-comint org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp org-table org-keys org-loaddefs find-func avl-tree ol
ob-exp ob-core org-compat advice ob-eval org-macs waffel hl-line tramp
tramp-loaddefs trampver tramp-integration tramp-compat ls-lisp etags
fileloop generator xref project paredit checkdoc flymake-proc flymake
thingatpt flyspell ispell noutline outline dired-x rcirc parse-time
iso8601 time bbdb bbdb-site timezone sendmail gnus nnheader
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode
diff git-commit log-edit message rmc puny rfc822 mml mml-sec epa epg
epg-config gnus-util rmail rmail-loaddefs time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor term disp-table ehelp eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util dired dired-loaddefs shell pcomplete server magit-mode
transient format-spec magit-git magit-section comp comp-cstr warnings
cl-extra help-mode derived benchmark magit-utils which-func imenu vc-git
diff-mode vc-dispatcher ido crm dash hippie-exp winner elec-pair recentf
tree-widget wid-edit saveplace savehist paren modus-operandi-theme
modus-themes edmacro kmacro holidays hol-loaddefs cal-menu calendar
cal-loaddefs cus-load setup load compile easy-mmode files-x pcase rx
text-property-search comint ansi-color ring autoload radix-tree lisp-mnt
tex-site slime-autoloads info package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util mailcap url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 2097065 1884485)
 (symbols 48 83251 983)
 (strings 32 372750 132457)
 (string-bytes 1 11364694)
 (vectors 16 144289)
 (vector-slots 8 3652510 1692147)
 (floats 8 1786 6127)
 (intervals 56 83336 56698)
 (buffers 992 157))

-- 
	Philip K.

bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

[Michael Heerdegen]

Philip Kaludercic <philipk@posteo.net> writes:

> I am not sure if this is intentional

Looks more like a typical problem with counting...

> but in case it is no, the following patch should fix it:
>
> From f7da2585886bd4fd795713a605ff6bd17a4c337a Mon Sep 17 00:00:00 2001
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Mon, 12 Jul 2021 17:26:43 +0200
> Subject: [PATCH] Fix dolist-with-progress-reporter behaviour
>
> * subr.el (dolist-with-progress-reporter): Use the length of list
>   argument as maximal value the reporter with reach
> ---
>  lisp/subr.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index e49c277335..20f40be40d 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -5880,7 +5880,7 @@ dolist-with-progress-reporter
>             (,count 0)
>             (,list ,(cadr spec)))
>         (when (stringp ,prep)
> -         (setq ,prep (make-progress-reporter ,prep 0 (1- (length ,list)))))
> +         (setq ,prep (make-progress-reporter ,prep 0 (length ,list))))

I think this patch is correct: 0 stands for 0% finished, and we have
finished after processing 100% of the list's elements, whose number is
(length list).  With other words: we have (+ 1 (length list)) steps: one
after having processed each element, plus the one before having started.

Are you able to install the patch, or does someone else have to do it?


Thanks,

Michael.

bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

[Philip Kaludercic]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> I am not sure if this is intentional
>
> Looks more like a typical problem with counting...
>
>> but in case it is no, the following patch should fix it:
>>
>> From f7da2585886bd4fd795713a605ff6bd17a4c337a Mon Sep 17 00:00:00 2001
>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Mon, 12 Jul 2021 17:26:43 +0200
>> Subject: [PATCH] Fix dolist-with-progress-reporter behaviour
>>
>> * subr.el (dolist-with-progress-reporter): Use the length of list
>>   argument as maximal value the reporter with reach
>> ---
>>  lisp/subr.el | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lisp/subr.el b/lisp/subr.el
>> index e49c277335..20f40be40d 100644
>> --- a/lisp/subr.el
>> +++ b/lisp/subr.el
>> @@ -5880,7 +5880,7 @@ dolist-with-progress-reporter
>>             (,count 0)
>>             (,list ,(cadr spec)))
>>         (when (stringp ,prep)
>> -         (setq ,prep (make-progress-reporter ,prep 0 (1- (length ,list)))))
>> +         (setq ,prep (make-progress-reporter ,prep 0 (length ,list))))
>
> I think this patch is correct: 0 stands for 0% finished, and we have
> finished after processing 100% of the list's elements, whose number is
> (length list).  With other words: we have (+ 1 (length list)) steps: one
> after having processed each element, plus the one before having started.
>
> Are you able to install the patch, or does someone else have to do it?

I could push it, if nobody objects.

> Thanks,
>
> Michael.

-- 
	Philip Kaludercic

bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> I could push it, if nobody objects.

Looks like the right fix to me, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49536: 28.0.50; Confusing dolist-with-progress-reporter behaviour

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> I could push it, if nobody objects.
>
> Looks like the right fix to me, too.

Looks like this was pushed at the time, but the bug report was left
open, so I'm closing it now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no