Patches item #1232343, was opened at 2005-07-04 20:03
Message generated for change (Comment added) made by greglielens
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1232343&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Lisandro Dalcin (dalcinl)
Assigned to: Nobody/Anonymous (nobody)
Summary: PyOS_Readline
Initial Comment:
Greg Lielens submitted some time ago a patch [id 955928] about
'PyOS_Readline()' behavior with non-interactive tty's. Basically,
there is no way to properly override 'PyOS_ReadlineFunctionPointer'
as
'PyOS_Readline()' calls 'PyOS_StdioReadline()' when 'stdin' or
'stdout' are not tty's. A snippet of "Parser/myreadline.c":
...
if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout)))
rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt);
else
rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
prompt);
...
Greg Lielens is completely right about the problem, but his patch is
perhaps a bit crude, it also modifies "Python/bltinmodule.c" to solve
the same issue with 'raw_input'.
I think I have found a not so crude solution, and completely
backward
compatible. Basically, I moved 'isatty()' test from 'PyOS_Readline()'
in file "Parser/myreadline.c" to 'call_readline()' in file
"Modules/readline.c". In order to do that, I believe
'PyOS_StdioReadline' have to be added to file "Include/pythonrun.h".
All those changes will not affect in any way the behavior in
interactive sessions. Now 'PyOS_ReadlineFunctionPointer' can be
properly overrode and users of 'readline' module will not see any
change: in non-interactive tty's 'PyOS_StdioReadline()' will be called
anyway. The problem in 'input' and 'raw_input' builtins remains, but
its solution is not so clear to me, I think this part should not be
touched right now.
This patch passes 'Lib/test/regrtest.py' (of course, all those tests
are non-interactive!). I use Python interpreter every day in the
standard way and also in MPI-parallelized interactive sessions with
this patch applied, and I never have any problem.
I send my patch obtained with 'cvs diff -c' from current sources.
----------------------------------------------------------------------
Comment By: Gregory Lielens (greglielens)
Date: 2005-08-06 21:14
Message:
Logged In: YES
user_id=1044428
Hi Lisandro, sorry that I did not follow this matter, we had
other short term focus in our company and I had trouble
finding time for it. Good that you subject this though, I
have checked it fast and from what I understand it would
have the same effect as my older patch...
However, I used some features of my patch that are not
present in yours:
in Parser/myreadline.c, I removed the test about
PyOS_ReadlineFunctionPointer being null to instead
initialising it at creation to the correct value depending
to system: This was important because it allows me in my
specific MPI Readline function to use this defautl functio
for the process 0...In general, it allows to re-use the
default in the new Readline function, incrementing
functionalities instead of re-implementing full
functionality. You will have the same mechanism with the
inclusion of PyOS_StdioReadline in the pythonrun API, but I
personally find this solution less appealing: it increase
the size of the API, and you do not have the correct
readline on VMS...That's why I have done it like this, even
if it may seems hackish ;) Of sourse manual has to be
updated to say that PyOS_ReadlineFunctionPointer has a
meaningfull defautl value...but it would also have to be
updated in your approach, to tell about PyOS_StdioReadline...
Regarding your modif of the Modules/readline.c, I have the
feeling that your approach is cleaner...because It seems
nicer to check for tty inside the new readline function,
instead of installing the new readline function only if
stdin/stdout are tty....You have to use PyOS_StdioReadline
though, that would not be accessible if one does not add it
to the pythonrun.h API, and that is not correct on vms
system anyway (at least that's how I understand it...but I
have no vms system to check ;) ). Maybe a better approach
would be to cache the original PyOS_ReadlineFunctionPointer
in the readline module, and re-use it if stdin/stdout are
non-tty? This would be close to the approach I used for
implementing my MPI readline: cache
PyOS_ReadlineFunctionPointer and re-use it for modif...
If this is considerd unclean, the approach putting
PyOS_StdioReadline in the API can be kept, but It would have
to be renamed something like PyOS_ReadlineFunction and be
defined as PyOS_StdioReadline/vms__StdioReadline depending
on the environment...
Now for my patch on "Python/bltinmodule.c" to solve
the same issue with 'raw_input', I think it was necessary to
be able to use ipython in an interractive MPI session...Our
embedded python interpreter had the possibility to use this
(it gives a far nicier shell than the standard one), but I
think It uses raw_input as entry mechanism...Could you test
your patch to see if it work with this?
The only drawback I see with this is that it could slow down
a bit the parsing when using this function on a
non-interractive file...but I doubt this is significative
and could be optimized by keeping the call to PyOS_Readline
but stripping out the prompt treatment in this case...
We now are moving from an embbedded python interpreter to
using a standard python...which make the acceptance of the
patch more interresting: it would allows us to uses stock
python interpreters > 2.x, no need to distribute our own
interpretter anymore!!! :)
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1232343&group_id=5470
_______________________________________________
Patches mailing list
[email protected]
http://mail.python.org/mailman/listinfo/patches
