-
-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Idle: Print filename when running a file from editor #65391
Comments
Upon restarting the user process, Idle prints a separator bar in the shell: |
Method: runcode Added To print file path that is executed |
I have not tried your patch yet, but the relevant code seems to be class ModifiedInterpreter(InteractiveInterpreter):
def runcode(self, code):
if code.co_filename[0] != '<': ## your patch
self.tkconsole.write('Executing ' + code.co_filename + '\n')
if self.tkconsole.executing:
self.interp.restart_subprocess()
def restart_subprocess(self, with_cwd=False):
if was_executing:
console.write('\n')
console.showprompt()
halfbar = ((int(console.width) - 16) // 2) * '='
console.write(halfbar + ' RESTART ' + halfbar) The 2 problems I see with the patch are that 1) it prints prematurely and 2) it does not replace RESTART. Both should be solved by passing the title string to restart_process as a new 'title' arg (title=' RESTART '). The halfbar expression should use len(title). I believe 16 is 9 (len(' RESTART ')) + 4 (len('>>> ')) + 3 (console.width (80) - 77). So
halfbar = ((int(console.width) - len(title) - 7) // 2) * '=' I think the bar would be better without a prompt before it. If console.showprompt() is deleted, 7 becomes 3. |
I tried to replace RESTART by doing these little changing # PyShell.Py
class ModifiedInterpreter(InteractiveInterpreter):
def restart_subprocess(self, with_cwd=False, with_msg=True):
...
if with_msg:
halfbar = ((int(console.width) - 16) // 2) * '='
console.write(halfbar + ' RESTART ' + halfbar)
def runcode(self, code):
with_msg = True
if code.co_filename[0] != '<':
self.tkconsole.write('Executing ' + code.co_filename + '\n')
with_msg = False
if self.tkconsole.executing:
self.interp.restart_subprocess(with_msg)
# ScriptBinding.Py
class ScriptBinding:
def _run_module_event(self, event):
filename = self.getfilename()
if not filename:
return 'break'
code = self.checksyntax(filename)
if not code:
return 'break'
if not self.tabnanny(filename):
return 'break'
interp = self.shell.interp
if PyShell.use_subprocess:
interp.restart_subprocess(with_cwd=False, with_msg=False) This works fine and replaces RESTART with Execute <filename> when file is executed in Python Shell. Also instead of this halfbar = ((int(console.width) - 16) // 2) * '='
console.write(halfbar + ' RESTART ' + halfbar) my recomemdation is: |
I took another look and tried the patch and discovered that my comments are partly wrong because restart_shell is called before runcode when runcode is use to run a file. (I am not sure how it is called otherwise with self.tkconsole.executing == True.) This happens in ScriptBinding.ScriptBinding._run_module_event and that is where filename could be added to the restart_shell call. The rest of my comment was about not printing fake prompts that the user can never respond to. In particular, Run F5 is like python -i, and for that the console interpreter does not print a prompt until it is done running the script and ready for user input. C:\Programs\Python34>python -i tem.py
|
New changeset 20a8e5dccf66 by Terry Jan Reedy in branch '2.7': New changeset 2ae12789dcb8 by Terry Jan Reedy in branch '3.4': |
This is how restarts look now. Thanks for the initial idea and patch.
xxxx
>>>
=============================== RESTART Shell =========================
|
New changeset edf9bfe36ad0 by Terry Jan Reedy in branch '2.7': |
I've found this to be a serious usability regression and think it should be reverted right-away. It makes IDLE unsuitable for evening showing turtle demos to kids. For adults in my classes, it was also confusing because unlike the old restart-bar it fails to make a clean distinction between sessions and making it clear that old variable definitions have be forgotten. The issue is even more acute because cmd-p can cycle through statements in previous sessions. |
How about 'RESTART: Shell' and 'RESTART: <name of file>' to make it clear that old definitions are forgotten in both cases while new definitions are added in the second case. I do not understand the comment about turtledemo as it runs separately from Idle. |
Now that Larry Hastings has posted directions for handling changes pulled into 3.5.0, I will soon commit the attached, or something close, and add a pull request. |
New changeset 69ea73015132 by Terry Jan Reedy in branch '2.7': New changeset 130a3edcac1d by Terry Jan Reedy in branch '3.4': |
Larry, please pull @restart.diff into 3.5.0. I have already applied it to 2.7, 3.4, 3.5.1, and 3.6, so I will null merge from 3.5.0 into 3.5.1 and 3.6. Reason. Aside from Raymond's request above, I was reminded about a week ago that a) when Idle runs in one process, running a file from the editor does *not* cause a restart, and b) we might want to add the option to not restart even when running with two processes. So I agree with Raymond that Idle should say RESTART (or the equivalent) when restarting the execution process and not say it when not, and that 3.5.0 should be consistent with other releases, and not make a change that is reverted in 3.5.1. |
Terry, if you want this pulled in to Python 3.5.0, you'll need to create a pull request on Bitbucket. Instructions are here: https://mail.python.org/pipermail/python-dev/2015-August/141167.html and here: https://mail.python.org/pipermail/python-dev/2015-August/141365.html |
On the other hand, I do not hold with marking a minor cosmetic change like this as "release blocker". I'm willing to accept the change, given PEP-434, but I'm not going to delay any releases for it. |
Sorry, my memory was that I was supposed to that as a signal, but maybe that was with a different RM. |
On 9/3/2015 4:31 AM, Larry Hastings wrote:
I don't remember seeing this, but done now.
I did get this. This step 4: Pull from the 3.5.0 repo into your "cpython351-merge" directory. gives me a Putty Fatal Error: Disconnected: No supported authentication methods I presume it would require a key on deposit, as with python.org. It There is something odd about the size of your clone. My cpython clone |
Pull request accepted, please forward-merge. Thanks!
I have no idea why. All I can tell you is, I made my cpython350 directory by forking the official Bitbucket mirror of the cpython tree ( https://bitbucket.org/mirror/cpython ) and updating it. If you're on a UNIX-y filesystem (something with hardlinks), the "relink" extension for Mercurial can help cut down on the disk space consumed. |
The pull requests are numbered by creation order, not by merge order. Normally I'd expect that yes, Serhiy's merge would include yours. But it didn't work out that way. If you look at the changeset graph: https://bitbucket.org/larry/cpython350/commits/all Serhiy merged his revision (2b6ce7e), not my merge commit afterwards (07e04c3) which includes your change. So your change still needs to be forward-merged. Please pull and merge the current head (07e04c3), thanks! |
Since your merge contains the changes that Serhiy already merged, I expect that there will be some conflicts. If so, the only thing I would know to do is revert and make mine and his null merges. If that is correct, will do. Making merge clone now. |
New changeset 09cd7d57b080 by Terry Jan Reedy in branch '3.5': |
There won't be conflicts with Serhiy's merge--just the opposite. His pull request was merged first, so it's perfect that he did his forward merge first. He's already resolved any conflicts with his merge, and so when you merge you'll only have to worry about your own changes. Just merge the current head (07e04c3), it'll be fine, honest! |
I was wrong, no conflicts. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: