compatibility with old roscpp using dxr/quick-xml fix #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PatWie
reviewed
Dec 2, 2024
| @@ -1011,7 +1011,7 @@ impl Handler for GetParamHandler { | |||
| let key_path = key_full.strip_prefix('/').unwrap_or(&key_full).split('/'); | |||
|
|
|||
| Ok(match params.get(key_path) { | |||
| Some(value) => (1, "".to_owned(), value.to_owned()), | |||
| Some(value) => (1, format!("Parameter [{}]", &key_full), value.to_owned()), | |||
There was a problem hiding this comment.
Can we for consistency keep the formatting string the same as below or adjust the line below?
PatWie
approved these changes
Dec 3, 2024
|
Just out of curiosity - what did you think about the simplified client API (i.e. removal of the |
|
How would that look like? |
|
I'm referring to changes like this one: (i.e. |
|
Oh, that makes a lot of sense. I wonder why I did separate it in the first place. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previously used version of
quick-xml(dependency ofdxr) sometimes emitted self-closing XML tags (like<this />), instead of the longer form (<this></this>). which is not supported by some versions of roscpp (causing a C++ exception). Whilequick-xmlhas an option calledexpand_empty_elements, whichdxruses (because apparently handling self-closing tags is a known issue in xmlrpc libraries), that option was not always respected.Dxr has now released v0.7, which comes with the fixed quick-xml version and some minor API changes.
Actual underlying fix:
https://github.com/tafia/quick-xml/blame/a5ad85e0020b62182a377338b887e8796ffb0acf/src/se/element.rs#L484
Issue thread on dxr's repo:
ironthree/dxr#23