From be009f6d2c68f26113832343db72a27d16481cb2 Mon Sep 17 00:00:00 2001 From: Olivier XILLO Date: Tue, 9 Feb 2021 20:41:13 +0100 Subject: [PATCH] ofXML fixes (#6678) #changelog #core --- libs/openFrameworks/utils/ofXml.cpp | 36 ++++- libs/openFrameworks/utils/ofXml.h | 24 +++- tests/utils/xml/addons.make | 1 + tests/utils/xml/src/main.cpp | 195 ++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 11 deletions(-) create mode 100644 tests/utils/xml/addons.make create mode 100644 tests/utils/xml/src/main.cpp diff --git a/libs/openFrameworks/utils/ofXml.cpp b/libs/openFrameworks/utils/ofXml.cpp index 66a81a53f68..33468a5e2d9 100644 --- a/libs/openFrameworks/utils/ofXml.cpp +++ b/libs/openFrameworks/utils/ofXml.cpp @@ -1,5 +1,6 @@ #include "ofXml.h" #include "ofUtils.h" +#include using namespace std; @@ -16,11 +17,13 @@ ofXml::ofXml(std::shared_ptr doc, const pugi::xml_node & xml bool ofXml::load(const std::filesystem::path & file){ auto auxDoc = std::make_shared(); - if(auxDoc->load_file(ofToDataPath(file).c_str())){ + auto res = auxDoc->load_file(ofToDataPath(file).c_str()); + if( res ){ doc = auxDoc; xml = doc->root(); return true; }else{ + ofLogWarning("ofXml") << "Cannot load file " << file << ":" << res.description(); return false; } } @@ -46,7 +49,10 @@ bool ofXml::parse(const std::string & xmlStr){ bool ofXml::save(const std::filesystem::path & file) const{ if(xml == doc->root()){ - return doc->save_file(ofToDataPath(file).c_str()); + auto res = doc->save_file(ofToDataPath(file).c_str()); + ofLogVerbose("ofXml")<<"ofXML Save : "<< res; + ofLogVerbose("ofXml")<toString(); + return res; }else{ pugi::xml_document doc; if(doc.append_copy(xml.root())){ @@ -201,7 +207,7 @@ bool ofXml::removeAttribute(ofXml::Attribute && attr){ ofXml ofXml::findFirst(const std::string & path) const{ try{ - return ofXml(doc, this->xml.select_single_node(path.c_str()).node()); + return ofXml(doc, this->xml.select_node(path.c_str()).node()); }catch(pugi::xpath_exception & e){ return ofXml(); } @@ -240,11 +246,19 @@ unsigned int ofXml::getUintValue() const{ } float ofXml::getFloatValue() const{ - return this->xml.text().as_float(); + auto loc = std::setlocale( LC_NUMERIC, NULL ); + std::setlocale( LC_NUMERIC, "C" ); + float f = this->xml.text().as_float(); + std::setlocale( LC_NUMERIC, loc ); + return f; } double ofXml::getDoubleValue() const{ - return this->xml.text().as_double(); + auto loc = std::setlocale( LC_NUMERIC, NULL ); + std::setlocale( LC_NUMERIC, "C" ); + float d = this->xml.text().as_double(); + std::setlocale( LC_NUMERIC, loc ); + return d; } bool ofXml::getBoolValue() const{ @@ -284,11 +298,19 @@ unsigned int ofXml::Attribute::getUintValue() const{ } float ofXml::Attribute::getFloatValue() const{ - return this->attr.as_float(); + auto loc = std::setlocale( LC_NUMERIC, NULL ); + std::setlocale( LC_NUMERIC, "C" ); + float f = this->attr.as_float(); + std::setlocale( LC_NUMERIC, loc ); + return f; } double ofXml::Attribute::getDoubleValue() const{ - return this->attr.as_double(); + auto loc = std::setlocale( LC_NUMERIC, NULL ); + std::setlocale( LC_NUMERIC, "C" ); + float d = this->attr.as_double(); + std::setlocale( LC_NUMERIC, loc ); + return d; } bool ofXml::Attribute::getBoolValue() const{ diff --git a/libs/openFrameworks/utils/ofXml.h b/libs/openFrameworks/utils/ofXml.h index 5fb5b438a0c..c4006615ff2 100644 --- a/libs/openFrameworks/utils/ofXml.h +++ b/libs/openFrameworks/utils/ofXml.h @@ -250,24 +250,40 @@ class ofXmlIterator{ } const ofXmlIterator& operator++(){ - this->xml = xml.getNextSibling(); + if( std::is_same::value ){ + this->xml = xml.getNextSibling( xml.getName() ); + }else{ + this->xml = xml.getNextSibling(); + } return *this; } ofXmlIterator operator++(int){ auto now = xml; - this->xml = xml.getNextSibling(); + if( std::is_same::value ){ + this->xml = xml.getNextSibling( xml.getName() ); + }else{ + this->xml = xml.getNextSibling(); + } return now; } const ofXmlIterator& operator--(){ - this->xml = xml.getPreviousSibling(); + if( std::is_same::value ){ + this->xml = xml.getPreviousSibling( xml.getName() ); + }else{ + this->xml = xml.getPreviousSibling(); + } return *this; } ofXmlIterator operator--(int){ auto now = xml; - this->xml = xml.getPreviousSibling(); + if( std::is_same::value ){ + this->xml = xml.getPreviousSibling( xml.getName() ); + }else{ + this->xml = xml.getPreviousSibling(); + } return now; } typedef It Base; diff --git a/tests/utils/xml/addons.make b/tests/utils/xml/addons.make new file mode 100644 index 00000000000..0e0303d2e09 --- /dev/null +++ b/tests/utils/xml/addons.make @@ -0,0 +1 @@ +ofxUnitTests diff --git a/tests/utils/xml/src/main.cpp b/tests/utils/xml/src/main.cpp new file mode 100644 index 00000000000..4c60d3fac16 --- /dev/null +++ b/tests/utils/xml/src/main.cpp @@ -0,0 +1,195 @@ +#include "utils/ofXml.h" +#include "ofxUnitTests.h" +#include + +using namespace std; + + +class ofApp: public ofxUnitTestsApp{ + void run(){ + + // Create an ofXml object - it should be empty + ofXml xml; + ofxTest(xml.getChildren().begin()==xml.getChildren().end(), "a newly created ofXml object should be empty"); + + // Create the root element + ofLogNotice() << "Building a SVG document..."; + auto svg = xml.appendChild("svg"); + + ofLogNotice() << "Testing attributes"; + ofLogVerbose() << "\tSetting '@width' to 500"; + svg.setAttribute("width",500); + ofLogVerbose() << "\tAdd '@height' and set it to 200"; + svg.appendAttribute("height"); + svg.getAttribute("height").set(200); + ofxTestEq(svg.getFirstAttribute().getName(),"width", "\tfirst attribute should be 'width'"); + ofxTestEq(svg.getLastAttribute().getName(),"height", "\tlast attribute should be 'height'"); + + ofLogVerbose() << "\tAdding svg namespace in first position"; + svg.prependAttribute("xmlns"); + svg.setAttribute("xmlns","http://www.w3.org/2000/svg"); + ofxTestEq(svg.getFirstAttribute().getName(),"xmlns", "\tfirst attribute should now be 'xmlns'"); + + ofLogVerbose() << "\tAdding more attributes"; + svg.appendAttribute("unexpected"); + svg.appendAttribute("version"); + ofxTestEq(svg.getLastAttribute().getName(),"version", "\tlast attribute should now be 'version'"); + ofLogVerbose() << "\tand remove some them"; + svg.removeAttribute( svg.getLastAttribute() ); //remove @version + svg.removeAttribute("unexpected"); //remove @unexpected + { // Add and remove attribute + auto a = svg.appendAttribute("todelete"); + svg.removeAttribute( a ); + } + ofxTestEq(svg.getLastAttribute().getName(),"height", "\tlast attribute should noww be 'height'"); + { // Iterate over attributes + std::string attrs(""); + for (auto it=svg.getAttributes().begin(); it!=svg.getAttributes().end(); ++it){ + attrs += it->getName() + ":" + it->getValue() + " "; + } + ofxTestEq(attrs,"xmlns:http://www.w3.org/2000/svg width:500 height:200 ", "\titerating over attributes to get their values"); + } + ofLogNotice()<<"\n"; + + ofLogNotice() << "Creating elements"; + { + auto el = svg.appendChild("circle"); + el.setAttribute("id","circle1"); + el.setAttribute("r",10.5); + el = svg.appendChild("circle"); + el.setAttribute("id","circle2"); + el.setAttribute("r",int(21)); + el = svg.appendChild("rect"); + el.setAttribute("id","rect1"); + el.setAttribute("width",200); + el.setAttribute("height",100); + + + // add a text element first + el = svg.prependChild("text"); + el.setAttribute("id","text1"); + // add an extra circle by adding an ellipse and changing its tag name + el = svg.appendChild("ellipse"); + el.setName("circle"); + el.setAttribute("id","circle3"); + + // build 2 other XML tree - SVG groups + // and add them at begin and end + ofXml otherxml; + el = otherxml.appendChild("g"); + el.setAttribute("id","group1"); + el.appendChild("line").setAttribute("id","line1"); + el.appendChild("image").setAttribute("id","image1"); + svg.prependChild(el); + + el = otherxml.appendChild("g"); + el.setAttribute("id","group2"); + el.appendChild("ellipse").setAttribute("id","ellipse1"); + el.appendChild("path").setAttribute("id","path1"); + svg.appendChild(el); + + + /*At this time, the XML should look like + + + + + + + + + + */ + auto child = svg.getFirstChild(); + ofxTestEq(child.getName(), "g", "\tfirst child should be "); + child = child.getNextSibling(); + ofxTestEq(child.getAttribute("id").getValue(), "text1", "\tfollowed by "); + child = child.getNextSibling("rect"); + ofxTestEq(child.getAttribute("id").getValue(), "rect1", "\twhich has a following-sibling '"); + child = child.getPreviousSibling(); + ofxTestEq(child.getAttribute("id").getValue(), "circle2", "\tprevious child should be 'circle2'"); + child = child.getPreviousSibling("g"); + ofxTestEq(child.getAttribute("id").getValue(), "group1", "\tprevious 'g' child should be 'group1'"); + child = child.getPreviousSibling("g"); + ofxTest(!child, "\tthere should be no previous sibling 'g'"); + child = svg.getLastChild(); + ofxTestEq(child.getAttribute("id").getValue(), "group2", "\tlast child should be 'group2'"); + ofxTest(!child.getPreviousSibling("ellipse"), "\tthere should be no previous 'ellipse' child"); + if( (child = svg.getNextSibling("rect").getNextSibling("circle")) ) { + ofxTest(child.getNextSibling("circle"), "\tthere should be no other sibling 'circle'"); + } + ofLogNotice()<<"\n"; + + ofLogNotice()<<"Iterating over children"; + std::string ids(""); + auto children = svg.getChildren(); + for (auto it=children.begin(); it!=children.end(); it++){ + ids += it->getAttribute("id").getValue(); + } + ofxTestEq(ids,"group1text1circle1circle2rect1circle3group2", "\telements are in expected order with no reference to grandchildren"); + ids=""; + for (auto& c : svg.getChildren("circle") ){ + ids += c.getAttribute("id").getValue(); + } + ofxTestEq(ids,"circle1circle2circle3", "\tcircle elements are the expected ones"); + ids=""; + auto children3 = svg.getChildren("ellipse"); + for (auto it=children3.begin(); it!=children3.end(); ++it){ + ids += it->getAttribute("id").getValue(); + } + ofxTestEq(ids,"", "\tthere is no ellipse direct children in svg"); + ofLogNotice()<<"\n"; + + // Testing search + ofLogNotice()<<"Searching for elements"; + el = svg.findFirst("g"); + ofxTestEq(el.getAttribute("id").getValue(), "group1", "\tThe first 'g' found should be 'group1'"); + ofLogNotice()<<"\n"; + } + + + ofLogNotice() << "Testing issue #6111"; + { + ofXml xml1, xml2; + + xml1.parse(""); + + std::locale saved_loc; + // Building a locale with comma as decimal point - inspired from https://stackoverflow.com/a/15221403 + class decimmal_comma: public std::numpunct { + protected: + char do_decimal_point() const { return ','; } + }; + std::locale decimal_comma_locale( std::locale("C"), new decimmal_comma() ); + std::locale decimal_point_locale( std::locale("C") ); + std::locale current_locale; + + ofLogNotice()<<" - using locale where ',' is the decimal separator"; + current_locale.global( decimal_comma_locale ); + + ofxTestEq(xml1.getFirstChild().getAttribute("float_period").getFloatValue(), 3.5, "\tnumbers with period are float"); + ofxTestEq(xml1.getFirstChild().getAttribute("float_comma").getFloatValue(), 2.0, "\tnumbers with comma are not float"); + + ofLogNotice()<<" - using locale where '.' is the decimal separator"; + current_locale.global( decimal_point_locale ); + xml2.parse(""); + ofxTestEq(xml1.getFirstChild().getAttribute("float_period").getFloatValue(), 3.5, "\tnumbers with period are float"); + ofxTestEq(xml1.getFirstChild().getAttribute("float_comma").getFloatValue(), 2.0, "\tnumbers with comma are not float"); + // set back locale + current_locale.global( saved_loc ); + } + ofLogNotice()<<"\n"; + } +}; + + +#include "ofAppNoWindow.h" +#include "ofAppRunner.h" +//======================================================================== +int main( ){ + ofInit(); + auto window = std::make_shared(); + auto app = std::make_shared(); + ofRunApp(window, app); + return ofRunMainLoop(); +}