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

add shebang line to script in tools #408

Merged
merged 4 commits into from
Apr 28, 2020
Merged

Conversation

fabbox
Copy link
Contributor

@fabbox fabbox commented Mar 27, 2020

This update fix #405.

It enables to run the scripts in tools (especially pdf2txt.py and dumppdf.py) directly from terminal.

@loqs
Copy link

loqs commented Mar 27, 2020

Should it not be python3 as the python 2 no longer supported in line with https://www.python.org/dev/peps/pep-0394/ ?

@fabbox
Copy link
Contributor Author

fabbox commented Mar 28, 2020

As python2 is now "deprecated" since January 2020, I didn't think about it. I must admit that according to https://www.python.org/dev/peps/pep-0394/#future-changes-to-this-recommendation, authors says the PEP394 should be update if the support for python2 is completly drop. On the other hand, most of scripts already containing the shebang line used the "python" and not "python3", so I really don't know.

tools/dumppdf.py Outdated Show resolved Hide resolved
@eli-schwartz
Copy link

eli-schwartz commented Apr 19, 2020

@fabbox the Python PEP requirement is that for programs, such as this, which are incompatible with python2 and only work on python3, the shebang should specifically reference python3.

It is wrong to use #!/usr/bin/env python anywhere in this project, ever since support for python2 was dropped in 3502dc9. The current PR as-is will break Debian, since python2 is the preferred unversioned python there and that is what will end up being used.

@fabbox fabbox closed this Apr 27, 2020
@eli-schwartz
Copy link

eli-schwartz commented Apr 27, 2020

Why did you close this PR, abandon the review, and open a new PR #421 with the exact same content?

@fabbox
Copy link
Contributor Author

fabbox commented Apr 27, 2020

I close this pull request and creat a new one with the proposition (with python3).
Sorry for the long delay between the two propositions.. I though it was the way to do. Should I be able to edit this PR ?

@eli-schwartz
Copy link

eli-schwartz commented Apr 27, 2020

Sure, this PR even updated when you pushed additional commits, then a minute later you closed it.

Github doesn't require you to close a PR and open a new one, ever.

@fabbox
Copy link
Contributor Author

fabbox commented Apr 27, 2020

OK, I think I miss something in the PR workflow. I didn't find how to edit the PR. I will close the new one and try to update this one.

Sorry to all, my bad here, I was thinking it do not accept my PR but it was updating it...
I completly agreed with the proposed change so I updated it with python3 everywhere the shebang line appeared.

@fabbox fabbox reopened this Apr 27, 2020
@pietermarsman pietermarsman merged commit 7eff108 into pdfminer:develop Apr 28, 2020
@pietermarsman
Copy link
Member

@fabbox Thanks!

@estshorter
Copy link
Contributor

estshorter commented May 17, 2020

@pietermarsman
I hope this fix will be released soon.

I'm a maintainer of a pdfminer.six-feedstock repositry which enables installation of pdfminer.six via conda.
The latest release (20200402) breaks tests because of no shebang line, and I cannot upgrade version on conda (sticking to 20191020)...

@pietermarsman
Copy link
Member

@estshorter done!

Let me know if it works.

@estshorter
Copy link
Contributor

Thanks @pietermarsman !
Now we can get the latest pdfminer.six via conda.

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.

pdf2txt.py and pdfdump.py should have a shebang
5 participants