It generates a standalone HTML file with all needed JS/CSS embedded.
This allows navigating the tree both with a tree widget and in the code,
inspecting nodes, and selecting ambiguous alternatives.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Super cool! Can't wait to use it.
The code looks good, just nits (feel free to ignore)
clang-tools-extra/pseudo/tool/CMakeLists.txt | ||
---|---|---|
29 | This likely introduces some pain. (An alternative is to inline the content of .js, .css, html in the cpp directly, but it is awkward). Would be nice to prepare an internal BUILD file change when landing (if you don't have time, I'm happy to help with it). | |
clang-tools-extra/pseudo/tool/HTMLForest.cpp | ||
65 | nit: it is clearer to use use llvm::formatv llvm::formatv(R"html( ... )html", js, css, forest_json, code, html);. | |
116 | I'd suggest renaming to Array and adding comment for the Index (it is the index in the Queue), Queue leaves an impression to me that we will do some pop_back. | |
171 | Having derived stream is fine to me at the moment, particularly useful for debugging. But as a regular user, I would prefer to see the original token stream (the derived stream is confusing, as a regular user doesn't know how pseudoparser work inside). | |
clang-tools-extra/pseudo/tool/HTMLForest.css | ||
51 | nit: just an idea we could use different background color for different forest node kind. | |
64 | nit: can we make the tree and code view resizable? so that I can adjust the width of tree/code view. | |
83 | nit: we could also add some style (possibly red indicating the error) for opaque node. | |
clang-tools-extra/pseudo/tool/HTMLForest.html | ||
2 | I think it might be better to move these .html, .css, .js files to a separate directory tool/resources rather than tool. | |
14 | nit: maybe name it i_context_stack, I had no idea what does the context mean until I read the actual implementation. |
Thanks for the review!
clang-tools-extra/pseudo/tool/CMakeLists.txt | ||
---|---|---|
29 |
Yeah, I think this is unreasonably difficult to edit, unfortunately - no editor understands JS/CSS/html inside string literals. (We can probably get away with the HTML, but the JS is a significant amount of code) (In fact had a second mode where the resources were read from disk without a recompile, which was faster while working on it, but I don't think it's necessary - inotifywait loop is enough)
Good point, I'll prepare that. | |
clang-tools-extra/pseudo/tool/HTMLForest.cpp | ||
65 | I guess, but:
Added a small tag(name, block) function instead which I think makes the structure easier to follow (basically the clang-formatted code shows the HTML structure) | |
116 | I agree about Queue. | |
171 | Yeah, I think we'll want this later changed to FIXME. | |
clang-tools-extra/pseudo/tool/HTMLForest.css | ||
51 | Done, and made the colors of the nodes in the tree match. | |
64 | resize: horizontal seems to do the trick | |
clang-tools-extra/pseudo/tool/HTMLForest.html | ||
2 | the tool/ directory is ~empty or I would... happy to add some more structure if it gets at all crowded | |
14 | Renamed to i_ancestors which seems explicit without being long |
This likely introduces some pain. (An alternative is to inline the content of .js, .css, html in the cpp directly, but it is awkward).
Would be nice to prepare an internal BUILD file change when landing (if you don't have time, I'm happy to help with it).