Re: [ELPA] New package: hcel

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> Given the fact that this tool has a fairly narrow focus, I must admit
>> that I'd prefer it doesn't eat up a two-letter file name like `hc`.
>> Any chance you can rename that main file to `hcel.el`?
>
> I applied your patch that does so.

Perfect, thanks.

>> Also, I see that your package has:
>>
>>     ;; Package-Requires: ((emacs "28") (haskell-mode))
>>
>> but `haskell-mode` is currently neither in GNU ELPA nor in NonGNU ELPA
>> so we can't include it in neither of them.  Is the dependency
>> unavoidable (so we should first include `haskell-mode` in (Non)GNU
>> ELPA), or can it be removed (and replaced with a runtime test, possibly
>> allowing the use of other haskell modes such as haskell-tng-mode, which
>> *is* in NonGNU ELPA)?
>
> It seems to me haskell-mode is in NonGNU ELPA[3]?

Oh, right, sorry.  It's currently not being tracked (see
https://github.com/haskell/haskell-mode/issues/1755), but it's still in
NonGNU ELPA, yes.

Sorry 'bout the noise, the dependency on `haskell-mode` is fine.

>>  (defun hcel-results-next-page ()
>>    (interactive)
>> +  ;; FIXME: Using `major-mode' is a code smell.
>>    (unless (memq major-mode '(hcel-refs-mode hcel-ids-mode))
>>      (error "Not in hcel-refs or hcel-ids mode: %S" major-mode))
>
> What do you mean, should I avoid using the variable `major-mode'? Why?

Experience over the years taught me that.  It's a bit like explicit type
tests in OO languages, I think.  Stefan Kangas's suggestion to use
`derived-mode-p` is a bit better, but it's still not great.
It's often better to "do something else".  Depending on the situation
the "something else" can be quite varied, but one common option is to
have the major mode set a buffer-local variable that gives the
needed info.

>> In end of data:
>> packages/hcel/hcel-client.el:142:8: Warning: the function
>> ‘delete-http-header’ is not known to be defined.
>> packages/hcel/hcel-client.el:59:19: Warning: the function
>> ‘hcel-location-tag’ is not known to be defined.
>> packages/hcel/hcel-client.el:47:43: Warning: the function
>> ‘hcel-format-package-id’ is not known to be defined.
>> Byte compiling packages/hcel/hcel-results.el
>
> How do I get these "end of data" warnings?  When I do M-x byte-compile
> or byte-recompile-directory I don't get them.

When you `M-x byte-compile`, the compilation is done in the running Emacs,
which already has lot of packages loaded an thus won't be able to notice
if your code relies on those packages without making sure they'll be
loaded before using them.

The above warnings come from a "clean compilation" where every file is
compiled in a separate dedicated Emacs session.

>> In hcel-module-selector:
>> packages/hcel/hcel.el:55:43: Warning: reference to free variable
>> ‘package-id’
> I thought this was OK because of lexical-binding?  For reference, here's
> the defun:

As Micheal points out, this is Bug#51695 :-(
And indeed there is no nice solution for it currently.

In Emacs-29, there is a (cumbersome) solution which involves the use of
an OClosure.  I'm hoping to find the time before Emacs-29.1 to write the
code so the bytecompiler can use the OClosure behind the scenes for you.


        Stefan