Skip to content

Support multi-argument Pythons (like env python)#79

Merged
JulianEberius merged 1 commit intoJulianEberius:masterfrom
szhu:master
Dec 15, 2015
Merged

Support multi-argument Pythons (like env python)#79
JulianEberius merged 1 commit intoJulianEberius:masterfrom
szhu:master

Conversation

@szhu
Copy link
Copy Markdown
Contributor

@szhu szhu commented Nov 16, 2015

The changes in this commit add support for things like #!/usr/bin/env python3 and make errors easier to find. Here are the details:

  1. Support multi-argument Python interpreters

    Many people specify a Python interpreter with a line like this: /usr/bin/env python3. However, SublimePythonIDE allows only to specify the path to a Python interpreter. This would be less portable: Python 3 might be at /usr/bin/python3 on one system and /usr/local/bin/python3 on another. This commit makes it possible.

    How: This commit changes the python interpreter path (the thing named python in proxy_for() and self.python in Proxy) to be a list of strings instead of a single string. This allows for multi-argument Pythons to be specified by any of the Python path providers, including get_setting("python_interpreter") (aka, the python_interpreter setting can now be a list).

    This change is backwards-compatible: Python providers that emit a string will have it converted into the correct format ("/usr/bin/python" -> ["/usr/bin/python"]). A new function, normalize_path, makes normalizing easier.

  2. Auto-detect Python interpreter from shebang line

    Perhaps the most common place to add such a Python interpreter specification is in the shebang (#!) line. This commit will choose the Python interpreter based on this.

    python interpreter autoselect based on #!/ on first line #61 tried to do this earlier, but without support for multi-argument Pythons, it was forced to resolve the path to the actual Python. As such, it only worked for absolute paths to Python and env and for env it had to simulate env's behavior (it "simulated" this by assuming Python is always in the same directory as env).

    This change is backwards-compatible: If the first line of the current file does not start with #!, the other Python path providers are tried.

  3. More informative error messages

    This commit makes the "Could not detect python" message more informative by indicating which paths were tried. Here's an example:

@szhu szhu force-pushed the master branch 2 times, most recently from 6b9477c to 554c5e1 Compare November 16, 2015 12:02
@szhu
Copy link
Copy Markdown
Contributor Author

szhu commented Nov 16, 2015

For people who want to try this out:

cd ~/'Library/Application Support/Sublime Text 3/Packages/'
rm -rf 'SublimePythonIDE'
git clone 'https://github.com/szhu/SublimePythonIDE.git'

@JulianEberius
Copy link
Copy Markdown
Owner

Thanks a lot for this contribution. Looks fine on first sight. I'll try to find some free time look over it and merge as soon as I can.

Thanks again
Julian

The changes in this commit add support for things like `#!/usr/bin/env python3` and make errors easier to find. Here are the details:

1. **Support multi-argument Python interpreters**

   Many people specify a Python interpreter with a line like this: `/usr/bin/env python3`. However, SublimePythonIDE allows only to specify the path to a Python interpreter. This would be less portable: Python 3 might be at `/usr/bin/python3` on one system and `/usr/local/bin/python3` on another. This commit makes it possible.

   How: This commit changes the python interpreter path (the thing named `python` in `proxy_for()` and `self.python` in `Proxy`) to be a list of strings instead of a single string. This allows for multi-argument Pythons to be specified by any of the Python path providers, including `get_setting("python_interpreter")` (aka, the `python_interpreter` setting can now be a list).

   This change is backwards-compatible: Python providers that emit a string will have it converted into the correct format (`"/usr/bin/python" -> ["/usr/bin/python"]`). A new function, `normalize_path`, makes normalizing easier.

2. **Auto-detect Python interpreter from shebang line**

   Perhaps the most common place to add such a Python interpreter specification is in the shebang (`#!`) line. This commit will choose the Python interpreter based on this.

   JulianEberius/pull/61 tried to do this earlier, but without support for multi-argument Pythons, it was forced to resolve the path to the actual Python. As such, it only worked for absolute paths to Python and `env` and for `env` it had to simulate `env`'s behavior (it "simulated" this by assuming Python is always in the same directory as `env`).

   This change is backwards-compatible: If the first line of the current file does not start with `#!`, the other Python path providers are tried.

3. **More informative error messages**

   This commit makes the "Could not detect python" message more informative by indicating which paths were tried. Here's an example:
   <img width="784" src="https://cloud.githubusercontent.com/assets/1570168/11181148/d40ce048-8c14-11e5-9990-f010174455f5.png">
@szhu
Copy link
Copy Markdown
Contributor Author

szhu commented Dec 6, 2015

@JulianEberius, any updates on this? (I've been using it for a while and nothing's broken yet.)

@JulianEberius
Copy link
Copy Markdown
Owner

No leisure time at the moment. Will get better after this week, so timeframe would be between next week and the end of the year. Sorry, no better estimate available :-)

@JulianEberius
Copy link
Copy Markdown
Owner

Hi,

I've merged a minimally changed version. If you can find some time, it would be great if you could test the current master. In addition to your contribution, it contains some bug fixes and updated bundled libraries.

Before I release a new (tagged) version for Package Control, it definitely needs some more testing, as well as documentation.

Anyway, thanks a lot for your contribution
Julian

@szhu
Copy link
Copy Markdown
Contributor Author

szhu commented Dec 16, 2015

Thanks Julian! 0e8faa6 breaks some functionality see 0e8faa6#commitcomment-15001630 for more details. I tried rewriting that error message code while still doing "check for python iteratively only as necessary", but it turned out to be too big of a task. Let me know what your thoughts are on this.

@JulianEberius
Copy link
Copy Markdown
Owner

And that's why they say that premature micro-optimization is the root of all evil... I'll have a look at it later. Thanks for paying attention :-)

@JulianEberius
Copy link
Copy Markdown
Owner

Rewrote the iterative Python detection and improved the error messaging and documentation. Care to take another look at the master branch?

@szhu
Copy link
Copy Markdown
Contributor Author

szhu commented Dec 17, 2015

Thanks for that, it's perfect! Now everything works :) I've submitted some aesthetic improvements as #80.

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