ioquatix / script-runner

This package will run various script files inside of Atom. It currently supports JavaScript, CoffeeScript, Ruby, and Python. You can add more.
http://atom.io/packages/script-runner
Other
62 stars 23 forks source link

Remove Windows support. For now... Sorry. #27

Closed huba closed 8 years ago

huba commented 9 years ago

It's probably for the best to prevent apm from installing it on windows, since execution is never going to get past fetching the shell executable.

huba commented 9 years ago

@ioquatix we should probably also make a note in the readme that ideas are welcome.

Glavin001 commented 9 years ago

Is the problem fetching the Shell environment variables? See https://github.com/ioquatix/script-runner/blob/master/lib/script-runner.coffee#L34-L52

If that is the case, then how about just wrapping that code in a check to see if it is Windows or not.

For instance, from https://github.com/Glavin001/atom-beautify/blob/4915c38b2a31d3a3b7053f8dab254a0f94d0bb20/lib/langs/cli-beautify.coffee#L42-L43

isWin = /^win/.test(process.platform)

Then Windows users would be supported, and if they have PATH or other environment variable issues, they could handle that on their own in their Atom Init Script.

huba commented 9 years ago

@Glavin001 Thanks for the idea. It would be possible to just use process.env instead of extracting an environment from the user's preferred interactive shell on Windows. There might still be some issues with the python pty wrapper.

ioquatix commented 9 years ago

This makes sense and as such I've forked out the code into shell-environment. Let's get that sorted and with Windows support and then integrate it back into script runner.

The other blocker is the pty support. I guess we should probably just find and existing package which does cross-platform PTY or make one?

huba commented 9 years ago

@ioquatix Python docs say that the pty code should work on any platform although it hasn't been tested yet.

https://github.com/Gottox/child_pty could be a good place to start if we want to make that a node module too.

Glavin001 commented 9 years ago

Great! I'll contribute to shell-environment. Hopefully we can get this working cross-platform and add many tests.

My current version for the next release of Atom Beautify: https://github.com/Glavin001/atom-beautify/blob/240368fb09efaf67b31f10f0cf7f3fdba46fa65f/src/beautifiers/beautifier.coffee#L80-L131

###
    Get Shell Environment variables
    Special thank you to @ioquatix
    See https://github.com/ioquatix/script-runner/blob/v1.5.0/lib/script-runner.coffee#L45-L63
    ###
    _envCache: null
    _envCacheDate: null
    _envCacheExpiry: 10000 # 10 seconds
    getShellEnvironment: ->
        return new @Promise((resolve, reject) =>
            # Check Cache
            if @_envCache? and @_envCacheDate?
                # Check if Cache is old
                if (new Date() - @_envCacheDate) < @_envCacheExpiry
                    # Still fresh
                    return resolve(@_envCache)

            # Check if Windows
            isWin = /^win/.test(process.platform)
            if isWin
                # Windows
                # Use default
                resolve(process.env)
            else
                # Mac & Linux
                # I tried using ChildProcess.execFile but there is no way to set detached and
                # this causes the child shell to lock up.
                # This command runs an interactive login shell and
                # executes the export command to get a list of environment variables.
                # We then use these to run the script:
                child = spawn process.env.SHELL, ['-ilc', 'env'],
                  # This is essential for interactive shells, otherwise it never finishes:
                  detached: true,
                  # We don't care about stdin, stderr can go out the usual way:
                  stdio: ['ignore', 'pipe', process.stderr]
                # We buffer stdout:
                buffer = ''
                child.stdout.on 'data', (data) -> buffer += data
                # When the process finishes, extract the environment variables and pass them to the callback:
                child.on 'close', (code, signal) =>
                    if code isnt 0
                        return reject(new Error("Could not get Shell Environment. Exit code: "+code+", Signal: "+signal))
                    environment = {}
                    for definition in buffer.split('\n')
                        [key, value] = definition.split('=', 2)
                        environment[key] = value if key != ''
                    # Cache Environment
                    @_envCache = environment
                    @_envCacheDate = new Date()
                    resolve(environment)
          )
huba commented 8 years ago

As far as I'm aware this issue has been solved by shell-environment.

ioquatix commented 8 years ago

Awesome.