Add autoload cookies in seq.el

Add autoload cookies in seq.el

[Tino Calancha]

Hi,

lisp/emacs-lisp/seq.el
lack of autoload cookies.

I see in the master branch that some files are starting
to use funtions from this library.
For instance,
* lisp/subr.el (read-multiple-choice)
* lisp/ibuffer.el (define-ibuffer-column name)
(it may be more cases, i didn't look in detail).

We should:
I) Add autoload cookies for the most useful functions
    defined in seq.el (as it was done in cl-seq).
II) Or add (require 'seq) in those files using such functions.

Re: Add autoload cookies in seq.el

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:

> Hi,
>
> lisp/emacs-lisp/seq.el
> lack of autoload cookies.
>
> I see in the master branch that some files are starting
> to use funtions from this library.

seq.el being a library with very few private functions, pretty much all
functions would have to be autoloaded then.

> We should:
> I) Add autoload cookies for the most useful functions
>     defined in seq.el (as it was done in cl-seq).
> II) Or add (require 'seq) in those files using such functions.

Where did you see usages of seq.el without a `require'?

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Add autoload cookies in seq.el

[Tino Calancha]

> Where did you see usages of seq.el without a `require'?
I saw for in following functions:

* lisp/subr.el (read-multiple-choice); line 2314
* lisp/ibuffer.el (define-ibuffer-column name) ; line 1746

(it may be more, i just found them accidentally).

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> Date: Fri, 20 May 2016 17:54:23 +0900 (JST)
> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: emacs-devel@gnu.org, Tino Calancha <tino.calancha@gmail.com>
> 
> 
> > Where did you see usages of seq.el without a `require'?
> I saw for in following functions:
> 
> * lisp/subr.el (read-multiple-choice); line 2314

IMO, it makes no sense to use a function in a preloaded file if the
definition of that function is not also preloaded.

Why is read-multiple-choice in subr.el in the first place?

Re: Add autoload cookies in seq.el

[Tino Calancha]

> IMO, it makes no sense to use a function in a preloaded file if the
> definition of that function is not also preloaded.
>
> Why is read-multiple-choice in subr.el in the first place?
Sorry, I cannot answer that question.
I just did:
grep seq-position lisp/*.el

and noticed that 'read-multiple-choice' uses seq.el without requiring.
That's all.  I didn't know even that function exist.

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> Date: Sat, 21 May 2016 09:07:38 +0900 (JST)
> From: Tino Calancha <tino.calancha@gmail.com>
> cc: Tino Calancha <tino.calancha@gmail.com>, nicolas@petton.fr, 
>     emacs-devel@gnu.org
> 
> > IMO, it makes no sense to use a function in a preloaded file if the
> > definition of that function is not also preloaded.
> >
> > Why is read-multiple-choice in subr.el in the first place?
> Sorry, I cannot answer that question.

Adding Lars who made that change.

Lars, why did you put read-multiple-choice in subr.el?  Its only user
currently is message.el, which is not preloaded, so I think preloading
seq.el as result of this is not justified.

Re: Add autoload cookies in seq.el

[Tino Calancha]

> Where did you see usages of seq.el without a `require'?
Hi Nico, i have performed an exhaustive search of files
calling functons from seq.el without requiring it.
Following is the result:

* lisp/subr.el (read-multiple-choice): seq-position ; line 2314
* lisp/ibuffer.el (define-ibuffer-column name): seq-position ; line 1746
* lisp/emacs-lisp/find-func.el (find-library--from-load-path):
   seq-filter ; line 223
* lisp/image.el (image--get-image): seq-find ; line 980


In GNU Emacs 25.1.50.2 (x86_64-pc-linux-gnu, GTK+ Version 2.24.30)
Repository revision: 162bc021a1cb4d7f578d1635a55bb7c38084f044

Tino

Re: Add autoload cookies in seq.el

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:

> Hi Nico, i have performed an exhaustive search of files
> calling functons from seq.el without requiring it.
> Following is the result:
>
> * lisp/subr.el (read-multiple-choice): seq-position ; line 2314
> * lisp/ibuffer.el (define-ibuffer-column name): seq-position ; line 1746
> * lisp/emacs-lisp/find-func.el (find-library--from-load-path):
>    seq-filter ; line 223
> * lisp/image.el (image--get-image): seq-find ; line 980

Thanks, I fixed the missing dependencies to seq.el (except for the one
in subr.el).

I think we should move `read-multiple-choice' out of subr.el, or rewrite
it to remove the dependency to seq.el.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Add autoload cookies in seq.el

[Tino Calancha]

> Thanks, I fixed the missing dependencies to seq.el

Hi nico,

after last change in
* lisp/image.el

building Emacs fails as follows:

Loading emacs-lisp/byte-run...
Loading emacs-lisp/backquote...
Loading subr...
Loading version...
Loading widget...
Loading custom...
Loading emacs-lisp/map-ynp...
Loading international/mule...
Loading international/mule-conf...
Loading env...
Loading format...
Loading bindings...
Loading window...
Loading files...
Loading emacs-lisp/macroexp...
Loading cus-face...
Loading faces...
Loading button...
Loading loaddefs.el (source)...
Loading emacs-lisp/nadvice...
Loading emacs-lisp/cl-preloaded...
Loading minibuffer...
Loading obarray...
Loading abbrev...
Loading simple...
Loading help...
Loading jka-cmpr-hook...
Loading epa-hook...
Loading international/mule-cmds...
Loading case-table...
Loading international/charprop.el (source)...
Loading international/characters...
Loading international/charscript...
Loading composite...
Loading language/chinese...
Loading language/cyrillic...
Loading language/indian...
Loading language/sinhala...
Loading language/english...
Loading language/ethiopic...
Loading language/european...
Loading language/czech...
Loading language/slovak...
Loading language/romanian...
Loading language/greek...
Loading language/hebrew...
Loading international/cp51932...
Loading international/eucjp-ms...
Loading language/japanese...
Loading language/korean...
Loading language/lao...
Loading language/tai-viet...
Loading language/thai...
Loading language/tibetan...
Loading language/vietnamese...
Loading language/misc-lang...
Loading language/utf-8-lang...
Loading language/georgian...
Loading language/khmer...
Loading language/burmese...
Loading language/cham...
Loading indent...
Loading emacs-lisp/cl-generic...
Loading frame...
Loading startup...
Loading term/tty-colors...
Loading font-core...
Loading facemenu...
Loading emacs-lisp/syntax...
Loading font-lock...
Loading jit-lock...
Loading mouse...
Loading scroll-bar...
Loading select...
Loading emacs-lisp/timer...
Loading isearch...
Loading rfn-eshadow...
Loading menu-bar...
Loading emacs-lisp/lisp...
Loading textmodes/page...
Loading register...
Loading textmodes/paragraphs...
Loading progmodes/prog-mode...
Loading emacs-lisp/lisp-mode...
Loading progmodes/elisp-mode...
Loading textmodes/text-mode...
Loading textmodes/fill...
Loading newcomment...
Loading replace...
Loading emacs-lisp/tabulated-list...
Loading buff-menu...
Loading fringe...
Loading emacs-lisp/regexp-opt...
Loading image...
(require seq) while preparing to dump
Makefile:545: recipe for target 'emacs' failed
make[2]: *** [emacs] Error 255


In GNU Emacs 25.1.50.2 (x86_64-pc-linux-gnu, GTK+ Version 2.24.30)
Repository revision: 2dc0ef29425b7ee4c6c13b832c2a11dfe26ebb35

With following patch i can build Emacs successfully:

diff --git a/lisp/image.el b/lisp/image.el
index a6464f7..e5550c5 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -25,7 +25,6 @@

  ;;; Code:

-(require 'seq)

  (defgroup image ()
    "Image support."
@@ -975,6 +974,7 @@ image-decrease-size
                          0.8)))

  (defun image--get-image ()
+  (require 'seq)
    (let ((image (or (get-text-property (point) 'display)
                     ;; `put-image' uses overlays, so find an image in
                     ;; the overlays.

Re: Add autoload cookies in seq.el

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:

>> Thanks, I fixed the missing dependencies to seq.el
>
> Hi nico,
>
> after last change in
> * lisp/image.el
>
> building Emacs fails as follows:

Thanks, I'll fix it ASAP.

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Add autoload cookies in seq.el

[Paul Eggert]

On 05/22/2016 06:49 PM, Tino Calancha wrote:
> With following patch i can build Emacs successfully: 

Thanks, the master build was failing for me too, so I installed that patch.

Re: Add autoload cookies in seq.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Lars, why did you put read-multiple-choice in subr.el?  Its only user
> currently is message.el, which is not preloaded, so I think preloading
> seq.el as result of this is not justified.

Perhaps subr-x.el would be a better place for it...

But I find it rather odd that we can't use the best and most modern
libraries in the most central parts of Emacs.  That's kinda backwards.
The most central parts of Emacs gets the most obfuscated code?

Not dumping seq with Emacs seems rather odd, because it's going to get
used a while lot.  That's the point of having a library like that.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Tino Calancha <tino.calancha@gmail.com>,  nicolas@petton.fr,  emacs-devel@gnu.org
> Date: Wed, 20 Jul 2016 15:24:01 +0200
> 
> But I find it rather odd that we can't use the best and most modern
> libraries in the most central parts of Emacs.  That's kinda backwards.
> The most central parts of Emacs gets the most obfuscated code?

Are we mis-communicating?  If somefile.el needs a function that is in
subr-x.el, all it has to do is (require subr-x).  How's that a
problem?

> Not dumping seq with Emacs seems rather odd, because it's going to get
> used a while lot.  That's the point of having a library like that.

When/if seq.el will become used so much that it will almost always
immediately load at the beginning of a session, we will indeed add it
to preloaded ones.  But as long as that doesn't happen, why should we
punish those users who have no use for seq?

Re: Add autoload cookies in seq.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Are we mis-communicating?  If somefile.el needs a function that is in
> subr-x.el, all it has to do is (require subr-x).  How's that a
> problem?

We seem to be.  That paragraph was about seq.el.

> When/if seq.el will become used so much that it will almost always
> immediately load at the beginning of a session, we will indeed add it
> to preloaded ones.  But as long as that doesn't happen, why should we
> punish those users who have no use for seq?

Every time somebody introduces seq-based code into the dumped files,
it's rewritten since we can't use seq, since it's not dumped.  seq was
written as a library meant to regularise all the different inconsistent
libraries and functions we use for that stuff.

Until seq stops being persona non grata in the most important Emacs Lisp
files, it'll probably not see much uptake in the rest of Emacs, either.
Who wants to go around remembering two different interfaces for lists,
one that's OK in subr.el and one that's not?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Add autoload cookies in seq.el

[Ted Zlatanov]

On Thu, 21 Jul 2016 12:51:21 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote: 

LI> Until seq stops being persona non grata in the most important Emacs Lisp
LI> files, it'll probably not see much uptake in the rest of Emacs, either.
LI> Who wants to go around remembering two different interfaces for lists,
LI> one that's OK in subr.el and one that's not?

FWIW I am in favor of making seq.el a first-class citizen.

Ted

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tino.calancha@gmail.com,  nicolas@petton.fr,  emacs-devel@gnu.org
> Date: Thu, 21 Jul 2016 12:51:21 +0200
> 
> > Are we mis-communicating?  If somefile.el needs a function that is in
> > subr-x.el, all it has to do is (require subr-x).  How's that a
> > problem?
> 
> We seem to be.  That paragraph was about seq.el.

Was it?  Here's what I saw and responded to:

> > Lars, why did you put read-multiple-choice in subr.el?  Its only user
> > currently is message.el, which is not preloaded, so I think preloading
> > seq.el as result of this is not justified.
> 
> Perhaps subr-x.el would be a better place for it...
> 
> But I find it rather odd that we can't use the best and most modern
> libraries in the most central parts of Emacs.  That's kinda backwards.
> The most central parts of Emacs gets the most obfuscated code?

IOW, from my POV some code which was only used by message.el (which is
not preloaded, and was never supposed to be) was added to subr.el,
which then required to preload seq.el.  How is that related to the
following complaint of yours:

> Every time somebody introduces seq-based code into the dumped files,
> it's rewritten since we can't use seq, since it's not dumped.  seq was
> written as a library meant to regularise all the different inconsistent
> libraries and functions we use for that stuff.

Since message.el is not a preloaded file, you are free to use
seq-based code in it, which will cause seq.el to be loaded when
message.el is.  What I'm objected to is to put the code which uses
seq.el in a preloaded file, when its _only_ user is not preloaded.  I
think such a policy makes sense; don't you?

> Until seq stops being persona non grata in the most important Emacs Lisp
> files, it'll probably not see much uptake in the rest of Emacs, either.
> Who wants to go around remembering two different interfaces for lists,
> one that's OK in subr.el and one that's not?

Maybe I'm misremembering, but I only saw discussions about preloading
seq once or twice, including this discussion.  So I'm not sure where
does "persona non grata" thing come from.  Can you point me to those
incidents which led you to this conclusion?

Re: Add autoload cookies in seq.el

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Until seq stops being persona non grata in the most important Emacs Lisp
> files, it'll probably not see much uptake in the rest of Emacs, either.

I presume the situation was (is) more like "let's see how many people
will use that thing".  In that case, maybe we can come to the conclusion
"enough" now.


Michael.

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> From: Ted Zlatanov <tzz@lifelogs.com>
> Date: Thu, 21 Jul 2016 09:27:49 -0400
> 
> FWIW I am in favor of making seq.el a first-class citizen.

It is already a first-class citizen.

Re: Add autoload cookies in seq.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Lars, why did you put read-multiple-choice in subr.el?  Its only user
>> > currently is message.el, which is not preloaded, so I think preloading
>> > seq.el as result of this is not justified.
>> 
>> Perhaps subr-x.el would be a better place for it...
>> 
>> But I find it rather odd that we can't use the best and most modern
>> libraries in the most central parts of Emacs.  That's kinda backwards.
>> The most central parts of Emacs gets the most obfuscated code?
>
> IOW, from my POV some code which was only used by message.el (which is
> not preloaded, and was never supposed to be) was added to subr.el,
> which then required to preload seq.el.  How is that related to the
> following complaint of yours:

I changed subject between the first and second paragraphs.

> Maybe I'm misremembering, but I only saw discussions about preloading
> seq once or twice, including this discussion.  So I'm not sure where
> does "persona non grata" thing come from.  Can you point me to those
> incidents which led you to this conclusion?

commit 9c969e1f848e65b24e06d3919cde9a7ae668bfb8
Author: Glenn Morris <rgm@gnu.org>
Date:   Mon May 23 12:48:19 2016 -0400

    * lisp/image.el (image--get-image): Avoid requiring a library for
    one trivial operation.

commit c6493c68f8fd6a5e6893eb3837a1a27f8d8a98a8
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Mon May 23 08:13:03 2016 -0700

    Fix seq requirement that broke bootstrap
    
    * lisp/image.el (image--get-image): Require seq here, not at the
    top level, to avoid ‘(require seq) while preparing to dump’ while
    bootstrapping.  Suggested by Tino Calancha in:
    http://lists.gnu.org/archive/html/emacs-devel/2016-05/msg00477.html

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Add autoload cookies in seq.el

[Stefan Monnier]

>> Until seq stops being persona non grata in the most important Emacs Lisp
>> files, it'll probably not see much uptake in the rest of Emacs, either.
> I presume the situation was (is) more like "let's see how many people
> will use that thing".  In that case, maybe we can come to the conclusion
> "enough" now.

There's also the fact that `seq' uses `cl-lib', so if we want to preload
one we have to preload the other as well.


        Stefan

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tino.calancha@gmail.com,  nicolas@petton.fr,  emacs-devel@gnu.org
> Date: Thu, 21 Jul 2016 17:01:29 +0200
> 
> > Maybe I'm misremembering, but I only saw discussions about preloading
> > seq once or twice, including this discussion.  So I'm not sure where
> > does "persona non grata" thing come from.  Can you point me to those
> > incidents which led you to this conclusion?
> 
> commit 9c969e1f848e65b24e06d3919cde9a7ae668bfb8
> Author: Glenn Morris <rgm@gnu.org>
> Date:   Mon May 23 12:48:19 2016 -0400
> 
>     * lisp/image.el (image--get-image): Avoid requiring a library for
>     one trivial operation.
> 
> commit c6493c68f8fd6a5e6893eb3837a1a27f8d8a98a8
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Date:   Mon May 23 08:13:03 2016 -0700
> 
>     Fix seq requirement that broke bootstrap
>     
>     * lisp/image.el (image--get-image): Require seq here, not at the
>     top level, to avoid ‘(require seq) while preparing to dump’ while
>     bootstrapping.  Suggested by Tino Calancha in:
>     http://lists.gnu.org/archive/html/emacs-devel/2016-05/msg00477.html

That's a single issue that got 2 separate changes.

So my memory was correct in this case.  I'd like this library to have
more (potential) users before we preload it (and its prerequisites).

Thanks.

Re: Add autoload cookies in seq.el

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> So my memory was correct in this case.  I'd like this library to have
> more (potential) users before we preload it (and its prerequisites).

OK; so somebody else (who has to be unaware that we can't use seq
functions in the central Lisp files) has to add something dependent on
seq a couple of times, and then it'll have to be rewritten by a
different maintainer a couple of times, and then it'll be OK.

As long as we have a plan.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Add autoload cookies in seq.el

[Ted Zlatanov]

On Fri, 22 Jul 2016 11:04:02 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote: 

LI> Eli Zaretskii <eliz@gnu.org> writes:
>> So my memory was correct in this case.  I'd like this library to have
>> more (potential) users before we preload it (and its prerequisites).

LI> OK; so somebody else (who has to be unaware that we can't use seq
LI> functions in the central Lisp files) has to add something dependent on
LI> seq a couple of times, and then it'll have to be rewritten by a
LI> different maintainer a couple of times, and then it'll be OK.

That seems reasonable. We just need some seq puppets. (collective groan)

Ted

Re: Add autoload cookies in seq.el

[Stefan Monnier]

>>> Until seq stops being persona non grata in the most important Emacs Lisp
>>> files, it'll probably not see much uptake in the rest of Emacs, either.
>> I presume the situation was (is) more like "let's see how many people
>> will use that thing".  In that case, maybe we can come to the conclusion
>> "enough" now.
> There's also the fact that `seq' uses `cl-lib', so if we want to preload
> one we have to preload the other as well.

Of course, rather than preload cl-lib, we might be able to reverse
the dependency.


        Stefan

Re: Add autoload cookies in seq.el

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

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

> Of course, rather than preload cl-lib, we might be able to reverse
> the dependency.

seq.el uses a lot of generic functions, as well as `cl-subseq', but this
one should be easy to get rid of.

BTW, I don't know if seq.el should be preloaded or not, but if it is,
I'd preload map.el as well, they go hand in hand for different data
structures.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Add autoload cookies in seq.el

[Stefan Monnier]

>> Of course, rather than preload cl-lib, we might be able to reverse
>> the dependency.
> seq.el uses a lot of generic functions,

That's OK: cl-generic is already preloaded, actually.


        Stefan

Re: Add autoload cookies in seq.el

[Eli Zaretskii]

> From: Nicolas Petton <nicolas@petton.fr>
> Date: Fri, 22 Jul 2016 17:07:30 +0200
> 
> BTW, I don't know if seq.el should be preloaded or not, but if it is,
> I'd preload map.el as well, they go hand in hand for different data
> structures.

Once again, we don't decide on preloading a package by these criteria.
A package needs to be _needed_ by preloaded code before it becomes a
candidate for preloading.  Its general utility, by itself, is
irrelevant.

IOW, having a package not preloaded doesn't make it second-class in
any way.  I wonder where this idea came from.  It is profoundly
false.  By that logic, Dired is not first-class citizen.

Re: Add autoload cookies in seq.el

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> IOW, having a package not preloaded doesn't make it second-class in
> any way.  I wonder where this idea came from.  It is profoundly
> false.  By that logic, Dired is not first-class citizen.

Dired is not a library like seq is.  Still, I do understand your point.

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]