Use "save" logger when saving data to standard output

This includes the output PDF, streams from --show-object and
attachments from --save-attachment. This also enables --verbose and
--progress to work with saving to stdout.

7 files changed, 60 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index ca652667..f0c14ff9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2022-06-18  Jay Berkenbilt  <ejb@ql.org>
+
+        * When --progress or --verbose is combined with writing to
+        standard output, progress reporting and verbose messages go to
+        standard error. Previously it was disabled in this case.
+
 2022-06-05  Jay Berkenbilt  <ejb@ql.org>
 
         * QPDFJob: API breaking change: QPDFJob::doIfVerbose passes a
diff --git a/TODO b/TODO
index b73cad5b..02dacc20 100644
--- a/TODO
+++ b/TODO
@@ -79,9 +79,6 @@ Find all places in the library that write to stdout/stderr/cout/cerr.
 Also find places that raise exceptions if unable to warn. These should
 use the global output writer.
 
-Figure out a good way to use the save pipeline for QPDFWriter as well
-as saving attachments, showing stream data, etc.
-
 
 QPDFPagesTree
 =============
diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc
index 4ea03edb..9f76bdf4 100644
--- a/libqpdf/QPDFJob.cc
+++ b/libqpdf/QPDFJob.cc
@@ -691,22 +691,21 @@ QPDFJob::checkConfiguration()
               " before the -- that follows --encrypt.");
     }
 
+    bool save_to_stdout = false;
     if (m->require_outfile && m->outfilename &&
         (strcmp(m->outfilename.get(), "-") == 0)) {
         if (m->split_pages) {
             usage("--split-pages may not be used when"
                   " writing to standard output");
         }
-        if (this->m->verbose) {
-            usage("--verbose may not be used when"
-                  " writing to standard output");
-        }
-        if (m->progress) {
-            usage("--progress may not be used when"
-                  " writing to standard output");
-        }
+        save_to_stdout = true;
+    }
+    if (!m->attachment_to_show.empty()) {
+        save_to_stdout = true;
+    }
+    if (save_to_stdout) {
+        this->m->log->saveToStandardOutput();
     }
-
     if ((!m->split_pages) &&
         QUtil::same_file(m->infilename.get(), m->outfilename.get())) {
         QTC::TC("qpdf", "QPDFJob same file error");
@@ -918,10 +917,11 @@ QPDFJob::doShowObj(QPDF& pdf)
                 obj.warnIfPossible("unable to filter stream data");
                 error = true;
             } else {
-                QUtil::binary_stdout();
-                Pl_StdioFile out("stdout", stdout);
+                // If anything has been written to standard output,
+                // this will fail.
+                this->m->log->saveToStandardOutput();
                 obj.pipeStreamData(
-                    &out,
+                    this->m->log->getSave().get(),
                     (filter && m->normalize) ? qpdf_ef_normalize : 0,
                     filter ? qpdf_dl_all : qpdf_dl_none);
             }
@@ -1023,9 +1023,10 @@ QPDFJob::doShowAttachment(QPDF& pdf)
             "attachment " + m->attachment_to_show + " not found");
     }
     auto efs = fs->getEmbeddedFileStream();
-    QUtil::binary_stdout();
-    Pl_StdioFile out("stdout", stdout);
-    efs.pipeStreamData(&out, 0, qpdf_dl_all);
+    // saveToStandardOutput has already been called, but it's harmless
+    // to call it again, so do as defensive coding.
+    this->m->log->saveToStandardOutput();
+    efs.pipeStreamData(this->m->log->getSave().get(), 0, qpdf_dl_all);
 }
 
 void
@@ -3138,14 +3139,17 @@ QPDFJob::setWriterOptions(QPDF& pdf, QPDFWriter& w)
         parse_version(m->force_version, version, extension_level);
         w.forcePDFVersion(version, extension_level);
     }
-    if (m->progress && m->outfilename) {
+    if (m->progress) {
+        char const* outfilename = this->m->outfilename
+            ? this->m->outfilename.get()
+            : "standard output";
         w.registerProgressReporter(
             std::shared_ptr<QPDFWriter::ProgressReporter>(
                 // line-break
                 new ProgressReporter(
                     *this->m->log->getInfo(),
                     this->m->message_prefix,
-                    m->outfilename.get())));
+                    outfilename)));
     }
 }
 
@@ -3273,7 +3277,15 @@ QPDFJob::writeOutfile(QPDF& pdf)
     } else {
         // QPDFWriter must have block scope so the output file will be
         // closed after write() finishes.
-        QPDFWriter w(pdf, m->outfilename.get());
+        QPDFWriter w(pdf);
+        if (this->m->outfilename) {
+            w.setOutputFilename(m->outfilename.get());
+        } else {
+            // saveToStandardOutput has already been called, but
+            // calling it again is defensive and harmless.
+            this->m->log->saveToStandardOutput();
+            w.setOutputPipeline(this->m->log->getSave().get());
+        }
         setWriterOptions(pdf, w);
         w.write();
     }
diff --git a/libqpdf/QPDFLogger.cc b/libqpdf/QPDFLogger.cc
index 859d6dcd..3b25c050 100644
--- a/libqpdf/QPDFLogger.cc
+++ b/libqpdf/QPDFLogger.cc
@@ -2,6 +2,7 @@
 
 #include <qpdf/Pl_Discard.hh>
 #include <qpdf/Pl_OStream.hh>
+#include <qpdf/QUtil.hh>
 #include <iostream>
 #include <stdexcept>
 
@@ -182,6 +183,9 @@ QPDFLogger::setError(std::shared_ptr<Pipeline> p)
 void
 QPDFLogger::setSave(std::shared_ptr<Pipeline> p)
 {
+    if (this->m->p_save == p) {
+        return;
+    }
     if (p == this->m->p_stdout) {
         auto pt = dynamic_cast<Pl_Track*>(p.get());
         if (pt->getUsed()) {
@@ -192,6 +196,7 @@ QPDFLogger::setSave(std::shared_ptr<Pipeline> p)
         if (this->m->p_info == this->m->p_stdout) {
             this->m->p_info = this->m->p_stderr;
         }
+        QUtil::binary_stdout();
     }
     this->m->p_save = p;
 }
diff --git a/manual/release-notes.rst b/manual/release-notes.rst
index 54ae799f..0e48c0f6 100644
--- a/manual/release-notes.rst
+++ b/manual/release-notes.rst
@@ -133,6 +133,10 @@ For a detailed list of changes, please see the file
       user password is not recoverable from the owner password when
       256-bit keys are in use.
 
+    - ``--verbose`` and ``--progress`` may be now used when writing
+      the output PDF to standard output. In that case, the verbose and
+      progress messages are written to standard error.
+
   - Library Enhancements
 
     - New methods ``insertItemAndGet``, ``appendItemAndGet``,
diff --git a/qpdf/qtest/progress-reporting.test b/qpdf/qtest/progress-reporting.test
index 3c7fb2cf..fe39a0df 100644
--- a/qpdf/qtest/progress-reporting.test
+++ b/qpdf/qtest/progress-reporting.test
@@ -14,13 +14,23 @@ cleanup();
 
 my $td = new TestDriver('progress-reporting');
 
-my $n_tests = 1;
+my $n_tests = 3;
 
 $td->runtest("progress report on small file",
-             {$td->COMMAND => "qpdf --progress minimal.pdf a.pdf",
+             {$td->COMMAND =>
+                  "qpdf --progress --deterministic-id minimal.pdf a.pdf",
                   $td->FILTER => "perl filter-progress.pl"},
              {$td->FILE => "small-progress.out", $td->EXIT_STATUS => 0},
              $td->NORMALIZE_NEWLINES);
+$td->runtest("progress report to stdout",
+             {$td->COMMAND =>
+                  "qpdf --progress --deterministic-id minimal.pdf - > b.pdf",
+                  $td->FILTER => "perl filter-progress.pl"},
+             {$td->FILE => "small-stdout-progress.out", $td->EXIT_STATUS => 0},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("compare",
+             {$td->FILE => "a.pdf"},
+             {$td->FILE => "b.pdf"});
 
 cleanup();
 $td->report($n_tests);
diff --git a/qpdf/qtest/qpdf/small-stdout-progress.out b/qpdf/qtest/qpdf/small-stdout-progress.out
new file mode 100644
index 00000000..c1ec4e64
--- /dev/null
+++ b/qpdf/qtest/qpdf/small-stdout-progress.out
@@ -0,0 +1,3 @@
+qpdf: standard output: write progress: 0%
+....other write progress....
+qpdf: standard output: write progress: 100%