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

some quick fixes #12

Closed
wants to merge 1 commit into from
Closed

some quick fixes #12

wants to merge 1 commit into from

Conversation

shreyas
Copy link

@shreyas shreyas commented Dec 31, 2014

pip install fails if PyPDF2 is not already installed, since setup.py
imports pdfmerge for doc and author, but pdfmerge in turn imports
PyPDF2. the import now happens if the module runs as main

also, the command fails if a pdf file is reported as encrypted, even
though no password is set. using default password, which is blank,
fixes these situations.

pip install fails if PyPDF2 is not already installed, since setup.py
imports pdfmerge for doc and __author__, but pdfmerge in turn imports
PyPDF2. the import now happens if the module works as __main__

also, the command fails if a pdf file is reported as encrypted, even
though no password is set. using default password, which is blank,
fixes these situations.
@metaist
Copy link
Owner

metaist commented Dec 31, 2014

Thank you for these fixes. I'll definitely pull the empty password encrypted file fix. Would it be valuable to let users provide a password on the command line if all the encrypted files had the same password?

The problem with the imports is that you can't use pdfmerge as a module if they are only pulled in during main. I think I'll use the conditional import (try: import ... except ImportError: ...) so that the tests don't fail.

@shreyas
Copy link
Author

shreyas commented Jan 1, 2015

Yes, I had thought about adding a -p for password, but then thought there would be two problems:

  1. chances of having all pdf files having the same password as pretty rare.
  2. That will require entering password on command line, which can be a strict no-no for many, including me.

So avoided adding it altogether.

As for importing pdfmerge as module, wonder how I missed that possibility. Try-except sounds better.

On Dec 31, 2014, at 8:55 PM, metaist notifications@github.com wrote:

Thank you for these fixes. I'll definitely pull the empty password encrypted file fix. Would it be valuable to let users provide a password on the command line if all the encrypted files had the same password?

The problem with the imports is that you can't use pdfmerge as a module if they are only pulled in during main. I think I'll use the conditional import (try: import ... except ImportError: ...) so that the tests don't fail.


Reply to this email directly or view it on GitHub.

@metaist
Copy link
Owner

metaist commented Jan 1, 2015

Agree with those points. I probably should first try to use the default password (empty string) and then, if the file is still encrypted, use getpass to ask for the password.

I've created a new issue to discuss this if you think there's a better approach.

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.

None yet

2 participants