Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[model_io] Fix implicit dependency of URDF parsers on system locale #289

Merged
merged 1 commit into from
May 8, 2017

Conversation

traversaro
Copy link
Member

See #288

}

bool inline stringToInt(const std::string & inStr, int & outInt)
bool inline stringToIntWithClassicLocale(const std::string & inStr, int & outInt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a curiosity, why not using a templated function now that inside there are no differences between the toDouble and toInt and toUnsigned functions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think we can do it once we fixed the bug.

}

bool inline stringToInt(const std::string & inStr, int & outInt)
bool inline stringToIntWithClassicLocale(const std::string & inStr, int & outInt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason for the code duplication?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for historical reason the urdf_model/boost_replacements.h headers is only included in Windows, and these are private functions of the library. We can think of moving this functions to a public header, but I would prefer to to that in a separate pull request, as any additional public function then needs to be maintained, documented and modified with care to avoid breaking downstream code.

@traversaro traversaro merged commit e668a2f into master May 8, 2017
@traversaro traversaro deleted the fix/locale branch May 8, 2017 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants