[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [GNU ELPA] New package: tam

From: Philip Kaludercic
Subject: Re: [GNU ELPA] New package: tam
Date: Wed, 20 Sep 2023 08:26:47 +0000

Lynn Winebarger <owinebar@gmail.com> writes:

> On Mon, Sep 18, 2023 at 1:02 PM Philip Kaludercic <philipk@posteo.net> wrote:
>> > Apologies, I renamed the library to tam.el and failed to note the
>> > changes I made to the renamed file did not get committed and pushed.
>> So what does that mean?
> That I don't use git enough to avoid rookie errors? I had already
> addressed some of the items you brought up in my working copy, they
> just hadn't been committed and pushed to the server.
> I think I've addressed all the items you brought up in the last
> version I pushed.

Ok.  What I wanted to make sure was that you didn't change the
repository URL or something like that.

Re your last change in [0], why use records directly instead of having
the code being generated via cl-defstruct?  The commit messages doesn't
really explain your reasoning to me.


>> I am the kind of person who thinks twice about installing a package when
>> it has dependencies.  But if you prefer to use a package available on
>> ELPA, then that is of course OK as well.
> I revised it to make use of cl-loop per your observation in the
> previous email.  I'm not really a fan of the loop imperative DSL, but
> this case was simple enough (as you pointed out).
> Some packages (like queue) provide pretty basic data structures, so I
> think avoiding packages that depend on them only encourages
> reimplementing the wheel.
> However, I might be the world record holder for the number of elisp
> libraries loaded and included in a dump file.
>> >> -(defsubst tam-table-size (tbl)
>> >> +(defsubst tam-table-size (tbl)         ;why not `defalias'
>> >
>> > I tried defalias first, but got a byte-compiler error about a void
>> > variable.  Which I found confusing, since it should be looking for a
>> > function definition, not a variable.  I'm using 28.3.
>> > Some of the following should have already been fixed from when I
>> > ran checkdoc.
>> What did you do?  That sounds like something was misquoted:
>> (defalias 'tam-table-size #'tam--table-size)
> I forgot defalias is a function and not a macro.  Thanks.
>> >                                                             I'll
>> > change it if required, but I find computing the place inside a set
>> > form to be disconcerting if it's not required.  For example, I
>> > wouldn't use a set form like
>> > (set (if P 'A 'B) some-value)
>> > in place of
>> > (if P (setq A some-value) (setq B some-value))
>> In that case, is there a reason you are using setf?
> I'm confused - the documentation does not indicate that an "if" form
> would be a "place" that setf knows how to handle.  The cl-lib doc does
> say it extends setf to handle macro expansions, but 'if' is a special
> form that does not macro-expand.
> I use setf because that's the cleanest way to set fields of a
> structure.  It's more like setq than set (it's a macro for one thing)
> as far as I can tell.

The question was supposed to be more general, sorry for the confusion.
I wanted to know if there was a reason you were using setf even when
setq would be enough, but it really doesn't matter either way since setf
on a symbol expands directly to setq.

>> > I do want to return lists reflecting the ordering of the slots.  I am
>> > not a fan of constructing an ordered structure only to reverse it.
>> FWIW this is standard practice, and what a cl-loop with collect would
>> also expand to.  And if I am not mistaken, this is more efficient, than
>> accumulating a linked list in the right order to begin with (it is a
>> difference of O(n) vs O(n^2), I believe).
> Wait, did you mean cl-loop with collect will do the nreverse?  I
> thought you meant it would keep track of the tail of the list and
> update it.

No, it uses nreverse:

--8<---------------cut here---------------start------------->8---
(macroexpand-all '(cl-loop for i to 10 collect i))
(let* ((i 0) (--cl-var-- nil))
  (while (<= i 10)
    (setq --cl-var-- (cons i --cl-var--))
    (setq i (+ i 1)))
  (nreverse --cl-var--))
--8<---------------cut here---------------end--------------->8---

> Accumulating a list in the right order is only O(n^2) if you only keep
> the head of the list.  The queue structure (or what I would write with
> let-bound variables) holds a reference to the last cons cell of the
> list to use in adding an entry on the end.  It's probably negligible
> for very short lists, but it's just bad form.
>> > That said, these functions are primarily intended for debugging.
>> Wouldn't that kind of a convenience be an argument against adding an
>> extra dependency?
> Only if I was trying to get a library included in startup.el, or if
> the dependency was not in GNU ELPA.  Otherwise, why are people writing
> packages?  They are not all stand-alone user interfacing programs.
> Some are just basic data structures and algorithms that haven't been
> included in core emacs for whatever reason.  Queue is one of those.

These kinds of arguments lead to leftpad like situations, where people
defer any slightly complicated functionality to a library.  The last
thing I want to see when installing a package is that it drags along
dozens or even hundreds of recursive dependants, causing me to loose an
overview of what I have installed and what is being installed.  Every
dependency a package brings with it (especially packages like dash &
co.) is an argument against using it, imo.

> Lynn

reply via email to

[Prev in Thread] Current Thread [Next in Thread]