Re: [ELPA] New package: listen

[Philip Kaludercic <philipk@posteo.net>]

Adam Porter <adam@alphapapa.net> writes:

>> There were a few other places where you did (delq nil (mapcar ...)) that
>> might be replaced by this pattern.
>
> I confess that I've hardly ever used MAPCAN in any of my code--still,
> I'm not sure how that would help to avoid that pattern.  For example:
>
>   (let ((a '(1 nil 2 nil 3))
>         (b '(4 nil 5 nil 6)))
>     (mapcan #'identity (list a b)))
>
>   ;; (1 nil 2 nil 3 4 nil 5 nil 6)
>
> But I'm probably missing something.

The pattern I usually use is something like this:

(mapcan
 (lambda (n)
   (if (> n 0) (list n) nil))
 '(1 -2 5 0 -10 10))
;;=> (1 5 10)

The idea is that when using `mapcan', you provide the cons-cells of the
resulting list, instead of having `mapcar' generate them for you and
then immediately discard them via `delq'.

There should be a seq method for this pattern imo.

>> Not really, if you don't care about compiler warnings.  It just seems
>> like the kind of things that could cause problems at some later time,
>> when definitions are moved around.
>
> I do care about compiler warnings--I wrote makem.sh to catch such
> warnings in my projects before they reach users--but in this case,
> that code's not going anywhere, because it's already with
> bookmark-related code.

Feel free to do as you like (none of this is barring inclusion, if that
wasn't clear), that was just a suggestion.

>> Ah, the `t did confuse me momentarily, but in that case you can replace
>> the (guard ...) with (and 't (guard ...)).
>
> As much as I advocate using pcase and its powerful expressions, I
> think that would make this example harder to follow.  The pcase
> pattern is used to test an argument, and the string test is a separate
> concern.

But consider the saved indentation!

>>>> @@ -328,7 +328,7 @@ PROMPT is passed to `format-prompt', which see."
>>>>             n)
>>>>        (when current-track
>>>>          (cl-callf2 delete current-track tracks))
>>>> -    (setf n (length tracks))
>>>> +    (setf n (length tracks))   ;why the variable?
>>>
>>> Because the value is used elsewhere in the function.  Am I missing
>>> something?  (Anyway, as noted in the source, I did not write that
>>> function.)
>> Then I missed something, because I just saw the variable being
>> declared
>> in the let-head, set here and used once later.
>
> It's used in two places, and it would be undesirable to recalculate
> the list's length every time through the loop.

I see it now, sorry.

>>>> On the topic of the readme/manual, wouldn't it be better to have a
>>>> separate README file?  Then again, the manual is pretty short, and I
>>>> don't know if it is worth having it in the first place...
>>>
>>> As you said, this readme is currently trivial.  Were it larger,
>>> however--well, I have other packages with much larger readmes that are
>>> also converted to manuals.  There would not be much gained by
>>> separating into files.
>> I don't think that is a good practice.  A README for when you have
>> fetched the sources and want to figure out what is what, a manual for
>> when you have already installed a package and a package description for
>> when you are previewing a package using C-h P are three different
>> things.  One shouldn't cover all of it with the same file if you ask me,
>> since they all have different audiences with different levels of
>> interest.
>
> I don't know about that.  Especially for small packages with trivial
> documentation.  Maintaining documentation and commentaries, keeping
> them reasonably in sync, etc. is enough work without having them split
> into multiple files.  Having a README.org which is viewable at the
> package's repo also generate the manual is a relief to me.

My point is that there shouldn't be an overlap.  I think a README
shouldn't contain too much detail, but serve as a signpost (suitable
both for online and offline (!) reading): "This is brief summary of what
you have found, the source code is hosted here, you can find the
documentation there, my contact information somewhere else, etc.", while
the package description gives a high level overview that doesn't have to
updated unless the entire idea of the package changes, while the
documentation goes into the nitty-gritty details.

>>>> Also, your README includes this line
>>>>     :vc (:fetcher github :repo "alphapapa/listen.el")
>>>> which is malformed.
>>>
>>> I just tested that, and it works for me.
>> On Emacs 30?  That is not the code we merged...
>
> No, I'm using Emacs 29 with `vc-use-package'.  Its documentation seems
> to suggest that it uses the same format as that merged into Emacs 30,
> since it says that its features were merged into Emacs 30.
>
> Maybe `vc-use-package's documentation should be updated to reflect this?

Do you mean this: https://github.com/slotThe/vc-use-package?  I have no
involvement with that project, but I don't see where they mention the
"fetcher" notation you mention.

>>>> What you want is
>>>>     :vc (:url "https://github.com/alphapapa/listen.el")
>
> Ok, so no ".git" on the end (i.e. relying on the GitHub redirect)?

That doesn't matter (FWIW I didn't know either of the two was a redirect).

> And does this mean that none of the host-specific "fetchers" are
> available in Emacs 30?  (Which FTR is fine with me, as the URL should
> be enough, I'm just curious.)

No, the package-vc extension for use-package uses the same package
specifications as package-vc?

-- 
	Philip Kaludercic on peregrine