Merge lp:~yagnesh/python-mode/py-shell into lp:python-mode

[yagnesh]

py-choose-shell should look at py-shell-name before guessing

[Andreas Roehler (a-roehler)]

Hi,

as `py-shell-name' will exist with some probability, don't see
how `py-choose-shell-by-shebang' get's a chance that way.

Do you experienced a related bug prior to this change?
Would prefer to proceed with some example.

Thanks contributing,

Andreas

[yagnesh (yagnesh)]

On 01/26/2012 04:45 PM, Andreas Roehler wrote:
> Hi,
>
> as `py-shell-name' will exist with some probability, don't see
> how `py-choose-shell-by-shebang' get's a chance that way.
>
> Do you experienced a related bug prior to this change?
> Would prefer to proceed with some example.
>
> Thanks contributing,
>
> Andreas

Hello Andreas,

lets say I have this in my .emacs

---------------------------------------------------
(require 'python-mode)
(add-hook 'python-mode-hook
           (lambda ()
             (setq py-shell-name "ipython")))
---------------------------------------------------

now I open a file with shebang containing

#!/usr/bin/env python3

and press C-c C-c it takes me to a python3 shell instead of ipython.
to make sure I verified with emacs -q and results the same.

What I expected is from
  ~ C-c C-c ~ to take me to *ipython* shell and inform
about my code is actually python3

Read further only If you are able to reproduce my report otherwise feel
free to skip.

As I gone through the code for C-c C-c (py-execute-buffer) it is calling
py-choose-shell down the line where my preference got overwritten.

After sending this patch I realized that it is bad idea idea to ignore
the shebang.

So the possible choices py-choose-shell should analyze:

shebang py-shell-name Callable
 winner
python python3 python shebang
python3 python python3 shebang
python ipython ipython py-shell-name
python3 ipython ipython py-shell-name

again calling Ipython is kind of problematic because we have to make choices
whether to call with 'exec' or 'execfile'.

So my conclusion was to have a internal local variable to store the
py-choose-shell-shebang return and compare that to py-shell-name to get
final the *shell* value. (gosh..)

Sorry for long reply. I am entry level elisper So please ignore any of
the above if it doesn't make any sense

and sorry for the long the long reply

[yagnesh (yagnesh)]

Thats a table mess.,

--------------------------------------------------------------
shebang py-shell-name Callable winner
--------------------------------------------------------------
python python3 python shebang
python3 python python3 shebang
python ipython ipython py-shell-name
python3 ipython ipython py-shell-name
--------------------------------------------------------------

another try.

[Andreas Roehler (a-roehler)]

Am 26.01.2012 09:43, schrieb yagnesh:
> On 01/26/2012 04:45 PM, Andreas Roehler wrote:
>> Hi,
>>
>> as `py-shell-name' will exist with some probability, don't see
>> how `py-choose-shell-by-shebang' get's a chance that way.
>>
>> Do you experienced a related bug prior to this change?
>> Would prefer to proceed with some example.
>>
>> Thanks contributing,
>>
>> Andreas
>
>
> Hello Andreas,
>
> lets say I have this in my .emacs
>
> ---------------------------------------------------
> (require 'python-mode)
> (add-hook 'python-mode-hook
> (lambda ()
> (setq py-shell-name "ipython")))
> ---------------------------------------------------
>
> now I open a file with shebang containing
>
> #!/usr/bin/env python3
>
> and press C-c C-c it takes me to a python3 shell instead of ipython.
> to make sure I verified with emacs -q and results the same.
>
> What I expected is from
> ~ C-c C-c ~ to take me to *ipython* shell and inform
> about my code is actually python3
>
> Read further only If you are able to reproduce my report otherwise feel
> free to skip.
>
> As I gone through the code for C-c C-c (py-execute-buffer) it is calling
> py-choose-shell down the line where my preference got overwritten.
>
> After sending this patch I realized that it is bad idea idea to ignore
> the shebang.
>
> So the possible choices py-choose-shell should analyze:
>
>
> shebang py-shell-name Callable
> winner
> python python3 python shebang
> python3 python python3 shebang
> python ipython ipython py-shell-name
> python3 ipython ipython py-shell-name
>
>
>
> again calling Ipython is kind of problematic because we have to make choices
> whether to call with 'exec' or 'execfile'.
>
> So my conclusion was to have a internal local variable to store the
> py-choose-shell-shebang return and compare that to py-shell-name to get
> final the *shell* value. (gosh..)
>
>
> Sorry for long reply. I am entry level elisper So please ignore any of
> the above if it doesn't make any sense
>
> and sorry for the long the long reply
>
>

Hi Yagnesh,

please make a bug report for the first item.
Maybe the second stuff is gone when settled the first.

Certainly it has a partly a matter of decision too - i.e. what behavior
we want to see.

Maybe some function needs an extension still, a kind of execute-with...

Andreas

[Andreas Roehler (a-roehler)]

Am 26.01.2012 09:48, schrieb yagnesh:
> Thats a table mess.,
>
> --------------------------------------------------------------
> shebang py-shell-name Callable winner
> --------------------------------------------------------------
> python python3 python shebang
> python3 python python3 shebang
> python ipython ipython py-shell-name
> python3 ipython ipython py-shell-name
> --------------------------------------------------------------
>
> another try.

if you call py-ececute-buffer whilst it has a shebang, shebang should win.

py-shell-name is only a default

Need show-cases which command fails in which circumstances

[Andreas Roehler (a-roehler)]

Am 26.01.2012 09:43, schrieb yagnesh:
> On 01/26/2012 04:45 PM, Andreas Roehler wrote:
>> Hi,
>>
>> as `py-shell-name' will exist with some probability, don't see
>> how `py-choose-shell-by-shebang' get's a chance that way.
>>
>> Do you experienced a related bug prior to this change?
>> Would prefer to proceed with some example.
>>
>> Thanks contributing,
>>
>> Andreas
>
>
> Hello Andreas,
>
> lets say I have this in my .emacs
>
> ---------------------------------------------------
> (require 'python-mode)
> (add-hook 'python-mode-hook
> (lambda ()
> (setq py-shell-name "ipython")))
> ---------------------------------------------------
>
> now I open a file with shebang containing
>
> #!/usr/bin/env python3
>
> and press C-c C-c it takes me to a python3 shell instead of ipython.

Hi all,

intend to change

py-execute-region

accepting an optional argument "execute-with-this-shell-instead-of-shebang"

would replace "&optional async", as "async" is broken for years and
no-one complained.

Agreed?

Cheers,

Andreas

[yagnesh (yagnesh)]

ohh
So you mean to say py-shell-name has no business in calling shell and its only for come as 'default' if all others fail?

So how do I choose the shell?
I see one possible way giving prefix argument. any other.?

I will create a bug report.

Thanks for clarifying

Yagnesh

[Andreas Roehler (a-roehler)]

Am 26.01.2012 10:47, schrieb yagnesh:
> ohh
:)
> So you mean to say py-shell-name has no business in calling shell and its only for come as 'default' if all others fail?

Basically.
Having multiple choices, you can't avoid some hierarchical order making
a selection.

if hard-coded `py-shell-name' always should prevail, you must it change
when executing code designed for a different Python by shebang.

The assumtion so far: shebang knows best which Python to use. Therefor
preceding by default.

Will consider possible change of py-execute-region permitting to enforce
a shell independent from shebang

>
> So how do I choose the shell?
> I see one possible way giving prefix argument. any other.?
>
>
> I will create a bug report.
>
> Thanks for clarifying
>
> Yagnesh

[Andreas Roehler (a-roehler)]

Hi Yagnesh,

think you pointed at a real issue. Hopefully the matter is solved by r804,
    Date: 2012-01-27 14:06:40
    Revision ID: <email address hidden>

https://launchpad.net/bugs/920079
user configured `py-shell-name' is not honored by `py-guess-default', lp:920079, fixed

new command `py-execute-region-default' forces the
systems default Python interpreter to execute, ignores
shebang

The patch provided with the branch reviewed however would introduce bugs IMO.
The title "py-choose-shell should respect user preference before going to guess" misinterpretes.
python-mode doesn't work that way.

Well, maybe I'm wrong instead :) We may always be wrong, that's inevitable.
You may request another one to review or/and also proceed with a fork that way.

Thanks contributing, let's go on in any case,

Andreas

review: Disapprove

Unmerged revisions

803. By yagnesh on 2012-01-25

* python-mode.el (py-choose-shell):

        py-choose-shell should respect user preference
        before going to guess.

=== modified file 'python-mode.el'
--- python-mode.el	2012-01-25 16:04:58 +0000
+++ python-mode.el	2012-01-25 20:44:26 +0000
@@ -7591,9 +7591,11 @@
   "Looks for an appropriate mode function.
 
 This does the following:
+ - with ARG prompts to user for choosing shell
+ - checks user has any preference for `py-shell-name'
  - look for an interpreter with `py-choose-shell-by-shebang'
  - examine imports using `py-choose-shell-by-import'
- - if not successful, return default value of `py-shell-name'
+ - if non successful, return default value of `py-shell-name'
 
 With \\[universal-argument]) user is prompted to specify a reachable Python version."
   (interactive "P")
@@ -7603,6 +7605,7 @@
                     (if (not (string= "" py-shell-local-path))
                         (expand-file-name py-shell-local-path)
                       (message "Abort: `py-use-local-default' is set to `t' but `py-shell-local-path' is empty. Maybe call `py-toggle-local-default-use'")))
+                   (py-shell-name py-shell-name)
                    ((py-choose-shell-by-shebang))
                    ((py-choose-shell-by-import))
                    (t (default-value 'py-shell-name)))))