ternjs / tern_for_sublime

Sublime Text package adding Tern support
MIT License
803 stars 54 forks source link

Change check_output to check_call #16

Closed martineno closed 11 years ago

martineno commented 11 years ago

The Python version in Sublime Text 2.0 doesn't have subprocess.check_output. It does have check_call which considering the context appears to have the required semantics for this particular function.

This fixes an issue where a user would have to manually run npm install in the plugin directory, because subprocess.check_output would fail with an AttributeError.

marijnh commented 11 years ago

This is how it used to look before f9641b7f6cba1d05f9dbb5e5535c215768b62ff2 (except that with your patch the line below it is accessing a non-existent .output property). My vim on Linux has Python 2.7, so I don't think it generally holds that vim ships with 2.6. Is there a sane way to check for support of a property and base the function we call on that? If not, maybe just base it on the python version that's present?

subhaze commented 11 years ago

Sublime Text 2, for OSX at least, is shipped with 2.6, you stated vim so I assume you got your wires crossed on which plugin repo this is :)

It looks like both functions raise subprocess.CalledProcessError, not sure why that code was removed. Python 2 and Python 3 both support check_call also, so it seems like it would be a safe move without the need of feature/version detection.

subhaze commented 11 years ago

Nevermind... I'm an idiot "otherwise raise CalledProcessError" I read that as "raises" so I guess it doesn't raise it but you could based on the return.

marijnh commented 11 years ago

Should be better with attached patch.