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

Exit peacefully on KeyboardInterrupt #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jackthepirate6
Copy link
Member

Captured keyboardinterrupt without traceback.

Closes issue #179

@@ -57,13 +58,18 @@ def _get_arg_parser():
return arg_parser


def signal_handler(signal, frame):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the need of passing arguments to this function if you are not using them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the following error if i don't pass the arguments,
TypeError: signal_handler() takes 0 positional arguments but 2 were given

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jackthepirate6 thats probably because you are forgetting self or cls. You need to remove those unnecessary arguments.

@khanchi97
Copy link
Member

And in commit description:
Use Closes link_to_the_issue instead of using Closes issue link_to_the_issue.

@@ -2,6 +2,7 @@
import logging
import os
import sys
import signal
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this import will be above import sys following alphabetical order ;)

def signal_handler(self, cls):
sys.exit(0)


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need of two blank lines.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to E302 of pep8, there should be two blank lines instead of one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the newlines are fine

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although I didn't mean for you to add both self and cls

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both are actually the same, with different names

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is difficult to know when a person will system exit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's kind of better to initialise exception handler first

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm... frankly speaking I don't understand what you are trying to do over here 🤪 but adding those two parameters without knowing why its working that way is definitely wrong

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am trying to capture the exception globally.

Copy link
Member Author

@jackthepirate6 jackthepirate6 Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will remove the unnecessary parameters.

Captured keyboardinterrupt without traceback.

Closes coala#179
Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review your commit message to follow http://coala.io/commit , and capitalise exception names correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants