Discussion:
[osg-users] osgDB XmlParser and localization
Trajce Nikolov NICK
2018-07-09 10:40:54 UTC
Permalink
Hi Robert,

I am working with some localized XML files (with chars out of 0-255 range)
and at present the XML Node parsing is not suitable to manage it. And I
think the support for is important - at my case it is OpenStreetMap with
street names containing these chars.

Attached is the modified source (based on the master) that fixes ity, if
you have time and will to review. I can do PR if you want

Thanks and cheers!

Nick

p.s. welcome back
--
trajce nikolov nick
Robert Osfield
2018-07-09 11:09:11 UTC
Permalink
Hi Nick,

I had a quick look at your changes and it seems to be like most of
them are changes for changes sake rather than likely to make any
functional difference. For non ascii char support I think what you'd
actually want to do is change the XmllNode::Input::string _buffer
member var to a std::vector<int> _buffer, and have the
Input::readAllDataIntoBuffer() method be adapted to read the file
accepting only chars greater than 0 rather than limiting to the ascii
0 to 255 range.

Robert.
On Mon, 9 Jul 2018 at 11:41, Trajce Nikolov NICK
Post by Trajce Nikolov NICK
Hi Robert,
I am working with some localized XML files (with chars out of 0-255 range) and at present the XML Node parsing is not suitable to manage it. And I think the support for is important - at my case it is OpenStreetMap with street names containing these chars.
Attached is the modified source (based on the master) that fixes ity, if you have time and will to review. I can do PR if you want
Thanks and cheers!
Nick
p.s. welcome back
--
trajce nikolov nick
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Trajce Nikolov NICK
2018-07-09 11:26:23 UTC
Permalink
hi Robert,
.... are changes for changes sake rather than likely to make any
functional difference

I knew you will have comments ;-). It works just fine with these changes
but yes, you are right - quick and somewhat dirty. So can you look at it or
you want to do a ping-pong code review with my changes by your advice? :-)

Please let me know

Nick
Hi Nick,
I had a quick look at your changes and it seems to be like most of
them are changes for changes sake rather than likely to make any
functional difference. For non ascii char support I think what you'd
actually want to do is change the XmllNode::Input::string _buffer
member var to a std::vector<int> _buffer, and have the
Input::readAllDataIntoBuffer() method be adapted to read the file
accepting only chars greater than 0 rather than limiting to the ascii
0 to 255 range.
Robert.
On Mon, 9 Jul 2018 at 11:41, Trajce Nikolov NICK
Post by Trajce Nikolov NICK
Hi Robert,
I am working with some localized XML files (with chars out of 0-255
range) and at present the XML Node parsing is not suitable to manage it.
And I think the support for is important - at my case it is OpenStreetMap
with street names containing these chars.
Post by Trajce Nikolov NICK
Attached is the modified source (based on the master) that fixes ity, if
you have time and will to review. I can do PR if you want
Post by Trajce Nikolov NICK
Thanks and cheers!
Nick
p.s. welcome back
--
trajce nikolov nick
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
--
trajce nikolov nick
Robert Osfield
2018-07-09 11:49:50 UTC
Permalink
Hi Nick,

On Mon, 9 Jul 2018 at 12:26, Trajce Nikolov NICK
Post by Robert Osfield
.... are changes for changes sake rather than likely to make any
functional difference
I knew you will have comments ;-). It works just fine with these changes but yes, you are right - quick and somewhat dirty. So can you look at it or you want to do a ping-pong code review with my changes by your advice? :-)
I might be quicker just to provide a link to or attach an xml file
that causes problem with the present parser, and then I can look at
what happens when the present code encounters it, and how your changes
affect things.

As a general note, PRs/commits should be focused on addressing one
issue at a time, so bundling a fix for a specific bug along with other
cosmetic or other changes just confuses what is functional and what is
not. It's not uncommon to have to go back to various revisions in
code to see where regressions may have occurred so having commits that
just wrap up a small set of associated changes is really helpful in
identifying what has been changed and why.

Robert.
Trajce Nikolov NICK
2018-07-09 11:58:58 UTC
Permalink
Hi again Robert,

fast hint: The nodes to be parsed are root->osm->node->tag and their
properties contains these non asci codes. But probably fastest is to write
recursive parser

Thanks again

On Mon, Jul 9, 2018 at 1:54 PM Trajce Nikolov NICK <
Thanks so much Robert !!! Attached is the file
Post by Robert Osfield
Hi Nick,
On Mon, 9 Jul 2018 at 12:26, Trajce Nikolov NICK
Post by Robert Osfield
.... are changes for changes sake rather than likely to make any
functional difference
I knew you will have comments ;-). It works just fine with these
changes but yes, you are right - quick and somewhat dirty. So can you look
at it or you want to do a ping-pong code review with my changes by your
advice? :-)
I might be quicker just to provide a link to or attach an xml file
that causes problem with the present parser, and then I can look at
what happens when the present code encounters it, and how your changes
affect things.
As a general note, PRs/commits should be focused on addressing one
issue at a time, so bundling a fix for a specific bug along with other
cosmetic or other changes just confuses what is functional and what is
not. It's not uncommon to have to go back to various revisions in
code to see where regressions may have occurred so having commits that
just wrap up a small set of associated changes is really helpful in
identifying what has been changed and why.
Robert.
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
--
trajce nikolov nick
--
trajce nikolov nick
Trajce Nikolov NICK
2018-08-31 15:11:13 UTC
Permalink
Hi Robert,

can you fix this too when you get back to OSG dev please?

Thank you a bunch
Nick

On Mon, Jul 9, 2018 at 1:58 PM Trajce Nikolov NICK <
Post by Trajce Nikolov NICK
Hi again Robert,
fast hint: The nodes to be parsed are root->osm->node->tag and their
properties contains these non asci codes. But probably fastest is to write
recursive parser
Thanks again
On Mon, Jul 9, 2018 at 1:54 PM Trajce Nikolov NICK <
Thanks so much Robert !!! Attached is the file
Post by Robert Osfield
Hi Nick,
On Mon, 9 Jul 2018 at 12:26, Trajce Nikolov NICK
Post by Robert Osfield
.... are changes for changes sake rather than likely to make any
functional difference
I knew you will have comments ;-). It works just fine with these
changes but yes, you are right - quick and somewhat dirty. So can you look
at it or you want to do a ping-pong code review with my changes by your
advice? :-)
I might be quicker just to provide a link to or attach an xml file
that causes problem with the present parser, and then I can look at
what happens when the present code encounters it, and how your changes
affect things.
As a general note, PRs/commits should be focused on addressing one
issue at a time, so bundling a fix for a specific bug along with other
cosmetic or other changes just confuses what is functional and what is
not. It's not uncommon to have to go back to various revisions in
code to see where regressions may have occurred so having commits that
just wrap up a small set of associated changes is really helpful in
identifying what has been changed and why.
Robert.
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
--
trajce nikolov nick
--
trajce nikolov nick
--
trajce nikolov nick
Robert Osfield
2018-09-04 14:40:27 UTC
Permalink
On Fri, 31 Aug 2018 at 08:06, Trajce Nikolov NICK
Post by Trajce Nikolov NICK
can you fix this too when you get back to OSG dev please?
This morning I have checked in UTF8 handling in XmlNode/Input to
master and the 3.6 branch.

https://github.com/openscenegraph/OpenSceneGraph/commit/afe5644b9fcf39a22a2ba0c8aefb26a340a5a457

Robert.
Trajce Nikolov NICK
2018-09-04 21:11:18 UTC
Permalink
Thanks a bunch Robert!

I will check it first thing tomorrow morning
Post by Robert Osfield
On Fri, 31 Aug 2018 at 08:06, Trajce Nikolov NICK
Post by Trajce Nikolov NICK
can you fix this too when you get back to OSG dev please?
This morning I have checked in UTF8 handling in XmlNode/Input to
master and the 3.6 branch.
https://github.com/openscenegraph/OpenSceneGraph/commit/afe5644b9fcf39a22a2ba0c8aefb26a340a5a457
Robert.
_______________________________________________
osg-users mailing list
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
--
trajce nikolov nick
Loading...