Hash-quote functions?

[Ambrevar]

Should we hash-quote all functions in all bindings?

[jojojames]

I try to do it when I remember, but it's not important to me whether it's there or not there.

[Ambrevar]

So it's supposed to be #' and it helps catching undefined functions, but considering none of them is defined, well, it does not really matter I guess.

[jojojames]

Lets go with 'both are fine, hash quote is better but not required.'?

[Ambrevar]

Yup.

[jojojames]

I'm actually realizing from #31 that it might be better to single-quote symbols if we don't want the compiler to complain when using optional packages.

Unless you know of a better option other than forward-declaring a bunch of functions, what do you think about just going with single quotes @Ambrevar?

@fbergroth Feel free to give your input too.

[fbergroth]

@jojojames it depends on how we want to the resolve compiler warnings. I don't believe you can forward-declare functions in the same way as variables. I experimented a bit, and found two ways to work around the unresolved function warnings. Either we wrap calls with with-no-warnings, or we insert this boilerplate at the top:

(eval-when-compile
  (setq byte-compile-warnings
        '(free-vars
          ;; unresolved
          callargs
          redefine
          obsolete
          noruntime
          cl-functions
          interactive-only
          lexical
          make-local
          mapcar
          constants
          suspicious)))

I think with-no-warnings is cleaner, and not sharp-quoting functions in 3rd party packages. If you prefer the latter, sharp-quoting will have no effect.

[jojojames]

Here's the ones I have so far. If you don't see one in the below list, assume I've done something for it. :)

cask exec emacs -Q -batch
-L .
--eval '(setq byte-compile-error-on-warn t)'
-f batch-byte-compile *.el

In toplevel form:
evil-edebug.el:111:1:Error: the function `evil-integration-edebug-mode-set-motion-default' is not known to be defined.

This looks to be because of a quick macro I wrote. Not sure what's going on here but I hacked up the code for edebug anyways.
I'd like to fix that instead..

In toplevel form:
evil-emms.el:80:1:Warning: Unused lexical variable `map'
evil-emms.el:185:1:Error: the following functions are not known to be defined: emms-with-inhibit-read-only-t, emms-playlist-mode-correct-previous-yank

The first seems to be due to evil-define-key and map variable. The second seems to be due to calling functions from third party packages.
In this case, I like your approach -> with-no-warnings.

We can also use fboundp.

(when (fboundp 'company-tng-configure-default)
(company-tng-configure-default))

In toplevel form:
evil-evilified-state.el:334:1:Error: the function `evil-surround-mode' is not known to be defined.

Same thing here, can just tweak the call to evil-surround-mode with either fboundp or with-no-warnings.

In toplevel form:
evil-pdf.el:221:1:Error: the following functions are not known to be defined: pdf-view-goto-page, pdf-view-last-page, pdf-view-first-page
make: *** [compile] Error 1

Also same, fboundp/with-no-warnings.

I'm not to worried about either/or. If anyone had strong objections to either, feel free to raise it. Otherwise use which ever one.
I usually use fboundp so that's what I would reach for.

Otherwise single quoting functions seem to be the right call. I'm not a fan of the boilerplate.

Thanks for looking into this @fbergroth.

[fbergroth]

I pushed another PR, #34. I went for with-no-warnings, but I can change it to fboundp if you prefer.

The warning in evil-edebug.el was solved by moving the macro call to the top level.

evil-emms.el:80:1:Warning: Unused lexical variable 'map' - this was fixed by calling the function evil-define-key* instead of the macro evil-define-key.

[jojojames]

Lets settle on no hash-quotes then. Feel free to reopen if you have an issue @Ambrevar.

[Ambrevar]

I think we should no block-wrap using with-no-warnings simply for the sake of silencing warnings: it defeats the purpose of having the byte-compiler at hand to help detecting actualy errors. Now if some actual warning were present in the with-no-warnings block, it will be spuriously silenced.

Besides it's not pretty and inconsistent with the rest of the code out there.

Warnings are what they are: indicators. Silencing warnings is not a purpose in itself. If we want to silence those specific warnings, it should be done on the linter side, not in the code which is fine.

[fbergroth]

@Ambrevar one item in the checklist when submitting a melpa recipe is "My elisp byte-compiles cleanly". Also, most good emacs packages makes sure byte compilation is without warnings. It's ugly for a user to get a bunch of warnings thrown at you when installing a package.

[Ambrevar]

Many MELPA packages throw warnings when they are byte-compiled.
The byte compiler is not perfect.

[jojojames]

I think that's their problem. We should strive to have a clean byte compile.

Fboundp is still an option.

@Ambrevar Do you have a problem with hash quotes or the -with-no-warnings? If the latter, let's use a different topic thread.

[jojojames]

I do believe that is a checklist item by the way. The fact that other packages stop "trying" later on is on them.

[fbergroth]

@Ambrevar with that reasoning, evil-collection shouldn't require with noerror as well. This all is a hack to release this as a single package without declaring a dependency on every third party package.

[Ambrevar]

Let's move this discussion to a new issue.

[jojojames]

@Ambrevar I am closing this one unless you have a problem with not hashquoting functions.