[ELPA] New package: progress-bar

[ELPA] New package: progress-bar

[Mariano Montone]

Hello,

I would like to propose adding this package to ELPA: 
https://github.com/mmontone/emacs-progress-bar/

progress-bar is a progress bar that is displayed in the echo area.

Please let me know if you accept, and what would be the next steps.

Thank you,

      Mariano

Re: [ELPA] New package: progress-bar

[Mariano Montone]

Btw, I have several usage of with-slots on a cl-structure object. That 
works in my local Emacs version 31, but when I try on older, 27 and 28, 
that is not supported.

Do you have suggestion of what to do? Use a class instead of a 
structure? Is there an alternative?

Thanks,

     Mariano

El 29/10/24 a las 10:42, Mariano Montone escribió:
> Hello,
>
> I would like to propose adding this package to ELPA: 
> https://github.com/mmontone/emacs-progress-bar/
>
> progress-bar is a progress bar that is displayed in the echo area.
>
> Please let me know if you accept, and what would be the next steps.
>
> Thank you,
>
>      Mariano
>

Re: [ELPA] New package: progress-bar

[Eli Zaretskii]

> Date: Tue, 29 Oct 2024 10:42:28 -0300
> From: Mariano Montone <marianomontone@gmail.com>
> 
> I would like to propose adding this package to ELPA: 
> https://github.com/mmontone/emacs-progress-bar/
> 
> progress-bar is a progress bar that is displayed in the echo area.

Can you tell how is it different from the built-in
make-progress-reporter and friends?

Re: [ELPA] New package: progress-bar

[Mariano Montone]

El 29/10/24 a las 11:47, Mariano Montone escribió:
> Btw, I have several usage of with-slots on a cl-structure object. That 
> works in my local Emacs version 31, but when I try on older, 27 and 
> 28, that is not supported.
>
> Do you have suggestion of what to do? Use a class instead of a 
> structure? Is there an alternative?
Nevermind. Using a class solves the problem in an easy way.

Re: [ELPA] New package: progress-bar

[Mariano Montone]

El 29/10/24 a las 11:49, Eli Zaretskii escribió:
>> Date: Tue, 29 Oct 2024 10:42:28 -0300
>> From: Mariano Montone <marianomontone@gmail.com>
>>
>> I would like to propose adding this package to ELPA:
>> https://github.com/mmontone/emacs-progress-bar/
>>
>> progress-bar is a progress bar that is displayed in the echo area.
> Can you tell how is it different from the built-in
> make-progress-reporter and friends?

It is visually different, you can observe here: 
https://github.com/mmontone/emacs-progress-bar/raw/master/progress-bar.gif

Also, it is more detailed in the process. make-progress-reporter uses a 
fixed string to inform the status of the operation; the progress bar 
allows a lambda that returns a string based on the current operation status.

        Mariano

Re: [ELPA] New package: progress-bar

[Philip Kaludercic]

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

Mariano Montone <marianomontone@gmail.com> writes:

> Hello,

Hi,

> I would like to propose adding this package to ELPA:
> https://github.com/mmontone/emacs-progress-bar/

Here are a few comments on the code:


[-- Attachment #2: Type: text/plain, Size: 9601 bytes --]

diff --git a/progress-bar.el b/progress-bar.el
index ab49ca9..f385f5a 100644
--- a/progress-bar.el
+++ b/progress-bar.el
@@ -24,15 +24,17 @@
 
 ;; A progress bar in the echo area.
 ;;
-;; This package contains the basic implementation.  For integration of progress-bar
-;; into common Emacs commands and behaviors, install progress-bar-integrations package.
+;; This package contains the basic implementation.  For integration of
+;; progress-bar into common Emacs commands and behaviors, install
+;; progress-bar-integrations package.
 ;; 
-;; Usage:
+;;;; Usage:
 ;;
-;; The preferred method for using a progress-bar is via the utility functions:
-;; `dolist-with-progress-bar', `dotimes-with-progress-bar' and `mapc-with-progress-bar'.
+;; The preferred method for using a progress-bar is via the utility
+;; functions: `dolist-with-progress-bar', `dotimes-with-progress-bar'
+;; and `mapc-with-progress-bar'.
 ;;
-;; Example:
+;;;; Example:
 ;;
 ;; (dolist-with-progress-bar (x (cl-loop for i from 1 to 10 collect i)
 ;;                              :status-message (list "Started ..."
@@ -40,11 +42,15 @@
 ;;                                                      (format "Processing %s..." (progress-bar-data pb)))
 ;;                                                    "Completed!"))
 ;;     (sit-for (seq-random-elt '(0.3 0.4 0.5))))
-;;
-;; TODO:
-;; - Consider putting event notification in call-with-progress-bar instead of in the utilities.
-;; - Consider implementing progress-bars with no total-steps specified.
-;; - Consider an option for hiding the progress-bar display after N seconds after completion.
+
+;;; TODO:
+
+;; - Consider putting event notification in call-with-progress-bar
+;;   instead of in the utilities.
+;; - Consider implementing progress-bars with no total-steps
+;;   specified.
+;; - Consider an option for hiding the progress-bar display after N
+;;   seconds after completion.
 
 ;;; Code:
 
@@ -57,48 +63,41 @@
 
 ;; Chosen from https://en.wikipedia.org/wiki/Block_Elements and inserted using `insert-char' command:
 
-(defcustom progress-bar-char ?▓
+(defcustom progress-bar-char
+  (eval-when-compile (char-from-name "DARK SHADE")) ;as a suggetion
   "Character for drawing progress bars."
-  :type 'character
-  :group 'progress-bar)
+  :type 'character)			;not necessary, :group default to the last `defgroup'
 
 (defcustom progress-bar-background-char ?░
   "Character for drawing progress bars background."
-  :type 'character
-  :group 'progress-bar)
+  :type 'character)
 
 (defcustom progress-bar-width 35
   "Standard width for progress bars."
-  :type 'integer
-  :group 'progress-bar)
+  :type 'natnum)
 
 (defcustom progress-bar-min-steps 0
   "Minimum number of steps for progress bars to be displayed."
-  :type 'integer
-  :group 'progress-bar)
+  :type 'natnum)
 
 (defcustom progress-bar-display-after-seconds 0
   "Display progress bars only after this number of seconds have passed."
-  :type 'float
-  :group 'progress-bar)
+  :type 'number)			;this was a type mismatch, 0 is not a flonum
 
-(defcustom progress-bar-format-string " [%d of %d](%d%%%%)"
+(defcustom progress-bar-format-string " [%d of %d](%d%%%%)" ;how about using `format-spec'?
   "String for formatting the progress bar.
 Arguments passed are current-step, total-steps and completed percentage.
 Consider using field number arguments for more flexibility.
 See `format' documentation."
-  :type 'string
-  :group 'progress-bar)
+  :type 'string)
 
 (defcustom progress-bar-min-time 0.2
   "The minimum time interval between progress bar displays."
-  :type 'float
-  :group 'progress-bar)
+  :type 'number)
 
 (defcustom progress-bar-min-change 1
   "The minimum percentage change required between progress bar displays."
-  :type 'integer
-  :group 'progress-bar)
+  :type 'number)
 
 (defcustom progress-bar-message-display-layout
   'concatenate
@@ -109,15 +108,14 @@ If `dynamic', the message is either concatenated or inserted after a new line
 depending on its length."
   :type '(choice (const concatenate)
                  (const newline)
-                 (const dynamic))
-  :group 'progress-bar)
+                 (const dynamic)))
 
 (defvar progress-bar-update-functions '()
   "An abnormal hook for getting notified of progress bar updates.
 Functions get called with a progress bar event, and a progress-bar instance.
 Progress bar events can be either `started', `updated' or `completed'")
 
-(cl-defstruct progress-bar
+(cl-defstruct progress-bar		;please address the `checkdoc' messages!
   (status-message nil
                   :documentation "The status-message can be either a status-formatter or a list of three status-formatters, the first applied when the progress-bar starts, the second applied for each element processed, the third when the progress-bar completes.
 A status-formatter is either a string or a function that takes a progress-bar instance and returns a string.")
@@ -159,7 +157,7 @@ See `progress-bar-update-functions' hook."
 ARGS is a property-list of slot-name and value.
 
 Example:
-(progress-bar-update pg 'current-step 2 'data 'foo)"
+\(progress-bar-update pg \\='current-step 2 \\='data \\='foo)"
   (cl-loop for (slot value) on args by 'cddr
            do (setf (slot-value progress-bar slot) value))
   (progress-bar--display progress-bar)
@@ -168,11 +166,12 @@ Example:
     (progress-bar-notify 'updated progress-bar)))
 
 (defun progress-bar-incf (progress-bar &optional increment display)
-  "Increment step in PROGRESS-BAR."
+  "Increment step by STEP in PROGRESS-BAR.
+If DISPLAY is non-nil, ..."
   (let ((inc (or increment 1)))
     (with-slots (current-step total-steps) progress-bar
       (when (and total-steps (> (+ current-step inc) total-steps))
-        (error "current-step > total-steps"))
+        (error "current-step > total-steps")) ;please rephrase this error message to be understandable on its own
       (cl-incf current-step inc)
       (when display
         (progress-bar--display progress-bar))
@@ -185,7 +184,7 @@ Example:
   (if (progress-bar-completed-p progress-bar)
       100
     (with-slots (current-step total-steps) progress-bar
-      (truncate (* (/ current-step (float total-steps)) 100)))))
+      (truncate (* current-step 100.0) total-steps))))
 
 (defun progress-bar--display (progress-bar)
   "Display PROGRESS-BAR in echo-area."
@@ -207,7 +206,7 @@ Example:
 
 (defun progress-bar--format-status-message (progress-bar message)
   (cl-etypecase message
-    ((or symbol function)
+    (function ;; you don't need to check symbols separately: (cl-typep 'insert 'function) ;=> t
      (funcall message progress-bar))
     (string message)))
 
@@ -256,7 +255,7 @@ evaluating FUNC, so that messages are displayed together with the progress bar."
       (funcall func progress-bar)
     ;; Replace the implementation of `message' temporarily, so that
     ;; messages sent by FUNC are shown together with the progress bar.
-    (let ((emacs-message (symbol-function 'message)))
+    (let ((emacs-message (symbol-function #'message)))
       (cl-flet ((pb-message (msg &rest args)
                   ;; This is only for logging. Can we log the message
                   ;; without calling `message' ?
@@ -286,10 +285,10 @@ evaluating FUNC, so that messages are displayed together with the progress bar."
 
 (defmacro with-progress-bar (spec &rest body)
   "Create a PROGRESS-BAR binding SPEC in BODY scope.
-SPEC has either the form (VAR PROGRESS-BAR-INSTANCE) or (VAR &rest INITARGS), with
-INITARGS used for creating a `progress-bar'.
-This macros sets up special treatment for calls to MESSAGE that may ocurr in BODY,
-so that messages are displayed together with the progress bar."
+SPEC has either the form (VAR PROGRESS-BAR-INSTANCE) or (VAR &rest
+INITARGS), with INITARGS used for creating a `progress-bar'.  This
+macros sets up special treatment for calls to MESSAGE that may ocurr in
+BODY, so that messages are displayed together with the progress bar."
   (declare (indent 2))
   (cl-destructuring-bind (var &rest initargs) spec
     (if (= (length initargs) 1)
@@ -298,9 +297,9 @@ so that messages are displayed together with the progress bar."
       `(let ((,var (make-progress-bar ,@initargs)))
          (call-with-progress-bar ,var (lambda (,var) ,@body))))))
 
-;; Utilities
+;;;; Utilities
 
-(defun mapc-with-progress-bar (func sequence &rest args)
+(defun progress-bar-mapc (func sequence &rest args)
   "Like `mapc' but using a progress-bar."
   (let ((progress-bar (if (= (length args) 1)
                           (car args)
@@ -318,7 +317,7 @@ so that messages are displayed together with the progress bar."
       (progress-bar--display progress-bar)
       (progress-bar-notify 'completed progress-bar))))
 
-(defmacro dolist-with-progress-bar (spec &rest body)
+(defmacro progress-bar-dolist- (spec &rest body)
   "Like DOLIST but displaying a progress-bar as items in the list are processed.
 ARGS are arguments for `make-progress-bar'.
 
@@ -334,7 +333,7 @@ Example:
   (cl-destructuring-bind (var list &rest args) spec
     `(mapc-with-progress-bar (lambda (,var) ,@body) ,list ,@args)))
 
-(defmacro dotimes-with-progress-bar (spec &rest body)
+(defmacro progress-bar-dotimes (spec &rest body)
   "Like `dotimes' but with a progress bar."
   (declare (indent 2))
   (let ((progress-bar (gensym "progress-bar-")))

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



> progress-bar is a progress bar that is displayed in the echo area.
>
> Please let me know if you accept, and what would be the next steps.

My main question, which I realised too late when reading the code, is if
you could rework this to integrate into existing instances of
`make-progress-reporter', just replacing the UI.  It seems like it would
be more effective and consistent, and avoid hard dependencies of
programs that want to use `dotimes-with-progress-bar' (or as I renamed
it `progress-bar-dotimes' to avoid namespace clashes), when
`dotimes-with-progress-reporter' already exists and is being used.

> Thank you,
>
>      Mariano
>
>
>

-- 
	Philip Kaludercic on siskin

Re: [ELPA] New package: progress-bar

[Mariano Montone]

El 29/10/24 a las 12:03, Philip Kaludercic escribió:
> My main question, which I realised too late when reading the code, is if
> you could rework this to integrate into existing instances of
> `make-progress-reporter', just replacing the UI.  It seems like it would
> be more effective and consistent, and avoid hard dependencies of
> programs that want to use `dotimes-with-progress-bar' (or as I renamed
> it `progress-bar-dotimes' to avoid namespace clashes), when
> `dotimes-with-progress-reporter' already exists and is being used.

Oh. Thanks for the patch! I'll look at it.

The integration part is in progress-bar-integrations.el. How does it 
looks to you?

     Mariano

Re: [ELPA] New package: progress-bar

[Philip Kaludercic]

Mariano Montone <marianomontone@gmail.com> writes:

> El 29/10/24 a las 12:03, Philip Kaludercic escribió:
>> My main question, which I realised too late when reading the code, is if
>> you could rework this to integrate into existing instances of
>> `make-progress-reporter', just replacing the UI.  It seems like it would
>> be more effective and consistent, and avoid hard dependencies of
>> programs that want to use `dotimes-with-progress-bar' (or as I renamed
>> it `progress-bar-dotimes' to avoid namespace clashes), when
>> `dotimes-with-progress-reporter' already exists and is being used.
>
> Oh. Thanks for the patch! I'll look at it.

Just keep in mind that it is not a patch, it is just a convenient way to
suggest changes and add comments.

> The integration part is in progress-bar-integrations.el. How does it
> looks to you?

Oh, I missed that.  My main issue is that this mixes both the
`progress-reporter-do-update' integration with other advice on functions
like `package-upgrade-all'.  I think having a global minor mode would be
the right approach, instead of advising on the top-level.

Generally it would be neat if we could find a solution that would avoid
the need for advice, but I don't see a clean way to do that right now.
Would you be interested in preparing a patch for subr.el that would make
progress-reporters more flexible?

>     Mariano
>

-- 
	Philip Kaludercic on siskin

Re: [ELPA] New package: progress-bar

[Mariano Montone]

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


El 29/10/24 a las 12:24, Philip Kaludercic escribió:
> Mariano Montone<marianomontone@gmail.com> writes:
>
>> El 29/10/24 a las 12:03, Philip Kaludercic escribió:
>>> My main question, which I realised too late when reading the code, is if
>>> you could rework this to integrate into existing instances of
>>> `make-progress-reporter', just replacing the UI.  It seems like it would
>>> be more effective and consistent, and avoid hard dependencies of
>>> programs that want to use `dotimes-with-progress-bar' (or as I renamed
>>> it `progress-bar-dotimes' to avoid namespace clashes), when
>>> `dotimes-with-progress-reporter' already exists and is being used.
>> Oh. Thanks for the patch! I'll look at it.
> Just keep in mind that it is not a patch, it is just a convenient way to
> suggest changes and add comments.
Yes.
>> The integration part is in progress-bar-integrations.el. How does it
>> looks to you?
> Oh, I missed that.  My main issue is that this mixes both the
> `progress-reporter-do-update' integration with other advice on functions
> like `package-upgrade-all'.  I think having a global minor mode would be
> the right approach, instead of advising on the top-level.
>
> Generally it would be neat if we could find a solution that would avoid
> the need for advice, but I don't see a clean way to do that right now.
> Would you be interested in preparing a patch for subr.el that would make
> progress-reporters more flexible?

Yes, but I would need more precise explanations from you on your idea of 
how to do it.

         Mariano

[-- Attachment #2: Type: text/html, Size: 2542 bytes --]

Re: [ELPA] New package: progress-bar

[Mariano Montone]

El 29/10/24 a las 12:48, Mariano Montone escribió:
>> Generally it would be neat if we could find a solution that would avoid
>> the need for advice, but I don't see a clean way to do that right now.
>> Would you be interested in preparing a patch for subr.el that would make
>> progress-reporters more flexible?
>
> Yes, but I would need more precise explanations from you on your idea 
> of how to do it.
>
Ideally, in my opinion, it would be good to make the progress-report 
object a bit more complex, contain more information.

Probably switch from its list representation to a struct or class, and 
make it more similar to what I have in the progress-bar class.

Keep backwards compatibility in the process.

Then divide model from user interface representation, and make it 
pluggable. So those new objects could be displayed via a progress-bar, 
or with current progress-reporters ui, or something else.

       Mariano