hexonaut / haxe-dom

A cross-platform implementation of the DOM. Built to reduce duplicate code across server and client.
MIT License
47 stars 4 forks source link

added prettyPrint option to HtmlSerializer.hx #15

Closed axelhuizinga closed 10 years ago

axelhuizinga commented 10 years ago

applied the prettyPrint option to the test, added compile.hxml and haxe-dom.hxproj

hexonaut commented 10 years ago

Okay this is getting better, but there is still a lot of collateral changes being done. If you look at the stats there are almost 1000 changes when I'm guessing you've only actually changed about 20 lines or so tops. It makes it hard for me to see where the differences are. It looks like you are adjusting the indentation on HtmlSerializer and Main which is causing git to detect differences in every line. I can understand the indentation may be more to your preference, but usually you want to go with the project authors code and format style even if it goes against your own.

Also I prefer to keep IDE specific stuff out of the repo so please keep the haxe-dom.hxproj out of the commit. The compile.hxml is a good idea though.

Could you resubmit the pull request with just the changes? IE the additions/deletions should be less than 50 or so. Thanks. :)

axelhuizinga commented 10 years ago

Hi Sam, just tried with a different editor and the result is the same - I've only changed this

public static var prettyPrint:Bool = false; public static var iStr:String = '\t'; public static var eol:String = '\r\n'; var indent:Int; var actIndent:String;

in new: indent = 0;

and inside the following methods the prettyPrint related switches: function text (t:VirtualNode):Void { if (prettyPrint) buf.add("".lpad(iStr, indent) + iStr + t.node.data + eol ); else buf.add(t.node.data); }

 function element (e:VirtualNode<Element>):Void {
     openTag(e);
     indent++;
     children(e);
     indent--;
     closeTag(e);
 }

 function openTag (e:VirtualNode<Element>):Void {
     if(prettyPrint)
     buf.add("".lpad(iStr, indent) + "<" + 

e.node.tagName.toLowerCase()); else buf.add("<" + e.node.tagName.toLowerCase()); attrs(e); buf.add(prettyPrint && e.node.hasChildNodes() ? ">" + eol: ">"); }

 function closeTag (e:VirtualNode<Element>):Void {
     if(prettyPrint)
     buf.add((e.node.hasChildNodes()? "".lpad(iStr, indent):'') + 

"</" + e.node.tagName.toLowerCase() + ">" + eol); else buf.add("</" + e.node.tagName.toLowerCase() + ">"); }

 /**
  * Space delimited list of ids with the element's first followed by 

one or more for text nodes that are direct decendants. * The length is used to find node boundaries when unserializing on the client. */ inline function elemIds (e:VirtualNode):Void { buf.add(" data-hxid='" + Reflect.field(e, "id")); if (!Reflect.hasField(e.node, "__htmlSnippet")) { for (i in e.node.childNodes) { if (i.nodeType == Node.TEXT_NODE) { var text:Text = Reflect.field(i, "__vdom"); if (prettyPrint) { buf.add(" " + Reflect.field(text, "id") + "-" + ("".lpad(iStr, indent) + iStr + untyped i.data + eol).length); } else { buf.add(" " + Reflect.field(text, "id") + "-" + untyped i.data.length); } } } } buf.add("'"); }

but the whole HtmlSerializer code gets marked as changed. Any hints how to avoid this?

Am 09.01.2014 23:46, schrieb Sam:

Okay this is getting better, but there is still a lot of collateral changes being done. If you look at the stats there are almost 1000 changes when I'm guessing you've only actually changed about 20 lines or so tops. It makes it hard for me to see where the differences are. It looks like you are adjusting the indentation on HtmlSerializer and Main which is causing git to detect differences in every line. I can understand the indentation may be more to your preference, but usually you want to go with the project authors code and format style even if it goes against your own.

Also I prefer to keep IDE specific stuff out of the repo so please keep the haxe-dom.hxproj out of the commit. The compile.hxml is a good idea though.

Could you resubmit the pull request with just the changes? IE the additions/deletions should be less than 50 or so. Thanks. :)

— Reply to this email directly or view it on GitHub https://github.com/Blank101/haxe-dom/pull/15#issuecomment-31985366.

hexonaut commented 10 years ago

For some reason your editor is adjusting the amount of spacing for the tabs. I don't know which editors you are using so I can't really say how to fix this, but just be sure you are not performing some sort of automatic alignment.

On a slightly different note, it looks like there is a problem with the recording of the text node length. You are adding the formatting characters to the text which will result in different values for the client and server. IE if you do this:

var txt = new Text("Some Text");
trace(txt.data); // traces "Some Text" as expected

// ... After being loaded on the client ...
trace(txt.data); // traces "            Some Text" with leading tabs

Adding an offset in the parent element should fix this problem, but that requires modification of the JS Boot class.