Bash script for reviewing .pdf articles

Question

I wrote a bash script for reviewing scientific articles. It adds a blank page for notes and creates a new landscape file. I'm sure it can be made better code wise. Any suggestions?

#!/bin/bash
if [ $# -ne 1 ]
then
 echo "Usage example: ./bashscript src.pdf"
 exit $E_BADARGS
else
 NUM=$(pdftk 1ドル dump_data | grep 'NumberOfPages' | awk '{split(0,ドルa,": "); print a[2]}')
 COMMSTR=''
 for i in $(seq 1 $NUM);
 do
 COMMSTR="$COMMSTR A$i B1 " 
 done
 $(echo "" | ps2pdf -sPAPERSIZE=a4 - pageblanche.pdf)
 $(pdftk A=1ドル B=pageblanche.pdf cat $COMMSTR output 'mod_'1ドル)
 (pdfnup 'mod_'1ドル --nup 2x1 --landscape --outfile 'print_'1ドル)
 $(rm pageblanche.pdf && rm 'mod_'1ドル)
fi
#for f in *.pdf; do ./bashscript.sh $f; done 2> /dev/null

Answer 1

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

• ucyo: Works perfectly. Thank you for your time and work.
• Etan Reisner: exit is not equivalent to exit 0 it is equivalent to exit $?.
• janos: @EtanReisner well spotted! I clarified that point now. Thanks for the review-review, as usual ;-)