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

Support visitor pattern for tree mutation and multiple files #6

Closed
Nanonid opened this issue Sep 7, 2014 · 10 comments
Closed

Support visitor pattern for tree mutation and multiple files #6

Nanonid opened this issue Sep 7, 2014 · 10 comments

Comments

@Nanonid
Copy link

Nanonid commented Sep 7, 2014

Bountysource

I have an XSD that imports another XSD.

<xs:import namespace="http://www.opengis.net/gml" schemaLocation="gml.xsd"/>

As package:xml exists, attributes, children etc are final and not mutable.

XmlBranch(Iterable<XmlNode> children): children = children.toList(growable: false) 

dart:html DOM is mutable, which is useful for tree transformation.

Ideally, the library could handle an import statement by one or more:

  1. natively, somehow support multi-file parsing within the library
  2. parse action, exposing a standard means of action augmentation in the base CompositeParser to redefine or add a post action to refine behavior.
  3. tree mutation so the xs:import node could have a child XmlDocument, etc.
@Nanonid
Copy link
Author

Nanonid commented Sep 8, 2014

I'm considering forking xml_dart to:

  1. Use/extend dart:html DOM nodes in place of local models
  2. Add factory and mutation support for tree building
  3. Add simple query and visitor support. I already have a xml_visitor project which I'm using, something based on that perhaps.

@Nanonid
Copy link
Author

Nanonid commented Sep 8, 2014

Hah! Some reason I've not perused XmlBuilder and just assumed that the tree was not build-able because XmlElement attributes are not mutable.
Can't figure out the reason for why I didn't read all the code. My fault.
Bounty still stands for tree mutation, actions, and whatever you think might make dart-xml better.

@renggli
Copy link
Owner

renggli commented Sep 8, 2014

I've just committed 61ba675 that adds a simple node visitor to the library. The change cleans up other parts of the code as well, for example the (pretty) printing is now a visitor and separate to the node classes.

The code also includes a transformation visitor that copies a tree and can be subclassed to perform custom transformations. I believe this allows a rather simple implementation of resolving imports.

The currently committed code is work in progress. Let me know what you think.

@renggli
Copy link
Owner

renggli commented Sep 8, 2014

04d2629 makes the XmlParser event based, so that subclasses can transform the input while the source file is parsed.

Again, the code is just experimental and work in progress. Let me know what you think.

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

Okay. will do. I just made an xml_visitor so I have something to compare with. Thanks.

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

Looks like in order to mutate a tree, I need to visit and copy. Very FP, but my visitors may be annotating, like adding new namespace elements and state. XmlElement ensures immutable. :-(
Still, I like the new code, and I'll see how far I can get before editing origin.

  @override
  final List<XmlAttribute> attributes;

  /**
   * Create an [XmlElement] with the given `name`, `attributes`, and `children`.
   */
  XmlElement(XmlName name, Iterable<XmlAttribute> attributes, Iterable<XmlNode> children)
      : super(children),
        name = name,
        attributes = attributes.toList(growable: false) {

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

Element 'XmlDocument' from SDK library 'html_dartium.dart' is implicitly hidden by 'xml.dart'
Which means nuisance as xml
I can just make more libraries though. NP

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

Argh! The embedded SVG and dataURI are killing me...
Diagram: 2420642 diagram_export_1.xml

void main(){
  group( "diagram", (){
    test( "parse visit", (){
      String xml = readFileTextSync("diagram_export_1.xml");
      Stopwatch stopwatch = new Stopwatch()..start();
      XmlDocument doc = parse(xml);
      print( "parse time ${stopwatch.elapsed}");
      stopwatch.reset();
      DiaTransformer xform = new DiaTransformer();
      DiaDoc dia = xform.visitDocument(doc);
      print( "visit ${stopwatch.elapsed}");
    });
  });
}
unittest-suite-wait-for-done
parse time 0:00:20.558550
visit 0:00:00.010319
PASS: diagram parse visit

All 1 tests passed.
unittest-suite-success

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

Seems to work OK. Transformer has many name specific specialization.
Difficult to accumulate state across children when everything is passed at constructor.
Can't pass down this if children need parent state for lookup.
Graphs don't work well.

  visitIcon( XmlElement el ) => new Icon(el);
  visitIcons( Iterable<XmlElement> icons ) => icons.map(visitIcon);
  visitDia( XmlElement el ) => new Dia(el, visitIcons(el.findElements("Icon")) );

Have Transformer state variable passed in to constructor?

  DocState state;
  visitIcon( XmlElement el ) => new Icon(el,state);
  visitIcons( Iterable<XmlElement> icons ) => icons.map(visitIcon);
  visitDia( XmlElement el ) => new Dia(el, visitIcons(el.findElements("Icon")), state );

@Nanonid
Copy link
Author

Nanonid commented Sep 9, 2014

I'll close this and just create a new issue if need be.
Thanks! 👍

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

No branches or pull requests

2 participants