search

protesilaos / denote

Simple notes for Emacs with an efficient file-naming scheme
https://protesilaos.com/emacs/denote
GNU General Public License v3.0
541 stars 55 forks source link

Fix unused let-bound variable warnings #457

Closed grtcdr closed 1 month ago

grtcdr commented 1 month ago

I noticed some warnings pop up during a reinstallation of Denote and went and fixed them.

Here are the errors for reference:

⛔ Warning (comp): denote.el:4347:15: Warning: variable ‘_’ not left unused
⛔ Warning (comp): denote-org-extras.el:640:15: Warning: variable ‘_’ not left unused
⛔ Warning (comp): denote-org-extras.el:637:15: Warning: variable ‘_’ not left unused

Unlike pcase which holds a special meaning for _, there's no way to mark a let-bound variable as ignored but we can omit the SYMBOL name from the VARLIST and the when-let expression will still evaluate correctly.

grtcdr commented 1 month ago

I found out that denote-get-identifier-at-point is an unused function through xref-find-references so you might consider removing it if no one's making use of it (which is hard to know for me).

protesilaos commented 1 month ago

This is fine and I am happy to merge it. Which version of Emacs are you using? I do not get these warnings on my end...

consider removing it if no one's making use of it (which is hard to know for me).

I know at least one case of someone using it. But we may want to think of how best to organise our files to decide where is the best place to put these sort of functions.

grtcdr commented 1 month ago

Yeah, I should've mentioned I was on version 29.4, if my changes don't cause noise on older versions of Emacs then I'd consider it safe to roll out.

By the way, you can find instances of this approach in built-in packages (see function tramp-accept-process-output for example), so it's safe to assume that it is backwards-compatible to some degree -- although to what degree I am not sure.

protesilaos commented 1 month ago

Merged, thank you!

I remember the standard approach of when-let to be like this:

(when-let (((stringp a))
           (one a))
  BODY)

We have those cases in the Denote source code. Though I do not get any errors on my end with this approach (Emacs 31, but I believe it also worked on Emacs 30):

(when-let ((_ (stringp a))
           (one a))
  BODY)

The latter seems more readable to me, but I will not argue with the byte compiler.