Re: [elpa] externals/detached 827e3d64fe: Bug fix: Incorrect initialization of projectile

Re: [elpa] externals/detached 827e3d64fe: Bug fix: Incorrect initialization of projectile

[Stefan Monnier]

Hi Niklas,

> @@ -150,7 +150,7 @@
>  
>  (defun detached-init--projectile ()
>    "Initialize integration with `projectile'."
> -  (when (functionp #'projectile)
> +  (when (featurep 'projectile)
>      (advice-add 'projectile-run-compilation
>                  :override #'detached-extra-projectile-run-compilation)))
>  

I suspect that you can then also remove the corresponding
`declare-function`.
More importantly, `advice-add` can be used on a symbol whose function is
not yet defined, so you can skip the `when` test altogether.

The same applies to the following:

    (when (functionp #'dired-rsync)
      (advice-add #'dired-rsync--do-run :override #'detached-extra-dired-rsync)))

where my stylistic checker additionally complains about:
- #' kind of says that `dired-rsync` is a function, so using `functionp`
  on it is a bit weird.  Usually, code does (fboundp 'dired-rsync) instead.
- `advice-add` takes as first argument a symbol.  That symbol is
  expected to contains a function, indeed, but it can't just take any
  function (e.g. a (lambda ...) would be an error), for that reason
  I think using ' rather than #' is preferable (e.g. if you consider
  that Common Lisp (and `cl-flet`) will treat #'<symbol> as a reference
  to the function stored in the symbol (much like `symbol-function`)
  rather than as the symbol itself).


        Stefan

Re: [elpa] externals/detached 827e3d64fe: Bug fix: Incorrect initialization of projectile

[Niklas Eklund]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

> Hi Niklas,
>
>> @@ -150,7 +150,7 @@
>>  
>>  (defun detached-init--projectile ()
>>    "Initialize integration with `projectile'."
>> -  (when (functionp #'projectile)
>> +  (when (featurep 'projectile)
>>      (advice-add 'projectile-run-compilation
>>                  :override #'detached-extra-projectile-run-compilation)))
>>  
>
> I suspect that you can then also remove the corresponding
> `declare-function`.
> More importantly, `advice-add` can be used on a symbol whose function is
> not yet defined, so you can skip the `when` test altogether.

That's very neat that the check can be skipped, thanks for the
suggestion!

> The same applies to the following:
>
>     (when (functionp #'dired-rsync)
>       (advice-add #'dired-rsync--do-run :override #'detached-extra-dired-rsync)))
>
> where my stylistic checker additionally complains about:
> - #' kind of says that `dired-rsync` is a function, so using `functionp`
>   on it is a bit weird.  Usually, code does (fboundp 'dired-rsync)
> instead.

Aha, I had been thinking it should be quoted since it was a function but
now I understand that better. Btw what stylistic checker are you running
that shows you this? :)

> - `advice-add` takes as first argument a symbol.  That symbol is
>   expected to contains a function, indeed, but it can't just take any
>   function (e.g. a (lambda ...) would be an error), for that reason
>   I think using ' rather than #' is preferable (e.g. if you consider
>   that Common Lisp (and `cl-flet`) will treat #'<symbol> as a reference
>   to the function stored in the symbol (much like `symbol-function`)
>   rather than as the symbol itself).

That make sense with the lambda example, thanks for the explenation!
>
>         Stefan

/Niklas