-
Notifications
You must be signed in to change notification settings - Fork 35
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
Using (the more generic) XmlNode in place of XmlTree #19
base: master
Are you sure you want to change the base?
Conversation
At a first view it seems, this is a rather large change for getting rid of the |
Then what would you do about those convenient combinators? addNav
//> withoutNav (hasName "table")
/> withoutNav (hasName "tr")
>>> make-some-replacement-here
>>> ... |
you're right, if that |
Yes, without the generalization, the What would you like to see before accepting the pull? As mentioned in the OP, I'll be happy to fill-out the generalization, starting with |
+1. Having to use |
Consider the situation of walking a NavXmlTree and filtering on nodes having certain properties (say, in their names). The current type signatures require sprinkling
withoutNav
throughout the code:By changing the type signature of
hasName
fromString -> a XmlTree XmlTree
toXmlNode xn => String -> a xn xn
, we avoid the need for thewithoutNav
call. This patch generalizeshasName
as well as many others.Most of the changes to
Text.XML.HXT.Arrow.XmlArrow
are pretty simple. The generalization ofchangeAttrValue
was more complex. To get it to work on an arrow ofXmlNode
s, I needed to make xshow accept such an arrow, which in turn required generalizingText.XML.HXT.DOM.ShowXml.xshow
. I chose to go about this by creating aToXmlTree
class for use in place of XmlTree (when conversion from anXmlNavTree
is desired).This is admittedly a bit too specific: why not a
ToNTree
class instead? Simply because it would have required adding aToXNode
class and I was tired of adding classes. If this patch is well-received, I'll have a look at further generalizations.