Home directory pollution

[svenssonaxel]

In GUI mode, the user can choose among files in $PDF_SIGNATURE_DIR or ~/.pdf_signatures. Perhaps the default should be ~/.local/SOMETING/pdf_signatures.

[ambroisie]

I would suggest using a sub-folder in XDG_DATA_HOME (which defaults to ~/.local/share/). To quote the spec:

There is a single base directory relative to which user-specific data files should be written.

If we consider signatures to be configuration instead of data (which is debatable), then XDG_CONFIG_HOME (i.e: ~/.config/) is another good candidate.

[svenssonaxel]

@ambroisie Something like this? Only one directory is searched for signatures. This directory is $PDF_SIGNATURE_DIR if it's set, otherwise $XDG_CONFIG_HOME/pdf_signatures if $XDG_CONFIG_HOME is set, otherwise ~/.config/pdf_signatures if it exists, otherwise ~/.pdf_signatures.

diff --git a/pdf-sign b/pdf-sign
index b83a954..da2b805 100755
--- a/pdf-sign
+++ b/pdf-sign
@@ -4,7 +4,11 @@

 import argparse, os, queue, re, subprocess, sys, tempfile, time

-signatureDir=os.path.expanduser(os.environ['PDF_SIGNATURE_DIR'] if 'PDF_SIGNATURE_DIR' in os.environ else "~/.pdf_signatures")
+signatureDir=os.path.expanduser(
+    os.environ['PDF_SIGNATURE_DIR'] if 'PDF_SIGNATURE_DIR' in os.environ else
+    os.path.join(os.environ['XDG_CONFIG_HOME'], 'pdf_signatures') if 'XDG_CONFIG_HOME' in os.environ else
+    "~/.config/pdf_signatures" if os.path.exists(os.path.expanduser("~/.config/pdf_signatures")) else
+    "~/.pdf_signatures")

 # Inspired by https://unix.stackexchange.com/a/141496
 def main(args):
@@ -36,6 +40,8 @@ def main(args):
         if not args.signature and args.batch:
             die('In batch mode, signature must be specified.')
         signatures=[*filter(lambda x: m("^.*\.pdf$", x), os.listdir(signatureDir))] if not args.signature else [None]
+        if not signatures:
+            die(f'No .pdf files found in {signatureDir}')
         signatureIndex=Cell(0)
         signaturePath=Cell(lambda: args.signature if args.signature else os.path.join(signatureDir, signatures[signatureIndex()]))
         signatureSize=Cell(lambda: pdfGetSize(signaturePath()))
@@ -384,7 +390,7 @@ if not 'BooleanOptionalAction' in dir(argparse):
 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 or ~/.pdf_signatures.')
+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('-x', '--x-coordinate', type=float, default=0.5, help='Horizontal coordinate of signature center, in page width units. (default: 0.5)')
 parser.add_argument('-y', '--y-coordinate', type=float, default=0.75, help='Vertical coordinate of signature center, in page height units. (default: 0.75)')
 parser.add_argument('-W', '--width', type=float, default=0.28, help='Width of box to fit signature to, in page width units. (default: 0.28)')

[ambroisie]

That sounds good to me (this fallback order is usually the one used by programs with legacy directories).

On a code-quality standpoint I would suggest extracting the fallback behaviour to a helper function rather than in-lining the ternary expressions.

If in the future your program adds ways of writing to its signature directory, I would suggest defaulting to the XDG-compliant location rather than the ~/.pdf_signatures one when none of the folders exist.