-
Notifications
You must be signed in to change notification settings - Fork 101
Html indent #568
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
Html indent #568
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
| `Class of signature * ClassName.t | ||
| `ClassType of signature * ClassTypeName.t ] | ||
|
||
type leaf = [ module_ | type_ ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about classes, substs and others ?
(Would help move the assert false
from render_resolved_fragment
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this refers to the type of the substitution - ie, X with module Y=Z
or X with type t = u
-- so only module or type required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I got confused by the type t
below. Why does it include everything ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's the union one, helpful for printers and the like. it's quite handy to have just the leaf one so we don't have to deal with the Root constructor unnecessarily
let indent = | ||
let doc = "Format the output HTML files with indentation" in | ||
Arg.(value & flag (info ~doc [ "indent" ])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about enabling it by default without exposing the option ? Whitespaces compress well and indented output is always nicer to look at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could do - I've got no strong views on this. I've added it because my previous script was just running things through html tidy
, and the larger my test set got the slower the tidying got!
let sg = Type_of.signature env m in | ||
Module (signature env (id :> Id.Signature.t) sg) | ||
| Pack _ -> failwith "Unhandled content" | ||
| Pack p -> Pack p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How hard would it be to actually compile packs ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not even looked into it to be honest - not many packages use packing right now, but the one I have installed was causing my tests to fail!
d8e79c7
to
ce98388
Compare
As @Julow said, It should probably be enabled by default. The performance cost is probably negligible, as is the size cost after compression. |
Add a parameter to
html-generate
to format the output nicely - useful for diffingAlso this no longer causes exceptions when processing 'packed' compilation units.