addressing issue #578 - Githubissues

jsvine / pdfplumber

Plumb a PDF for detailed information about each char, rectangle, line, et cetera — and easily extract text and tables.

MIT License

6.57k stars 659 forks source link

addressing issue #578 #581

Closed augeos-grosso closed 2 years ago

augeos-grosso commented 2 years ago

This PR fixes #578 . Indeed the line causing the missed close() call on Windows (and only on Windows it seems) was the one @samkit-jain pointed out.

The original exception was pdfminer.pdfparser.PDFSyntaxError, but i figured it would be best using pdfminer.psparser.PSException, since it is the most general exception i could find that made sense using.

jsvine commented 2 years ago

Thanks for filing this @augeos-grosso! Two quick things:

Could you push an empty/minor commit? It looks like the PR-checking workflow got tripped up on a network issue. We can squash the commit when we merge.
Thanks for adding yourself to the contributor's list — well deserved! But could you tweak it (i.e., choose just one link/name/handle) for the purposes of consistency with the other contributors in the list? Doing that would actually take care of the needs-another-commit-to-rerun-workflow situation above.

jsvine commented 2 years ago

Thanks! And, hmmm, workflow still seems gummed up. I'll take a closer look later today; I could just run the tests locally if it comes to that.

augeos-grosso commented 2 years ago

Of course! I'm here to help if you need me

jsvine commented 2 years ago

Thanks again @augeos-grosso! Ran the tests locally — they passed, but coverage decreased since there were no tests added for this change. That's fine. They're simple tests. I've now written them and will push them to develop.