Fix/clarify meaning of depth parameter to json write methods

4 files changed, 58 insertions, 41 deletions

diff --git a/include/qpdf/JSON.hh b/include/qpdf/JSON.hh
index 2bc88605..554823e5 100644
--- a/include/qpdf/JSON.hh
+++ b/include/qpdf/JSON.hh
@@ -65,16 +65,26 @@ class JSON
     QPDF_DLL
     void write(Pipeline*, size_t depth = 0) const;
 
-    // Helper methods for writing JSON incrementally. Several methods
-    // take a `bool& first` parameter. The open methods always set it
-    // to true, and the methods to output items always set it to
-    // false. This way, the item and close methods can always know
-    // whether or not a first item is being written. The intended mode
-    // of operation is to start with `bool first = true` (though it
-    // doesn't matter how it's initialized) and just pass the same
-    // `first` through to all the methods, letting the JSON object use
-    // it to keep track of when it's writing a first object and when
-    // it's not.
+    // Helper methods for writing JSON incrementally.
+    //
+    // "first" -- Several methods take a `bool& first` parameter. The
+    // open methods always set it to true, and the methods to output
+    // items always set it to false. This way, the item and close
+    // methods can always know whether or not a first item is being
+    // written. The intended mode of operation is to start with a new
+    // `bool first = true` each time a new container is opened and
+    // to pass that `first` through to all the methods that are
+    // called to add top-level items to the container as well as to
+    // close the container. This lets the JSON object use it to keep
+    // track of when it's writing a first object and when it's not. If
+    // incrementally writing multiple levels of depth, a new `first`
+    // should used for each new container that is opened.
+    //
+    // "depth" -- Indicate the level of depth. This is used for
+    // consistent indentation. When writing incrementally, whenever
+    // you call a method to add an item to a container, the value of
+    // `depth` should be one more than whatever value is passed to the
+    // container open and close methods.
 
     // Open methods ignore the value of first and set it to false
     QPDF_DLL
@@ -106,9 +116,12 @@ class JSON
         Pipeline*, bool& first, JSON const& element, size_t depth = 0);
     // If writing nested structures incrementally, call writeNext
     // before opening a new array or container in the midst of an
-    // existing one. The first you pass to writeNext should be the one
-    // for the parent object. Then start a new first for the nested
-    // item.
+    // existing one. The `first` you pass to writeNext should be the
+    // one for the parent object. The depth should be the one for the
+    // child object. Then start a new `first` for the nested item.
+    // Note that writeDictionaryKey and writeArrayItem call writeNext
+    // for you, so this is most important when writing subsequent
+    // items or container openers to an array.
     QPDF_DLL
     static void writeNext(Pipeline* p, bool& first, size_t depth = 0);
 
diff --git a/libqpdf/JSON.cc b/libqpdf/JSON.cc
index d380049f..c825413e 100644
--- a/libqpdf/JSON.cc
+++ b/libqpdf/JSON.cc
@@ -56,7 +56,7 @@ JSON::writeNext(Pipeline* p, bool& first, size_t depth)
         *p << ",";
     }
     *p << "\n";
-    writeIndent(p, 1 + depth);
+    writeIndent(p, depth);
 }
 
 void
@@ -102,7 +102,7 @@ JSON::writeDictionaryItem(
     size_t depth)
 {
     writeDictionaryKey(p, first, key, depth);
-    value.write(p, 1 + depth);
+    value.write(p, depth);
 }
 
 void
@@ -110,7 +110,7 @@ JSON::writeArrayItem(
     Pipeline* p, bool& first, JSON const& element, size_t depth)
 {
     writeNext(p, first, depth);
-    element.write(p, 1 + depth);
+    element.write(p, depth);
 }
 
 void
@@ -119,7 +119,7 @@ JSON::JSON_dictionary::write(Pipeline* p, size_t depth) const
     bool first = true;
     writeDictionaryOpen(p, first, depth);
     for (auto const& iter: members) {
-        writeDictionaryItem(p, first, iter.first, iter.second, depth);
+        writeDictionaryItem(p, first, iter.first, iter.second, 1 + depth);
     }
     writeDictionaryClose(p, first, depth);
 }
@@ -130,7 +130,7 @@ JSON::JSON_array::write(Pipeline* p, size_t depth) const
     bool first = true;
     writeArrayOpen(p, first, depth);
     for (auto const& element: elements) {
-        writeArrayItem(p, first, element, depth);
+        writeArrayItem(p, first, element, 1 + depth);
     }
     writeArrayClose(p, first, depth);
 }
diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc
index 2cbabac0..06e93704 100644
--- a/libqpdf/QPDFJob.cc
+++ b/libqpdf/QPDFJob.cc
@@ -1082,7 +1082,7 @@ QPDFJob::doJSONObject(
     Pipeline* p, bool& first, std::string const& key, QPDFObjectHandle& obj)
 {
     if (this->m->json_version == 1) {
-        JSON::writeDictionaryItem(p, first, key, obj.getJSON(1, true), 1);
+        JSON::writeDictionaryItem(p, first, key, obj.getJSON(1, true), 2);
     } else {
         auto j = JSON::makeDictionary();
         if (obj.isStream()) {
@@ -1093,14 +1093,14 @@ QPDFJob::doJSONObject(
             j.addDictionaryMember(
                 "value", obj.getJSON(this->m->json_version, true));
         }
-        JSON::writeDictionaryItem(p, first, key, j, 1);
+        JSON::writeDictionaryItem(p, first, key, j, 2);
     }
 }
 
 void
 QPDFJob::doJSONObjects(Pipeline* p, bool& first, QPDF& pdf)
 {
-    JSON::writeDictionaryKey(p, first, "objects", 0);
+    JSON::writeDictionaryKey(p, first, "objects", 1);
     bool first_object = true;
     JSON::writeDictionaryOpen(p, first_object, 1);
     bool all_objects = m->json_objects.empty();
@@ -1124,7 +1124,7 @@ QPDFJob::doJSONObjects(Pipeline* p, bool& first, QPDF& pdf)
 void
 QPDFJob::doJSONObjectinfo(Pipeline* p, bool& first, QPDF& pdf)
 {
-    JSON::writeDictionaryKey(p, first, "objectinfo", 0);
+    JSON::writeDictionaryKey(p, first, "objectinfo", 1);
     bool first_object = true;
     JSON::writeDictionaryOpen(p, first_object, 1);
     bool all_objects = m->json_objects.empty();
@@ -1147,7 +1147,7 @@ QPDFJob::doJSONObjectinfo(Pipeline* p, bool& first, QPDF& pdf)
                                  this->m->json_version, true)
                            : JSON::makeNull()));
             JSON::writeDictionaryItem(
-                p, first_object, obj.unparse(), j_details, 1);
+                p, first_object, obj.unparse(), j_details, 2);
         }
     }
     JSON::writeDictionaryClose(p, first_object, 1);
@@ -1156,9 +1156,9 @@ QPDFJob::doJSONObjectinfo(Pipeline* p, bool& first, QPDF& pdf)
 void
 QPDFJob::doJSONPages(Pipeline* p, bool& first, QPDF& pdf)
 {
-    JSON::writeDictionaryKey(p, first, "pages", 0);
+    JSON::writeDictionaryKey(p, first, "pages", 1);
     bool first_page = true;
-    JSON::writeArrayOpen(p, first_page, 1);
+    JSON::writeArrayOpen(p, first_page, 2);
     QPDFPageLabelDocumentHelper pldh(pdf);
     QPDFOutlineDocumentHelper odh(pdf);
     int pageno = -1;
@@ -1232,7 +1232,7 @@ QPDFJob::doJSONPages(Pipeline* p, bool& first, QPDF& pdf)
                 "dest", oiter.getDest().getJSON(this->m->json_version, true));
         }
         j_page.addDictionaryMember("pageposfrom1", JSON::makeInt(1 + pageno));
-        JSON::writeArrayItem(p, first_page, j_page, 1);
+        JSON::writeArrayItem(p, first_page, j_page, 2);
     }
     JSON::writeArrayClose(p, first_page, 1);
 }
@@ -1262,7 +1262,7 @@ QPDFJob::doJSONPageLabels(Pipeline* p, bool& first, QPDF& pdf)
                 "label", (*iter).getJSON(this->m->json_version));
         }
     }
-    JSON::writeDictionaryItem(p, first, "pagelabels", j_labels, 0);
+    JSON::writeDictionaryItem(p, first, "pagelabels", j_labels, 1);
 }
 
 void
@@ -1306,7 +1306,7 @@ QPDFJob::doJSONOutlines(Pipeline* p, bool& first, QPDF& pdf)
     JSON j_outlines = JSON::makeArray();
     QPDFOutlineDocumentHelper odh(pdf);
     addOutlinesToJson(odh.getTopLevelOutlines(), j_outlines, page_numbers);
-    JSON::writeDictionaryItem(p, first, "outlines", j_outlines, 0);
+    JSON::writeDictionaryItem(p, first, "outlines", j_outlines, 1);
 }
 
 void
@@ -1374,7 +1374,7 @@ QPDFJob::doJSONAcroform(Pipeline* p, bool& first, QPDF& pdf)
                 "annotationflags", JSON::makeInt(aoh.getFlags()));
         }
     }
-    JSON::writeDictionaryItem(p, first, "acroform", j_acroform, 0);
+    JSON::writeDictionaryItem(p, first, "acroform", j_acroform, 1);
 }
 
 void
@@ -1470,7 +1470,7 @@ QPDFJob::doJSONEncrypt(Pipeline* p, bool& first, QPDF& pdf)
         "stringmethod", JSON::makeString(s_string_method));
     j_parameters.addDictionaryMember(
         "filemethod", JSON::makeString(s_file_method));
-    JSON::writeDictionaryItem(p, first, "encrypt", j_encrypt, 0);
+    JSON::writeDictionaryItem(p, first, "encrypt", j_encrypt, 1);
 }
 
 void
@@ -1532,7 +1532,7 @@ QPDFJob::doJSONAttachments(Pipeline* p, bool& first, QPDF& pdf)
                 null_or_string(QUtil::hex_encode(efs.getChecksum())));
         }
     }
-    JSON::writeDictionaryItem(p, first, "attachments", j_attachments, 0);
+    JSON::writeDictionaryItem(p, first, "attachments", j_attachments, 1);
 }
 
 JSON
@@ -1755,7 +1755,7 @@ QPDFJob::doJSON(QPDF& pdf, Pipeline* p)
     // ignore unrecognized keys, so we only update the version of a
     // key disappears or if its value changes meaning.
     JSON::writeDictionaryItem(
-        p, first, "version", JSON::makeInt(this->m->json_version), 0);
+        p, first, "version", JSON::makeInt(this->m->json_version), 1);
     JSON j_params = JSON::makeDictionary();
     std::string decode_level_str;
     switch (m->decode_level) {
@@ -1774,7 +1774,7 @@ QPDFJob::doJSON(QPDF& pdf, Pipeline* p)
     }
     j_params.addDictionaryMember(
         "decodelevel", JSON::makeString(decode_level_str));
-    JSON::writeDictionaryItem(p, first, "parameters", j_params, 0);
+    JSON::writeDictionaryItem(p, first, "parameters", j_params, 1);
 
     bool all_keys = m->json_keys.empty();
     // The list of selectable top-level keys id duplicated in the
diff --git a/libqpdf/QPDF_json.cc b/libqpdf/QPDF_json.cc
index 5527318c..59aac05e 100644
--- a/libqpdf/QPDF_json.cc
+++ b/libqpdf/QPDF_json.cc
@@ -739,7 +739,7 @@ QPDF::writeJSONStream(
         obj.getStreamJSON(
             version, json_stream_data, decode_level, stream_p, filename));
 
-    JSON::writeDictionaryItem(p, first, key, j, 2);
+    JSON::writeDictionaryItem(p, first, key, j, 3);
     if (f) {
         f_pl->finish();
         f_pl = nullptr;
@@ -757,7 +757,7 @@ QPDF::writeJSONObject(
 {
     auto j = JSON::makeDictionary();
     j.addDictionaryMember("value", obj.getJSON(version, true));
-    JSON::writeDictionaryItem(p, first, key, j, 2);
+    JSON::writeDictionaryItem(p, first, key, j, 3);
 }
 
 void
@@ -774,19 +774,23 @@ QPDF::writeJSON(
             "QPDF::writeJSON: only version 2 is supported");
     }
     bool first = true;
-    JSON::writeDictionaryOpen(p, first, 0);
-    JSON::writeDictionaryKey(p, first, "qpdf-v2", 0);
+    if (complete) {
+        JSON::writeDictionaryOpen(p, first, 0);
+    } else {
+        first = first_key;
+    }
+    JSON::writeDictionaryKey(p, first, "qpdf-v2", 1);
     bool first_qpdf = true;
-    JSON::writeDictionaryOpen(p, first_qpdf, 1);
+    JSON::writeDictionaryOpen(p, first_qpdf, 2);
     JSON::writeDictionaryItem(
-        p, first_qpdf, "pdfversion", JSON::makeString(getPDFVersion()), 1);
+        p, first_qpdf, "pdfversion", JSON::makeString(getPDFVersion()), 2);
     JSON::writeDictionaryItem(
         p,
         first_qpdf,
         "maxobjectid",
         JSON::makeInt(QIntC::to_longlong(getObjectCount())),
-        1);
-    JSON::writeDictionaryKey(p, first_qpdf, "objects", 1);
+        2);
+    JSON::writeDictionaryKey(p, first_qpdf, "objects", 2);
     bool first_object = true;
     JSON::writeDictionaryOpen(p, first_object, 2);
     bool all_objects = wanted_objects.empty();