mafredri / zsh-async

Because your terminal should be able to perform tasks asynchronously without external tools!
MIT License
766 stars 34 forks source link

Async jobs should inherit PWD and all environment variables #25

Closed maximbaz closed 6 years ago

maximbaz commented 6 years ago

The life would be much simpler if jobs could inherit all environment variables and current directory from the parent. Currently one has to remember to pass all needed information via job arguments, and that is very annoying, easy to forget and takes time to find out.

Example in pure prompt — this should have been done by zsh-async itself.

I'm asking because people are catching bugs like "forgot to cd in proper folder", or "$PATH is missing some folders", and such bugs will keep coming unless we fix this once and for all in the framework (https://github.com/maximbaz/spaceship-prompt/pull/3)

mafredri commented 6 years ago

Do you think it would suffice to propagate pwd at the time async_job is called? That should be possible.

I agree that we should improve the default behavior if possible. Worried it might break some previous assumptions but I’ll look into this. Thanks for raising the issue.

maximbaz commented 6 years ago

Not only pwd, but if possible also all the other environment variables. For example, you export ABC, then start a job, and inside the job people expect to be able to use the ABC variable - but it is not present.

mafredri commented 6 years ago

Hmm, I'm not sure how I feel about that as the default behavior. I imagine there might be extra overhead in serializing (escaping) all env variables and sending them along with the async job to the worker.

This behavior is documented in Usage. Perhaps it would help if that was made more clear?

Issue #4 is somewhat related to this discussion, there I decided to not introduce any new APIs for updating the environment.

maximbaz commented 6 years ago

I know it's documented, but it is still inconvenient, basically a user spends some time configuring their environment only to realize that async job doesn't inherit their configs. How difficult would it be to measure the overhead, to see the real numbers and decide based on them?

At the very least I would like to see $PWD and $PATH being passed to the worker, because these are wanted most often (in my limited experience), but if the overhead is minimal, I'd much prefer worker inherit all of the variables.

maximbaz commented 6 years ago

I saw the workaround in the mentioned thread is to restart the worker before rendering PROMPT. Without having any numbers and only based on intuition, this sounds like a heavier approach, isn't it? Even if this is not a "official" solution, if this is the easiest one available and every async prompt starts restarting workers all the time, the end users will have a worse experience.

But if I was unable to convince you, I can also make my prompt to restart the worker all the time 🙂

In any case let's propagate the current working directory to the worker.

mafredri commented 6 years ago

How difficult would it be to measure the overhead, to see the real numbers and decide based on them?

I'll look at creating some benchmarks, shouldn't be too hard, a matter of timing some tests, although it won't be super scientific.

At the very least I would like to see $PWD and $PATH being passed to the worker

I agree with pwd, it's common enough. But I'm curious, how often is path modified after the worker is started? At least in Pure, we do a lazy init of the worker (on first prompt).

I saw the workaround in the mentioned thread is to restart the worker before rendering PROMPT.

Personally I don't agree with this approach and I see no reason to prefer it. How many variables is anyone likely to need in a job? My guess is one, maybe two (excl. pwd). Passing them as arguments is easier.

In any case let's propagate the current working directory to the worker.

At least this, I think I can get on board with. But let's see what the benchmarks tell us about serializing the entire env.

maximbaz commented 6 years ago

Thank you! 🙂

I agree with pwd, it's common enough. But I'm curious, how often is path modified after the worker is started? At least in Pure, we do a lazy init of the worker (on first prompt).

Ah, I've just realized that I've been initializing the worker on sourcing the prompt sources, not on the first time the prompt is actually rendered! So if I have some modifications to the PATH after the prompt is sourced, these are not seen by the async worker.

Let's see the results of the benchmarks, if serialization has 0 impact - let's have this feature, if it proves to be heavy - I agree with only propagating the working directory, and you gain a strong argument for future cases when someone asks for this again 😃

annacrombie commented 6 years ago

Here is another use case for especially an inherited PWD:

I use xcwd to make it so that when I open a new terminal window it has the same pwd as the window that I had focused when I opened it. xcwd makes this work by looking at the deepest process of the currently focused window and returning that processe's PWD. Until now I haven't experienced any issues, but now that I started using pure and by extension async.zsh, the async worker is always the deepest process, and since it doesn't inherit the PWD, new terminal windows always open to my home directory.

In this case, the pwd would need to be updated in real time, not only propogated when a function is called.