Use :after advice instead

nagy commented 2 years ago

This also clears up the stack-trace during profiling.

wyuenho commented 2 years ago

What's wrong with the stack trace currently?

nagy commented 2 years ago

When debugging, for example, #'dired-revert with edebug and getting a stacktrace, this is the difference cause by this PR:

Before:

(closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
apply((closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
all-the-icons-dired--refresh-advice((closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
apply(all-the-icons-dired--refresh-advice (closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
dired-revert(nil nil)
revert-buffer()
dired-internal-noselect("/home/.../" nil)
dired-noselect("~" nil)
ls-lisp--dired(#<subr dired> "~")
apply(ls-lisp--dired #<subr dired> "~")
dired("~")
dired-home()
funcall-interactively(dired-home)
command-execute(dired-home)

After:

(closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
apply((closure (dired-file-version-alist t) (&optional _arg _noconfirm) ...
dired-revert(nil nil)
revert-buffer()
dired-internal-noselect("/home/.../" nil)
dired-noselect("~" nil)
ls-lisp--dired(#<subr dired> "~")
apply(ls-lisp--dired #<subr dired> "~")
dired("~")
dired-home()
funcall-interactively(dired-home)
command-execute(dired-home)

As you can see, the execution does not go through #'all-the-icons-dired--refresh-advice but rather is more direct.

This becomes more apparent when viewing a tree-like profiler, where each stack is accumulating running time of a function. Slow (dired-)functions now make it seem like that all-the-icons-dired is consuming much time, when in reality it doesn't.

Also, since we are actually intending to call the advice "after" the function, we might as well just codify it semantically with the use of :after.

But feel free to close this if this does not make sense.

wyuenho / all-the-icons-dired

Use :after advice instead #12