Add a visual progress for when evaluating a directory recursively

[J0sueTM]

Recently, since I'm working with a big clj project, I periodically use cider-eval-all-files, and it works fine. The only problem is that I always underestimate the time it takes to compile all the files in a directory, and endup evaluating another directory while the first one is still in progress.

That's what I try to solve with this PR. By adding a visual progress, it becomes easier to know if its OK to continue evaluating other modules of my code that depends on the thing that's currently being done.

BTW, I ran eldev lint, and some errors showed up, but none of them had to do with my changes.

---

Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• All tests are passing (eldev test)
• All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • byte-compilation, checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the user manual (if adding/changing user-visible functionality)

Thanks!

If you're just starting out to hack on CIDER you might find this section of its
manual extremely useful.

[lanjoni]

LGTM

[vemv]

Seems like a reasonable addition - kudos!

Please simply satisfy the small feedback and ensure there's a green build.

Cheers - V

[vemv]

It would seem to me that you could use a simple counter that would be incremented on each iteration

[bbatsov]

I've got some concern as to whether this should be in the public API of the command, given that it's needed only by the counter. Perhaps we can introduce a helper function to hide this?

[bbatsov]

@J0sueTM ping :-)

[J0sueTM]

Thanks for pinging @bbatsov completely forgot about it! My time hasn't been the most generous recently 😅 . I'll be coming with changes in the coming days.

[vemv]

Thanks for picking up this!

Left a last round, please ensure a green build + no conflicts as well

[vemv]

While number-sequence is a nice function, I'd prefer it you used local mutation as it would seem easier to follow here.

An example:

(let* ((files '("a" "b" "c"))
       (i 0))
        (mapcar (lambda (file)
                  (message (format "%s %s" file i))
                  (setq i (inc i)))
                files))

So, you can use the existing mapcar to mutate a local counter, so we have one mapcar instead of two (plus the helper function).

[J0sueTM]

Thanks for pointing out. My elisp knowledge is really limited and that helped a lot. Thanks!

[bbatsov]

It's better to use mapc instead of mapcar if all you care about are the side-effects.

[bbatsov]

Looking again at the code - probably using dolist would be even better than mapc, as it reflects best what we want to do anyways.

[bbatsov]

You should not use concat in message, as you can just use format specifiers like %s, %d, etc. It's similar to Clojure's format.

[bbatsov]

Btw, I still don't like that we're now passing some text around. Might be better to make the message optional or move the progress reporting to the function that deals with many namespaces or to rename the progress param to something like msg, so it's clearer what this is.

Also - for a lot of files some status bar might be a better option (e.g. https://www.gnu.org/software/emacs/manual/html_node/elisp/Progress.html or spinner.el that we're using elsewhere in the code)

[bbatsov]

@J0sueTM ping :-)

[J0sueTM]

hey @bbatsov. I'm probably not able to finish this PR in the way you guys want. Nothing wrong with that, of course, I just don't know much about ELisp or have enough time in order to accomodate your quality requests 😅.

I'm sorry for taking your time. Could you please either close the PR or backlog it so, if on interest of anyone, implement this better later?

Thanks for the great lib. I use it daily and it work flawlessly!

[vemv]

It's no issue. Normally in these cases someone from us can pick up the branch and do whatever's needed, preserving attribution to you.

(Will do)

[bbatsov]

@katomuso That's a fairly small and simple task, that might be interesting to you if you're looking for more tickets to tackle.

[katomuso]

@katomuso That's a fairly small and simple task, that might be interesting to you if you're looking for more tickets to tackle.

I will look into it sometime this month. Do I need to create a separate PR and link to this one, or is there a better way to do this?

[bbatsov]

@katomuso You can just do a separate PR - as noted in my comments, I think it's much better (and simpler) to just use a real progress indicator (either a built-in one or coming from spinner.el, which is a CIDER dep) instead of indicating progress via a message printed in the minibuffer.

[katomuso]

@katomuso You can just do a separate PR - as noted in my comments, I think it's much better (and simpler) to just use a real progress indicator (either a built-in one or coming from spinner.el, which is a CIDER dep) instead of indicating progress via a message printed in the minibuffer.

As far as I understand, spinner.el can't show exact progress (number of evaluated files / number of total files). I will look into the ways in which this is possible, as it is not very reassuring to wait a minute or two without knowing how much longer it will take.

[bbatsov]

That's the classic built-in approach - https://www.gnu.org/software/emacs/manual/html_node/elisp/Progress.html I mentioned spinner just because some indication that something is happening will still be an improvement over no indication. :-)