python / cpython

The Python programming language
https://www.python.org/
Other
59.97k stars 29.02k forks source link

Management of KeyboardInterrupt in cmd.py #45635

Closed 55a8d35c-d1cd-4aaf-bd18-313ac62e4f32 closed 14 years ago

55a8d35c-d1cd-4aaf-bd18-313ac62e4f32 commented 16 years ago
BPO 1294
Nosy @gvanrossum, @avassalotti, @ngie-eign
Files
  • cmd.py
  • cmd.py.keyboardinterrupt.patch
  • cmd.diff
  • keyboardcmd.py
  • 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: ```python assignee = None closed_at = created_at = labels = ['type-feature', 'library'] title = 'Management of KeyboardInterrupt in cmd.py' updated_at = user = 'https://bugs.python.org/stephbul' ``` bugs.python.org fields: ```python activity = actor = 'Philip.Zerull' assignee = 'none' closed = True closed_date = closer = 'gvanrossum' components = ['Library (Lib)'] creation = creator = 'stephbul' dependencies = [] files = ['8562', '8566', '8568', '27752'] hgrepos = [] issue_num = 1294 keywords = ['patch'] message_count = 23.0 messages = ['56524', '56529', '56551', '56557', '56561', '56606', '58168', '58173', '58174', '58175', '58498', '59904', '59906', '59970', '59976', '59980', '95051', '95146', '95174', '95177', '95179', '145854', '173968'] nosy_count = 7.0 nosy_names = ['gvanrossum', 'isandler', 'draghuram', 'alexandre.vassalotti', 'stephbul', 'ngie', 'Philip.Zerull'] pr_nums = [] priority = 'low' resolution = 'rejected' stage = 'test needed' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue1294' versions = ['Python 2.7', 'Python 3.2'] ```

    55a8d35c-d1cd-4aaf-bd18-313ac62e4f32 commented 16 years ago

    According to me, the Ctrl-C is not managed correctly in cmd.py. Ctrl-C generates a a KeyboardInterrupt exceptions, and only EOFError is managed. I propose to manage KeyboardInterrupt on line 130: print 'are you sure you want to exit? y/n' answer ='' while (answer != 'y') & (answer != 'n'): answer = raw_input() if answer == 'y': exit(0)

    Here is attached my new cmd.py Hope ti will help.

    gvanrossum commented 16 years ago

    Would you mind submitting a patch instead of a whole new file?

    55a8d35c-d1cd-4aaf-bd18-313ac62e4f32 commented 16 years ago

    Hello, Here is my patch for cmd.py Regards stephbul

    gvanrossum commented 16 years ago

    Hmm... I don't think this is the right thing to do. The code is broken in several ways. I recommend you find someone to help you come up with a better patch in comp.lang.python.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    I tested cmd.py on Linux and two things (including the one reported by OP) looked odd to me.

    1) CTRL-D produces a message "*** Unknown syntax: EOF". 2) CTRL-C produces a KeyboardInterrupt exception and the session terminates.

    I am attaching a patch that changes the behaviour to a more typical one (at least, in IMHO). It terminates the session on CTRL-D and it just ignores CTRL-C. Both of these can be overridden, if required. If the patch is accepted, a doc change would be required in addition to the change in doc string. I tested the patch on Linux and Windows.

    55a8d35c-d1cd-4aaf-bd18-313ac62e4f32 commented 16 years ago

    Well, I made it with a diff -ruN, it works fine on my ubuntu. It is only a ctrl-C management only, not a ctrl-D. What do you mean by broken? Regards. Stephbul

    2007/10/19, Guido van Rossum \report@bugs.python.org\:

    Guido van Rossum added the comment:

    Hmm... I don't think this is the right thing to do. The code is broken in several ways. I recommend you find someone to help you come up with a better patch in comp.lang.python.

    ---------- keywords: +patch


    Tracker \report@bugs.python.org\ \http://bugs.python.org/issue1294\


    avassalotti commented 16 years ago

    First, I would like to say thank you both for spending your time trying to do a contribution to Python.

    However, I believe the current behavior of cmd.py is correct. The module documentation states clearly that "End of file on input is processed as the command 'EOF'." For the KeyboardInterrupt issue, it thinks the exception should be handled by the application with a try-statement. For example,

       >>> import sys, cmd
       >>> c = cmd.Cmd()
       >>> try:
       ...     c.cmdloop()
       ... except KeyboardInterrupt:
       ...     print "\nGot keyboard interrupt. Exiting..."
       ...     sys.exit(0)
       ...
       (Cmd) ^C
       Got keyboard interrupt. Exiting...

    I am closing and marking this bug as rejected. If you feel this is inappropriate, please request with an appropriate explanation to reopen it.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    My patch adds very sensible default behaviour when Ctrl-C or Ctrl-D are entered. It follows the tradition of many unix programs such as bash and bc which exit on Ctrl-D and ignore Ctrl-c in one way or another. Most importantly, a user can always override the behaviour but I expect the added functionality would suffice in most if not all cases. Do you mind opening the issue in the hope that we can have more comments?

    gvanrossum commented 16 years ago

    I will look into this for 2.6. No promises. Somebody ought to check whether this does not cause problems for pdb.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    I will look into this for 2.6. No promises. Somebody ought to check whether this does not cause problems for pdb.

    Thanks. I will check about pdb tomorrow.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    "Tomorrow" took a while to come by :-)

    With out the patch, pdb prints this on Ctrl-C:

    "KeyboardInterrupt Uncaught exception. Entering post mortem debugging Running 'cont' or 'step' will restart the program"

    With the patch, pdb prompt appears again. Ctrl-D change doesn't effect pdb because do_EOF() is already implemented. My test was on SuSE 10 Linux.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    bugday task?

    gvanrossum commented 16 years ago

    To mark things for the bugday, set the 'easy' keyword.

    However, this particular one is IMO too subtle for a bugday, witness the discussion here. Perhaps a bugday could come up with an ultimate patch, but I'd be hesitant to submit it without having reviewed it personally.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    Ok. BTW, can I get tracker permissions? I will try to check old bugs to update their information and if required, close them.

    gvanrossum commented 16 years ago

    I've added developer status to your username. Let me know if it doesn't work.

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 16 years ago

    I've added developer status to your username. Let me know if it doesn't work.

    It does. Thanks.

    fd930960-0aae-426c-bbb8-8549e6684f33 commented 14 years ago

    Is not this patch backward incompatible?

    E.g any cmd-based application which expects Ctrl-C to propagate to the top level will be broken by this patch.

    As for pdb, I don't think pdb will benefit from this patch: as I believe that pdb needs something along the lines of patch bpo-7245 for Ctrl-C (temporary interrupt of execution with ability to resume similar to what gdb does)

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 14 years ago

    On Sun, Nov 8, 2009 at 8:22 PM, Ilya Sandler \report@bugs.python.org\ wrote:

    Is not this patch backward incompatible?

    E.g any cmd-based application which expects Ctrl-C to propagate to the top level will be broken by this patch.

    But currently, CTRL-C terminates the session instead of propagating upstream. The only way it would do so is if do_KeyboardInterrupt() is implemented in which case, the behaviour would remain same even with the patch.

    fd930960-0aae-426c-bbb8-8549e6684f33 commented 14 years ago

    But currently, CTRL-C terminates the session instead of propagating upstream

    I am not sure I understand: currently Ctrl-C generates a KeyboardInterrupt, which can be caught by the application which can then decide how to proceed (in particular it can start another command loop or exit with a meaningful message or anything else).

    This patch would suppress KeyboardInterrupt and thus interfere with such applications. Or am I missing something?

    823ad82f-2e9f-4388-9c64-a3126f202a71 commented 14 years ago

    I am not sure I understand: currently Ctrl-C generates a KeyboardInterrupt, which can be caught by the application which can then decide how to proceed (in particular it can start another command loop or exit with a meaningful message or anything else).

    This patch would suppress KeyboardInterrupt and thus interfere with such applications. Or am I missing something?

    I checked the patch and tested with python from trunk. You are right that the patch catches KeyboardInterrupt thus interfering with any applications that expect it to be propagated upstream. Perhaps, this can be made conditional so that we can keep both behaviors.

    But CTRL-D processing doesn't suffer from any backwards compatible issues and that part of the patch should be able to be applied safely.

    gvanrossum commented 14 years ago

    I don't think this is a good idea.

    cd099432-c439-4196-9816-d22314ec360c commented 12 years ago

    I realize that this bug is closed, but I just had a comment to make.

    Handling EOF is simple:

    def do_EOF(self, arg):
        pass

    For my purposes I want to raise an EOFError so I can trickle up the chain to the appropriate caller because I'm coding a CLI where I have a nested set of commands, e.g.

    command subcommand_0 command subcommand_1

    I'd like the behavior to match what's done in Cisco IOS or IronPort's CLI (to some degree).

    The only part that's annoying is that I have to hide do_EOF in the help and completion output, otherwise the user will see the handler when completing or running help, but I'll bring that up in another issue.

    6f25470b-fa4a-4560-8a65-4e80bf2600af commented 11 years ago

    Hello,

    Being of a similar mindset to draghuram on the do_KeyboardInterrupt idea and thought I'd implement it as a subclass. While this probably wasn't fully implemented correctly, I think it provides an interesting solution to stephbul's frustrations and won't break anything.

    I'm not suggesting that this be included in the standard library but just thought you all might find it interesting.