Code review comment for lp:~yagnesh/python-mode/py-shell

Revision history for this message
Andreas Roehler (a-roehler) wrote :

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

« Back to merge proposal