First of all "bashscript" is a very poor name:
- It doesn't tell what the script does. "prepare-articles-for-review.sh" would be better. Or just "prepare-articles.sh"
- It is customer to use
.sh
extension for Bash scripts
If your typical use case is this:
for f in *.pdf; do ./bashscript.sh $f; done 2> /dev/null
Then it would be better to make the script handle multiple file parameters.
exit $E_BADARGS
I'm not familiar with such variable. I don't think it's a standard, and it's not mentioned in my man bash
. If the variable is undefined (and I think it is), then this will have the same effect as exit
, which is equivalent to exit $?
, where $?
is the exit code of the last statement, in this case the echo
on the previous line, which is most probably 0. I think you intended to exit with failure, with non-zero exit code.
NUM=$(pdftk $1 dump_data | grep 'NumberOfPages' | awk '{split($0,a,": "); print a[2]}')
Many problems here:
- You should double-quote
"$1"
, to protect from spaces in the filename
- The quoting in
grep 'NumberOfPages'
is redundant
- Are you sure there won't be more than one lines matching "NumberOfPages"? Just to be safe, I would add
exit
in the Awk command
- The Awk command can be written simpler as
awk -v FS=": " '{print $2}'
COMMSTR=''
You can simplify to:
COMMSTR=
for i in $(seq 1 $NUM);
seq
is not recommended because it's not portable. You can achieve the same using native Bash functionality:
for ((i=1; i<=$NUM; i++));
$(echo "" | ps2pdf -sPAPERSIZE=a4 - pageblanche.pdf)
The $(...)
wrapping is really pointless, and awful.
No need for echo ""
, simply echo
is exactly the same.
$(pdftk A=$1 B=pageblanche.pdf cat $COMMSTR output 'mod_'$1)
Does this work at all? Unfortunately I don't have pdftk
so cannot try.
Maybe it's fine, but it looks suspicious.
Btw, you don't need the quotes in 'mod_'$1
.
And again, the $(...)
wrapping is pointless.
(pdfnup 'mod_'$1 --nup 2x1 --landscape --outfile 'print_'$1)
As earlier, you don't need those quotes,
and the (...)
wrapping is pointless.
The same goes for the rest of the code.
Suggested implementation
I cannot fully test this because I don't have pdftk
, but it should work:
#!/bin/bash
if [ $# = 0 ]
then
echo "Usage: $0 file1.pdf file2.pdf ..."
exit 1
fi
for file; do
NUM=$(pdftk "$file" dump_data | awk -v FS=": " '/NumberOfPages/ { print $2; exit }')
COMMSTR=
for ((i=1; i<=$NUM; i++)); do COMMSTR="$COMMSTR A$i B1 "; done
blank=blank.pdf
ps2pdf -sPAPERSIZE=a4 - $blank < /dev/null
pdftk A="$file" B=$blank cat $COMMSTR output mod_"$file"
pdfnup mod_"$file" --nup 2x1 --landscape --outfile print_"$file"
rm $blank && rm mod_"$file"
done