jwiegley / emacs-async

Simple library for asynchronous processing in Emacs
GNU General Public License v3.0
832 stars 68 forks source link

async: allow passing of initfile to child process (FR#39) #73

Closed stsquad closed 1 year ago

stsquad commented 7 years ago

Sometimes passing all your state setup in the async elisp call can be painful, especially if the library is being used by another one (in my case ob-async for org-mode). Luckily we have a method for dealing with boilerplate setup called an initfile. This allows an initfile to be specified for all async calls. It does not re-use the existing initfile as the interactive one is likely too cluttered for a child process.

jwiegley commented 7 years ago

@thierryvolpiatto What do you think?

thierryvolpiatto commented 7 years ago

John Wiegley notifications@github.com writes:

@thierryvolpiatto What do you think?

Passing THE init file to async is not a good idea IMHO, it is too much things to load that will never be used, however loading a small file with just the code needed, why not, but I hardly see how to provide the location of such file easily when writing an emacs extension using async, I thing using async-inject-variables is a much better idea. In addition of async-inject-variables one can also use require or load at beginning of the async-start function to load what's needed.

-- Thierry

stsquad commented 7 years ago

The use-case that I cam across was with ob-sync. It calls async with the following:

(async-start
 `(lambda ()
    ;; TODO: Put this in a function so it can be overidden
    ;; Initialize the new Emacs process with org-babel functions
    (setq exec-path ',exec-path)
    (package-initialize)
    (org-babel-do-load-languages 'org-babel-load-languages ',org-babel-load-languages)
    (,cmd ,body ',params))

And the org-babel-do-load-languages fails in the child because the path isn't setup for all the languages I have defined in org-babel-load-languages in the parent Emacs. However with this patch I set async-child-init to point to a very simple mininit that just sets the paths up. This saves me having to hack the ob-async code to inject the load-path for me which might not always be the solution (it assumes package-initialize solves all the problems).

thierryvolpiatto commented 7 years ago

Alex Bennée notifications@github.com writes:

The use-case that I cam across was with ob-sync. It calls async with the following:

(async-start `(lambda () ;; TODO: Put this in a function so it can be overidden ;; Initialize the new Emacs process with org-babel functions (setq exec-path ',exec-path) (package-initialize) (org-babel-do-load-languages 'org-babel-load-languages ',org-babel-load-languages) (,cmd ,body ',params))

And the org-babel-do-load-languages fails in the child because the path isn't setup for all the languages I have defined in org-babel-load-languages in the parent Emacs. However with this patch I set async-child-init to point to a very simple mininit that just sets the paths up. This saves me having to hack the ob-async code to inject the load-path for me which might not always be the solution (it assumes package-initialize solves all the problems).

Why don't you add "org-babel" with async-inject-variables and perhaps also "load-path" after (package-initialize) ?

The problem with the mininit file is "where is the file ?", for you no problem but for your users, how do you do ?

Also if you want to load a file, your mininit or whatever file, you can do this in the lambda used in async-start with (load "mininit.el").

It's not I don't want to install your patch which is correct, just want to understand in what this change will help writing emacs extensions using async.

-- Thierry

stsquad commented 7 years ago

ob-async is not my package, just one that I'm using. So for me this allows me to work around an assumption it's making by tweaking the child's environment. However you are right I could send a patch upstream to also include load-path in the async environment.

I would envisage async-child-init being used much like a hook used by users to take into account things the 3rd party package didn't consider.

thierryvolpiatto commented 7 years ago

Alex Bennée notifications@github.com writes:

ob-async is not my package, just one that I'm using.

Ah! Yes, so I understand you can't modify the function. Perhaps you can submit upstream (ob-async) and ask them if they can make the lambda a named function so that you can advice it ?

So for me this allows me to work around an assumption it's making by tweaking the child's environment. However you are right I could send a patch upstream to also include load-path in the async environment.

Yes or even better make a function as suggested above.

I would envisage async-child-init being used much like a hook used by users to take into account things the 3rd party package didn't consider.

The problem is that it will be a global var that will affect other async-start calls and no one can know what will contain this file. i.e If we add such var I will have to let-bind it to nil in async-byte-recompile-directory to ensure users don't use e.g a huge init file here.

-- Thierry

astahlman commented 6 years ago

@stsquad Just happened across this thread.

However you are right I could send a patch upstream to also include load-path in the async environment.

I'm the maintainer of ob-async, and I'd happily merge that patch.

astahlman commented 6 years ago

@stsquad I finally got around to creating the patch in ob-async to support a pre-execute hook. I think the merge of https://github.com/astahlman/ob-async/pull/40 would make this request superfluous. Care to review?

jwiegley commented 1 year ago

I don't mind having this change, if @thierryvolpiatto agrees.

thierryvolpiatto commented 1 year ago

John Wiegley @.***> writes:

I don't mind having this change, if @thierryvolpiatto agrees.

I was reluctant to add this because I guess most people will set async.el to use their init file and then complain async is too slow (most people have config with hundred of packages actually), but I may be wrong.

-- Thierry

jwiegley commented 1 year ago

Well, it is a defvar, so it would only be set at the Emacs Lisp level, and hopefully this will read the docstring before doing so. I don't mind have the capability, though.

thierryvolpiatto commented 1 year ago

John Wiegley @.***> writes:

Well, it is a defvar, so it would only be set at the Emacs Lisp level, and hopefully this will read the docstring before doing so.

If you think it is ok to merge this, I fully trust you, don't know if there is a PR yet though.

I don't mind have the capability, though.

Sorry for my bad english, I don't understand this sentence.

-- Thierry

jwiegley commented 1 year ago

It was my sentence whose English was bad! I don't mind having this capability in the code, i.e., making the feature available to programmers. Since it's not a defcustom, no one will know it's there who doesn't read the source or search for the variable.

thierryvolpiatto commented 1 year ago

John Wiegley @.***> writes:

It was my sentence whose English was bad! I don't mind having this capability in the code, i.e., making the feature available to programmers. Since it's not a defcustom, no one will know it's there who doesn't read the source or search for the variable.

Ah! Ok, I fully understand now! And agree. Thanks.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.*Message ID: @.***>

-- Thierry