Add --print-path option and README updates #8
Conversation
There was a problem hiding this comment.
Glad that you like the tool.
I think this is an improvement, but I think we should improve it some more before merging. You're very welcome to address these comments, but if you don't want to, I'll eventually get around to doing it myself. In the meantime, thank you!
|
|
||
| ```sh | ||
| apt-get update | ||
| apt-get install -y coreutils git python3 python3-tk ghostscript pdftk poppler-utils | ||
| git clone https://github.com/svenssonaxel/pdf-sign.git | ||
| cd pdf-sign | ||
| cp pdf-sign pdf-create-empty /usr/local/bin/ | ||
| # Get signatures folder | ||
| ./pdf-sign "" --print-path |
There was a problem hiding this comment.
I wish the empty "" wasn't needed.
There was a problem hiding this comment.
I wish too, it's quite ugly ! I'm not an argparse expert but I havn't succeed achieving it using argparse.
Anyway, after reconsideration I'd remove this option, as there is an error message that prints the signatures dir if it doesn't exists. In the README we could write something like:
To know the signature directory to create, run
./pdf-sign my_pdf.pdfand see the error message
What would you think about this ?
| ``` | ||
|
|
||
| Now you'll have to put your signature in a PDF file in this folder, you can use `empty-3inx2in.pdf` empty PDF template for example. |
There was a problem hiding this comment.
Good, but the instruction under ## How also needs correcting.
There was a problem hiding this comment.
Oh yes indeed. I'll merge both sections if you are okay
| ``` | ||
|
|
||
| Now you'll have to put your signature in a PDF file in this folder, you can use `empty-3inx2in.pdf` empty PDF template for example. | ||
|
|
There was a problem hiding this comment.
For clarity, perhaps add a line such as Thereafter, you can begin using pdf-sign. For example:
Otherwise the below code block could appear like it was about creating signatures.
| @@ -6,6 +6,9 @@ import argparse, os, queue, re, subprocess, sys, tempfile, time | |||
|
|
|||
| # Inspired by https://unix.stackexchange.com/a/141496 | |||
| def main(args): | |||
| if args.print_path: | |||
| print(getSignatureDir()) | |||
There was a problem hiding this comment.
This will print the correct path in almost all cases. The exception is when
- The user prefers to put the signatures under
~/.configin order not to litter the home directory. ~/.config/pdf_signaturesdoes not exist (yet)$PDF_SIGNATURE_DIRandXDG_CONFIG_HOMEare both unset.
In this situation, pdf-sign "" --print-path will print /home/user/.pdf_signatures even though it should have printed /home/user/.config/pdf_signatures.
I'm not exactly sure how to fix this, but I think it's a shame to make this improvement without going all the way.
There was a problem hiding this comment.
Indeed.
But this is out of the scope of this PR, as this is not a problem introduced by this PR, it was already the case before
Anyway, I'll finally suggest to turn back and forget adding this option.
| @@ -402,6 +408,7 @@ parser = argparse.ArgumentParser(description='Sign a PDF file.') | |||
| parser.add_argument('input', metavar='input.pdf', type=str, help='Input PDF file.') | |||
| parser.add_argument('-p', '--page', type=int, default=-1, help='The page to sign, negative for counting from the end. (default: -1)') | |||
| parser.add_argument('-s', '--signature', type=str, help='Path to file used as signature. Required in batch mode. In GUI mode, the user can choose among files in $PDF_SIGNATURE_DIR, $XDG_CONFIG_HOME/pdf_signatures (defaulting to ~/.config/pdf_signatures), or ~/.pdf_signatures.') | |||
| parser.add_argument('-S', '--print-path', help='Print signatures path.', action='store_true') | |||
There was a problem hiding this comment.
Perhaps -S and -s are confusingly similar?
|
@brunetton I came up with a different solution that requires no extra options. Essentially, the help text and error messages are customized to explain the situation on your particular system. Check my branch |
|
@brunetton PR rejected since the solution I mentioned above is now merged. Feel free to open an issue if you are not satisfied. |
Thank you for this great software, I was looking for a tool like this one since some weeks ago.
I used it today and I missed some details to understand how it works. So I updated README and added a command line option to print signatures folder.
Hope this looks okay to you. Tell me !
Thanks again !