Skip to content
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

Closed
terryjreedy opened this issue Apr 9, 2014 · 25 comments
Closed

Idle: Print filename when running a file from editor #65391

terryjreedy opened this issue Apr 9, 2014 · 25 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 21192
Nosy @rhettinger, @terryjreedy, @larryhastings, @benjaminp
Files
  • PyShell.py
  • @restart.diff: one line change
  • 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:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2015-09-04.08:22:53.678>
    created_at = <Date 2014-04-09.21:24:46.177>
    labels = ['expert-IDLE', 'type-feature']
    title = 'Idle: Print filename when running a file from editor'
    updated_at = <Date 2015-09-04.08:22:53.677>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2015-09-04.08:22:53.677>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2015-09-04.08:22:53.678>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2014-04-09.21:24:46.177>
    creator = 'terry.reedy'
    dependencies = []
    files = ['34783', '40278']
    hgrepos = []
    issue_num = 21192
    keywords = ['patch']
    message_count = 25.0
    messages = ['215846', '215888', '216277', '216497', '216525', '247795', '247796', '247825', '248864', '248876', '249263', '249590', '249592', '249619', '249686', '249692', '249704', '249705', '249714', '249733', '249734', '249736', '249737', '249739', '249740']
    nosy_count = 6.0
    nosy_names = ['rhettinger', 'terry.reedy', 'larry', 'benjamin.peterson', 'python-dev', 'Adnan.Umer']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21192'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

    @terryjreedy
    Copy link
    Member Author

    Upon restarting the user process, Idle prints a separator bar in the shell:
    ====================================== RESTART=====================....
    When restart is due to running a file with F5, print "RUN filename" instead of "RESTART". Idea from Adnan Umer, pydev list. For Shell / Restart Shell Cntl+F6, maybe change to "RESTART Shell" (my additional idea).

    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Apr 9, 2014
    @AdnanUmer AdnanUmer mannequin added the topic-IDLE label Apr 10, 2014
    @AdnanUmer
    Copy link
    Mannequin

    AdnanUmer mannequin commented Apr 10, 2014

    Method: runcode
    Class: ModifiedInterpreter
    Line: 752

    Added
    if code.co_filename[0] != '<':
    self.tkconsole.write('Executing ' + code.co_filename + '\n')

    To print file path that is executed

    @terryjreedy
    Copy link
    Member Author

    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 will code this later.

    @terryjreedy terryjreedy self-assigned this Apr 15, 2014
    @AdnanUmer
    Copy link
    Mannequin

    AdnanUmer mannequin commented Apr 16, 2014

    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:
    console.write('[SHELL RESTART]')

    @terryjreedy
    Copy link
    Member Author

    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
    start
    stop

    >>

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 1, 2015

    New changeset 20a8e5dccf66 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21192: Idle Editor. When a file is run, put its name in the restart bar.
    https://hg.python.org/cpython/rev/20a8e5dccf66

    New changeset 2ae12789dcb8 by Terry Jan Reedy in branch '3.4':
    Issue bpo-21192: Idle Editor. When a file is run, put its name in the restart bar.
    https://hg.python.org/cpython/rev/2ae12789dcb8

    @terryjreedy
    Copy link
    Member Author

    This is how restarts look now. Thanks for the initial idea and patch.

    >>
    ====================== RUN C:\Programs\Python34\tem.py =================

    xxxx
    >>> 
    =============================== RESTART Shell 

    =========================

    >>

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 1, 2015

    New changeset edf9bfe36ad0 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21192: acks for 2.7
    https://hg.python.org/cpython/rev/edf9bfe36ad0

    @rhettinger
    Copy link
    Contributor

    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.

    @rhettinger rhettinger reopened this Aug 19, 2015
    @terryjreedy
    Copy link
    Member Author

    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.

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 3, 2015

    New changeset 69ea73015132 by Terry Jan Reedy in branch '2.7':
    Issue bpo-21192: Change 'RUN' back to 'RESTART' when running editor file.
    https://hg.python.org/cpython/rev/69ea73015132

    New changeset 130a3edcac1d by Terry Jan Reedy in branch '3.4':
    Issue bpo-21192: Change 'RUN' back to 'RESTART' when running editor file.
    https://hg.python.org/cpython/rev/130a3edcac1d

    @terryjreedy
    Copy link
    Member Author

    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.

    @larryhastings
    Copy link
    Contributor

    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

    @larryhastings
    Copy link
    Contributor

    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.

    @terryjreedy
    Copy link
    Member Author

    Sorry, my memory was that I was supposed to that as a signal, but maybe that was with a different RM.

    @terryjreedy
    Copy link
    Member Author

    @terryjreedy
    Copy link
    Member Author

    On 9/3/2015 4:31 AM, Larry Hastings wrote:

    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

    I don't remember seeing this, but done now.

    and here:

    https://mail.python.org/pipermail/python-dev/2015-August/141365.html

    I did get this. This step

    4: Pull from the 3.5.0 repo into your "cpython351-merge" directory.
    % hg pull ssh://hg@bitbucket.org/larry/cpython350

    gives me a

    Putty Fatal Error: Disconnected: No supported authentication methods
    available (server sent publickey).

    I presume it would require a key on deposit, as with python.org. It
    seems however that
    hg pull https://terryjreedy@bitbucket.org/larry/cpython350
    might work in that it searched for changes and found none (as expected).

    There is something odd about the size of your clone. My cpython clone
    is 928 MB on disk with 30300 files, while the clone of my fork of your
    repository is 1.59 GB for 14500 files.

    @larryhastings
    Copy link
    Contributor

    Pull request accepted, please forward-merge. Thanks!

    There is something odd about the size of your clone. My cpython
    clone is 928 MB on disk with 30300 files, while the clone of my fork
    of your repository is 1.59 GB for 14500 files.

    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.

    @terryjreedy
    Copy link
    Member Author

    This was pull #12. Am I correct in thinking that all the merges Serhiy did after for pull #13 included the null merge for this?

    @larryhastings
    Copy link
    Contributor

    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!

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 4, 2015

    New changeset 09cd7d57b080 by Terry Jan Reedy in branch '3.5':
    Issue bpo-21192: Change 'RUN' back to 'RESTART' when running editor file.
    https://hg.python.org/cpython/rev/09cd7d57b080

    @larryhastings
    Copy link
    Contributor

    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!

    @terryjreedy
    Copy link
    Member Author

    I was wrong, no conflicts.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants