Added indent to arrays

[mathieu]

I had this :

{
 "Fer" :  {
  "color" : [
0.296361,
0.307408,
0.331609,
1.0
  ]
 }
}

And wanted this :

{
 "Fer" :  {
  "color" : [
   0.296361,
   0.307408,
   0.331609,
   1.0
  ]
 }
}

[flavio]

Hi, thanks for your contribution. However there are a couple of issues to address.

1. Build is broken

The code is not building:

flavio@roesti ~/hacking/qjson/build ±mathieu-array_indent⚡ » make
Scanning dependencies of target qjson
[  4%] Building CXX object src/CMakeFiles/qjson.dir/serializer.cpp.o
/home/flavio/hacking/qjson/src/serializer.cpp: In member function ‘QByteArray QJson::Serializer::SerializerPrivate::serialize(const QVariant&, bool*, int)’:
/home/flavio/hacking/qjson/src/serializer.cpp:98:42: error: expected ‘)’ before ‘innerIndent’
       str = "[\n" + join( values, ",\n"  innerIndent ) + "\n" + indent + "]";
                                          ^
make[2]: *** [src/CMakeFiles/qjson.dir/serializer.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/qjson.dir/all] Error 2
make: *** [all] Error 2

I think it can be fixed in the following way:

diff --git a/src/serializer.cpp b/src/serializer.cpp
index cb1e7f1..12bef7e 100644
--- a/src/serializer.cpp
+++ b/src/serializer.cpp
@@ -95,7 +95,7 @@ QByteArray Serializer::SerializerPrivate::serialize( const QVariant &v, bool *ok
     else if (indentMode == QJson::IndentMedium || indentMode == QJson::IndentFull) {
       QByteArray indent = buildIndent(indentLevel);
       QByteArray innerIndent = buildIndent(indentLevel + 1);
-      str = "[\n" + join( values, ",\n"  innerIndent ) + "\n" + indent + "]";
+      str = "[\n" + join( values, ",\n" + innerIndent) + "\n" + indent + "]";
     }
     else if (indentMode == QJson::IndentCompact) {
       str = "[" + join( values, "," ) + "]";

Could you please fix your pull request?

2. Indentation is still slightly broken

I created a indent_array.json file with the following contents:

{
 "Fer" :  {
  "color" : [
0.296361,
0.307408,
0.331609,
1.0
  ]
 }
}

And I used the cmdline_tester tool to check parsing -> serialization -> indentation.

That's the output I got:

flavio@roesti ~/hacking/qjson/build ±mathieu-array_indent⚡ » ./tests/cmdline_tester/cmdline_tester --serialize --indent full  indent_array.json         
Parsing of "indent_array.json" took 0 ms 
QVariant(QVariantMap, QMap(("Fer", QVariant(QVariantMap, QMap(("color", QVariant(QVariantList, (QVariant(double, 0.296361) ,  QVariant(double, 0.307408) ,  QVariant(double, 0.331609) ,  QVariant(double, 1) )  ) ) )  ) ) )  ) 
Serializing...  
Serialization took: 0 ms 
"{
 "Fer" :  {
  "color" : [
0.296361,
   0.307408,
   0.331609,
   1.0
  ]
 }
}" 
JOB DONE, BYE

As you can see the first item of the array is still indented wrong.

3. Update the test suite

I need you to update qjson's test suite in order to merge your code.

There's already a unit test covering indentation, you can find it inside of test/serializer/testserializer.cpp. Just add new data to be tested to TestSerializer::testIndentation_data().

[mathieu]

Hi @flavio !

I'm ashamed I did the initial request while editing in the web browser. I did not test build nor check the unit tests. This is an update (big refactoring) that involves modifications on the way arrays are indented and tested. I also added compact to the cmdline_tester indent flags

One of the caveats is that the way it works one cannot determine if a 'simple' value needs indenting or not, so there is a fair amount of adding indents and removing them depending of the IndentMode choosen.

I also used switches instead of if then else as I found it easier to understand which code path was taken on the differents IndentMode.

What do you think of this now ? Regards, @mathieu

[flavio]

Hi @mathieu, thanks for your contribution. Everything looks fine to me, except for the small issues I pointed up above.

Could you please fix them and then merge all your changes into a single commit?

You can do that inside of your branch with git rebase -i and then with a git push --force.

[mathieu]

Hi @flavio, I fixed the issues you outlined and squashed my pull request to a single commit. Regards, @mathieu

[flavio]

@mathieu: thanks for your contribution!