[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/json_writer into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
The problem with the streaming was that it's very hacky and hard to read. So, we have sacrificed some efficiency and gained a lot of readability/code stability. Thanks for the review! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
This affects uploading a map onto the website (wl_map_info) and creating the encyclopedia (wl_map_object_info) https://wl.widelands.org/encyclopedia/ -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Review: Approve code review OK, for the Reeview: This branch substitutes a streaming JSON aproach with a Tree Object model. Streaming is normally better as of resource usage, but in this case this does not matter. If kaputtnik likes it his way, its all fine with me. (Solange nix kaputt geht ;-) Please explain which website tools are actually affected. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
It has already been tested, what's missing is a code review. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Anything I can test for this? -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Continuous integration builds have changed state: Travis build 4401. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/478975042. Appveyor build 4192. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_json_writer-4192. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Continuous integration builds have changed state: Travis build 4395. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/478759403. Appveyor build 4186. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_json_writer-4186. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Continuous integration builds have changed state: Travis build 4178. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/447958040. Appveyor build 3976. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_json_writer-3976. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/json_writer into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
Continuous integration builds have changed state: Travis build 4173. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/447150251. Appveyor build 3971. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_json_writer-3971. -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/json_writer into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/json_writer into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/json_writer into
lp:widelands.
Commit message:
Refactor website binaries to use a real tree data structure for writing JSON
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908
Getting rid of some spaghetti code. Only affects the website, not Widelands
itself.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/json_writer into lp:widelands.
=== modified file 'src/website/CMakeLists.txt'
--- src/website/CMakeLists.txt 2018-08-14 20:42:18 +
+++ src/website/CMakeLists.txt 2018-10-27 16:48:10 +
@@ -1,3 +1,5 @@
+add_subdirectory(json)
+
wl_library(website_common
SRCS
website_common.cc
@@ -23,6 +25,7 @@
graphic_surface
io_fileread
io_filesystem
+json
logic
map_io_map_loader
website_common
@@ -38,6 +41,7 @@
graphic
io_fileread
io_filesystem
+json
logic
logic_tribe_basic_info
website_common
=== added directory 'src/website/json'
=== added file 'src/website/json/CMakeLists.txt'
--- src/website/json/CMakeLists.txt 1970-01-01 00:00:00 +
+++ src/website/json/CMakeLists.txt 2018-10-27 16:48:10 +
@@ -0,0 +1,10 @@
+wl_library(json
+ SRCS
+json.cc
+json.h
+value.cc
+value.h
+ DEPENDS
+io_fileread
+io_filesystem
+)
=== added file 'src/website/json/json.cc'
--- src/website/json/json.cc 1970-01-01 00:00:00 +
+++ src/website/json/json.cc 2018-10-27 16:48:10 +
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2018 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "io/filewrite.h"
+#include "website/json/json.h"
+
+// ## JSON Element #
+
+namespace JSON {
+const std::string JSON::Element::tab_ = " ";
+
+JSON::Object* Element::add_object(const std::string& key) {
+ children_.push_back(std::unique_ptr(new JSON::Object(key, level_ + 1)));
+ return dynamic_cast(children_.back().get());
+}
+
+JSON::Array* Element::add_array(const std::string& key) {
+ children_.push_back(std::unique_ptr(new JSON::Array(key, level_ + 1)));
+ return dynamic_cast(children_.back().get());
+}
+
+void Element::add_bool(const std::string& key, bool value) {
+ values_.push_back(std::make_pair(key, std::unique_ptr(new JSON::Boolean(value;
+}
+
+void Element::add_double(const std::string& key, double value) {
+ values_.push_back(std::make_pair(key, std::unique_ptr(new JSON::Double(value;
+}
+void Element::add_int(const std::string& key, int value) {
+ values_.push_back(std::make_pair(key, std::unique_ptr(new JSON::Int(value;
+}
+void Element::add_empty(const std::string& key) {
+ values_.push_back(std::make_pair(key, std::unique_ptr(new JSON::Empty(;
+}
+void Element::add_string(const std::string& key, const std::string& value) {
+ values_.push_back(std::make_pair(key, std::unique_ptr(new JSON::String(value;
+}
+
+void Element::write_to_file(FileSystem& fs, const std::string& filename) const {
+ FileWrite file_writer;
+ file_writer.text(as_string());
+ file_writer.write(fs, filename);
+}
+
+std::string Element::as_string() const {
+ return "{\n" + children_as_string() + "}\n";
+}
+
+std::string Element::values_as_string(const std::string& tabs) const {
+ std::string result = "";
+ if (!values_.empty()) {
+ for (size_t i = 0; i < values_.size() - 1; ++i) {
+ const auto& element = values_.at(i);
+ const std::string element_as_string = element.second->as_string();
+ result +=
+ tabs + tab_ + key_to_string(element.first, element_as_string.empty()) + element_as_string + ",\n";
+ }
+ const auto& element = values_.at(values_.size() - 1);
+ const std::string element_as_string = element.second->as_string();
+ result += tabs + tab_ + key_to_string(element.first, element_as_string.empty()) + element_as_string +
+ (children_.empty() ? "\n" : ",\n");
+ }
+ return result;
+}
+
+std::string Element::children_as_string() const {
+ std::string result = &q
