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

Added new functions: replace and poke for inner xml #970

Merged
merged 2 commits into from
Oct 8, 2015

Conversation

jsauvexamarin
Copy link
Contributor

Added new functions:

  • XPathReplaceInnerText
  • XmlPokeInnerText
  • XPathReplaceInnerTextNS
  • XmlPokeInnerTextNS

The existing XPathReplace and XmlPoke functions are incapable of modifying the Value property of XmlNode. This is due to both Mono and .NET throwing an InvalidOperationException in the body of the XmlNode.Value property setter:

Mono:
https://github.com/mosa/Mono-Class-Libraries/blob/master/mcs/class/System.XML/System.Xml/XmlNode.cs#L309

.NET:
https://github.com/Microsoft/referencesource/blob/master/System.Xml/System/Xml/Dom/XmlNode.cs#L92

I'm seeing that InvalidOperationException in practice in one of my FAKE build scripts. I know the XPath expression I'm using is valid; if I change it to be invalid, I get the "node not found" message. What I'm saying is: I'm definitely using XmlPoke properly, and it's throwing a new InvalidOperationException ("This node does not have a value") for any string value that I feed it.

So, I've added some functions that allow updating the InnerText value of a node; something for which I have a need in my FAKE build scripts. InnerText IS a mutable property, unlike Value.

Suggestion: Consider removing XPathReplace, XPathReplaceNS, XmlPoke, and XmlPokeNS. I'm not sure what purpose they serve, because they don't seem to work. XmlNode.Value is unsettable in both .NET and Mono.

Just to provide a concrete example of where I'm using this in a custom Android resources file:

<resources>
    <string name="GoogleMapsKey"></string>
</resources>

I want to update the value between the opening and closing string tags; the 'inner text' of the string node.

    XPathReplaceInnerText
    XmlPokeInnerText
    XPathReplaceInnerTextNS
    XmlPokeInnerTextNS

The existing XPathReplace and XPathPoke functions are incapable of modifying the Value property of XmlNode. This is due to both Mono and .NET throwing an InvalidOperationException in the body of the XmlNode.Value property setter:

Mono: https://github.com/mosa/Mono-Class-Libraries/blob/master/mcs/class/System.XML/System.Xml/XmlNode.cs#L309
.NET: https://github.com/Microsoft/referencesource/blob/master/System.Xml/System/Xml/Dom/XmlNode.cs#L92

I'm getting seeing that InvalidOperationException in practice in one of my FAKE build scripts. I know the XPath expression I'm using is valid; if I change it to be invalid, I get the "node not found" message.

So, I've added some functions that allow updating the InnerText value of a node; something for which I have a need in my FAKE build scripts. InnerText IS a mutable property, unlike Value.

Suggestion: Consider removing XPathReplace, XPathReplaceNS, XmlPoke, and XmlPokeNS. I'm not sure what purpose they serve, because they don't seem to work. XmlNode.Value is unsettable in both .NET and Mono.
@jsauvexamarin
Copy link
Contributor Author

Never mind. I just wrote a test for XmlPokeInnerText and it's failing. I'll get the tests working and add a commit.

@jsauvexamarin
Copy link
Contributor Author

Okay! Added passing tests! XmlPokeInnerText and XmlPokeInnerTextNS should prove useful for anyone wanting to modify xml node inner text values in-place on a filesystem. Please merge this! Thanks!

@jsauvexamarin
Copy link
Contributor Author

Crap. The AppVeyor build failed on the when_poking_xml_innertext test. That's odd, because it's passing locally. Maybe an encoding issue? Investigating.

@jsauvexamarin
Copy link
Contributor Author

Another note on the existing XmlPoke and XmlPokeNS (as well as their backing "Replace" functions): maybe they should be renamed to XmlAttributePoke et al, because that seems to be the only thing they actually do...modify attribute values. Just a friendly suggestion! :)

@forki
Copy link
Member

forki commented Oct 8, 2015

thx a lot

@jsauvexamarin
Copy link
Contributor Author

My pleasure! Thx for maintaining this great project!

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.

2 participants