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

Catch exceptions in _check_module #269

Merged
merged 2 commits into from
Dec 21, 2019
Merged

Catch exceptions in _check_module #269

merged 2 commits into from
Dec 21, 2019

Conversation

vstinner
Copy link
Contributor

@vstinner vstinner commented Oct 2, 2019

  • _check_module.main() now catchs IndexError and FileNotFoundError
    errors: raise ArgcompleteMarkerNotFound exception instead.
  • Replace open() with tokenize.open() to handle "# coding: xxx"
    encoding cookie: support source code using an encoding different
    than UTF-8 on Python 3.

@vstinner
Copy link
Contributor Author

vstinner commented Oct 2, 2019

On Fedora, a default exception hook is installed on the whole system. Uncatched exceptions are reported to Fedora using ABRT. Type "python3 -m http.ser[TAB]" on Fedora detects a bug and invites the user to report the bug to Fedora. Example of such report:
https://bugzilla.redhat.com/show_bug.cgi?id=1737212

Bonus: this change also adds support for encoding different than UTF-8 on Python 3. Tell me if you prefer a separated PR for that.

Note: I tested my change on Python 3.7 using my other PR #268.

* _check_module.main() now catchs IndexError and FileNotFoundError
  errors: raise ArgcompleteMarkerNotFound exception instead.
* Replace open() with tokenize.open() to handle "# coding: xxx"
  encoding cookie: support source code using an encoding different
  than UTF-8 on Python 3.
@codecov-io
Copy link

codecov-io commented Oct 2, 2019

Codecov Report

Merging #269 into master will decrease coverage by 1.37%.
The diff coverage is 6.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   83.85%   82.47%   -1.38%     
==========================================
  Files           7        7              
  Lines         706      719      +13     
==========================================
+ Hits          592      593       +1     
- Misses        114      126      +12
Impacted Files Coverage Δ
argcomplete/_check_module.py 58.73% <6.66%> (-13.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31972ee...fa727ef. Read the comment docs.

@vstinner
Copy link
Contributor Author

vstinner commented Oct 2, 2019

I rebased this PR on top on my tox change.

Oops, argcomplete supports Python 2: I didn't notce that. I adapted my PR for Python 2.

@evanunderscore
Copy link
Collaborator

Thanks for the PR @vstinner! I'm happy to merge this for you, however I won't be able to push a release.

@vstinner
Copy link
Contributor Author

I forgot this PR that I wrote 2 months ago. Feel free to modify it if you want :-)

@kislyuk
Copy link
Owner

kislyuk commented Dec 20, 2019

@evanunderscore your discretion with this PR, please merge as you see fit and I will cut a release when ready.

@evanunderscore evanunderscore merged commit c7e5755 into kislyuk:master Dec 21, 2019
@evanunderscore
Copy link
Collaborator

Thanks for your work @vstinner!

@vstinner vstinner deleted the catch_errors branch December 21, 2019 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants