Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: reloadable init scripts #1110

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

mauricioszabo
Copy link
Contributor

Currently, init scripts are a very powerful feature that I feel are sometimes underused. The fact we need to reload the editor to apply the changes, together with the fact that if we have any error in the init script we will have it applied partially, or even have undesirable results, is also a problem

This PR adds a config "auto reload init script", which will basically do four things:

  1. Change the way we load init script - instead of using require, we will use eval, which will also allow us to reload the script and pass a new parameter - disposable - which will be used to dispose old commands from previous scripts
  2. Watch the init script with Pathwatcher or any other mechanism - so when the user saves the init script, it'll dispose the old commands, and apply the new script
  3. Add an error treatment - when the script have some failure, we'll dispose everything so that the script won't be partially applied, possibly causing unpredictable results
  4. Keep everything the same if the config is not changed

I'm drafting this PR to request for comments, and also to ask for opinions on:

  1. Should we keep the option to load .coffee init scripts? If so, we need to transpile .coffee files before running eval
  2. Where should we watch the init script? Supposedly, we need to watch for changes in the config, so we can decide if we want to watch the file, or if we want to "unwatch" it (if the "auto reload" changes from true to false)
  3. How should we watch? In the keymaps config, we watch using pathwatcher. We also have the nsfw watcher that is built-in into Pulsar, I'm unsure why Atom originally had these two options so I don't really know which one should we use

Missing in this PR:

  • Actually watch the config
  • Check if we need to transpile CoffeeScript files

@savetheclocktower
Copy link
Contributor

Another option might be to figure out how to purge the require cache (just for this one file) so that the init.js or init.coffee can be re-required. This would save you from having to reimplement the transpilation.

You'd need a different way of making the init file’s Disposables available; perhaps a property on the atom global.

@mauricioszabo
Copy link
Contributor Author

I kinda don't like this idea of making the disposable as an atom global. Things can become weird - supposedly it's an API that should be used only by the init script, and not be available elsewhere. Because init scripts can run async code too, it is quite possible, we can't for example add a global atom Disposable and then remove as soon as the init script loads...

@savetheclocktower
Copy link
Contributor

I kinda don't like this idea of making the disposable as an atom global. Things can become weird - supposedly it's an API that should be used only by the init script, and not be available elsewhere. Because init scripts can run async code too, it is quite possible, we can't for example add a global atom Disposable and then remove as soon as the init script loads...

A magic disposables value feels icky to me. Maybe there's a third option? Hanging it off of require? I'd be willing to do something extremely silly if it saves us from having to eval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants