Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Stefan Monnier
>    (cl-multiple-value-bind (hip-crowd lamers)
> -      (cl-values-list
> -       (ibuffer-split-list (lambda (bufmark)
> -     (ibuffer-included-in-filters-p (car bufmark)
> -    filterset))
> -   bmarklist))
> +      (ibuffer-split-list (lambda (bufmark)
> +    (ibuffer-included-in-filters-p (car bufmark)
> +   filterset))
> +  bmarklist)

This is "broken" in the sense that it makes assumptions about how the
"multiple values" are implemented.

We do know that `ibuffer-split-list` returns a list, so there's no point
going through CL's multiple-values simulation.  We should instead treat
it as a list and extract the elements with `pcase-let`
(or `cl-destructuring-bind`).


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero
On Sun, Dec 1, 2019 at 7:29 PM Stefan Monnier <[hidden email]> wrote:

> This is "broken" in the sense that it makes assumptions about how the
> "multiple values" are implemented.

Yes, though we've had that implementation for decades and we're not likely going to add true multiple values to elisp.

> We do know that `ibuffer-split-list` returns a list, so there's no point
> going through CL's multiple-values simulation.  We should instead treat
> it as a list and extract the elements with `pcase-let`
> (or `cl-destructuring-bind`).

I tried to do a minimal fix so close to branching for 27, but I can do that if you want.

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero
Pushed as 9c0ac88199accb4133d9fbf36d3c4adc3705b22f
Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Stefan Monnier
In reply to this post by Juanma Barranquero
>> This is "broken" in the sense that it makes assumptions about how the
>> "multiple values" are implemented.
> Yes, though we've had that implementation for decades and we're not likely
> going to add true multiple values to elisp.

I must admit I'm biased: I find c-lib's multiple-value support to be
completely worthless.  I don't know anything you can do with that you
can't do just as easily without it.  IOW it only makes sense as
a "compatibility with Common-Lisp", yet it doesn't provide the right
semantics either so it doesn't work for that either.

I regret not having noticed it back in Emacs-24 time when I could have
marked those functions as obsolete (or even kept them in cl.el but not
in cl-lib.el).

>> We do know that `ibuffer-split-list` returns a list, so there's no point
>> going through CL's multiple-values simulation.  We should instead treat
>> it as a list and extract the elements with `pcase-let`
>> (or `cl-destructuring-bind`).
> I tried to do a minimal fix so close to branching for 27, but I can do that
> if you want.

I consider the patch you showed as broken (regardless of whether it
does work in practice, obviously).  Require cl-lib is another way to fix
this, if you want a less intrusive patch.  But we're still "before
branching", so I wouldn't worry about intrusiveness yet.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero
On Mon, Dec 2, 2019 at 2:55 AM Stefan Monnier <[hidden email]> wrote:

> I must admit I'm biased: I find c-lib's multiple-value support to be
> completely worthless.  I don't know anything you can do with that you
> can't do just as easily without it.  IOW it only makes sense as
> a "compatibility with Common-Lisp", yet it doesn't provide the right
> semantics either so it doesn't work for that either.
>
> I regret not having noticed it back in Emacs-24 time when I could have
> marked those functions as obsolete (or even kept them in cl.el but not
> in cl-lib.el).

I think we should mark them as obsolete and work to remove them some day.
They add clutter with no real benefit. It's silly to have compatibility
with such a feature while we aren't compatible with other useful features
of Common Lisp, like namespaces (I mean, "packages") or format.

> I consider the patch you showed as broken (regardless of whether it
> does work in practice, obviously).  Require cl-lib is another way to fix
> this, if you want a less intrusive patch.

I've commited a fix with cl-destructuring-bind. This is a case where cl
functions (cl-multiple-value-bind and cl-values-list) didn't add anything
of value (no pun intended).

Once we branch, we could perhaps try using more cl and not less. Not just
in that case, but using cl functions instead of reimplementing them here
and there.

I mean,

  (ibuffer-remove-alist 'key alist) == (cl-remove 'key alist :key #'car)

and ibuffer-remove-duplicates is just a reimplementation of
cl-remove-duplicates, without the bells and whistles.

*But*, I think we should have in place a definite policy about cl-lib.

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Eli Zaretskii
> From: Juanma Barranquero <[hidden email]>
> Date: Mon, 2 Dec 2019 03:20:31 +0100
> Cc: Emacs developers <[hidden email]>
>
> *But*, I think we should have in place a definite policy about cl-lib.

We do.

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero
On Mon, Dec 2, 2019 at 4:40 AM Eli Zaretskii <[hidden email]> wrote:

> We do.

Care to elaborate, please?

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Eli Zaretskii
> From: Juanma Barranquero <[hidden email]>
> Date: Mon, 2 Dec 2019 04:43:00 +0100
> Cc: Stefan Monnier <[hidden email]>, Emacs developers <[hidden email]>
>
> Care to elaborate, please?

Yes, we don't preload packages just because it's convenient.

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero


On Mon, Dec 2, 2019 at 4:38 PM Eli Zaretskii <[hidden email]> wrote:

> Yes, we don't preload packages just because it's convenient.

Oh, well, sorry I was unclear. I was not asking about that. I said that IMO we should preload cl-lib, but I know that's... unwanted.

What I'm really asking is what is our stance regarding using cl-lib in lisp/*. I mean, are we trying to minimize run-time dependencies on cl-lib, or it is ok to use freely?

TIA,

    Juanma


Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Eli Zaretskii
> From: Juanma Barranquero <[hidden email]>
> Date: Mon, 2 Dec 2019 18:18:35 +0100
> Cc: Stefan Monnier <[hidden email]>, Emacs developers <[hidden email]>
>
> What I'm really asking is what is our stance regarding using cl-lib in lisp/*. I mean, are we trying to minimize
> run-time dependencies on cl-lib, or it is ok to use freely?

It's OK to use cl-lib.  It's cl that we don't want (and the byte
compiler will warn about its use at runtime).

Reply | Threaded
Open this post in threaded view
|

Re: master f225011: ibuffer-do-isearch: don't depend on `cl-values-list' (bug#38430)

Juanma Barranquero

On Mon, Dec 2, 2019 at 6:40 PM Eli Zaretskii <[hidden email]> wrote:

> It's OK to use cl-lib.

Great.

> It's cl that we don't want 

Yep, cl is gross.