codespan-reporting layout
#14
Conversation
- '_' instead of '-' for top/bottom rules - get rid of ‘/’ and '\' at the corners (IMO they look ugly)
|
For completeness, here are two "screenshots" 1 of the test suite:
Footnotes
|
|
There's a lot to unpack here. |
|
|
||
| This is an implementation of a custom layout for Diagnose, heavily inspired by [`codespan-reporting`](https://github.com/brendanzab/codespan). | ||
|
|
||
| TODO: add example rendering |
There was a problem hiding this comment.
I see this has not yet been replaced.
Should I add examples myself (for consistency with other screenshots)?
There was a problem hiding this comment.
Ah, I totally forgot about this. If you could point me to the code responsible for generating the example, I can add the screenshots here. By the way, I think SVG screenshots (as shown in my previous comments in this PR) might have better display quality (and also smaller in size); I can replace other screenshots to use SVG format if you want (I already have the tool installed so it might be more convenient to do it on my side).
|
Overall, this should be almost ready to merge. The code is however quite "blocky", as in there are big chunks of code with no empty lines. Would you be willing to maintain this new package? I'd be grateful. :D |
I agree. Ideally I would break those long functions into smaller ones, with descriptive names and only a few local bindings, but passing around the configurations (like "width of the line number section" etc.) would also clutter the code, so I left it as is. I will have a look at your implementation and add more spacing (or even better, more comments). This shouldn't take long.
My pleasure. Should any issue arises, ping me and I will try to address them ASAP. |
|
Hello @ruifengx, it's been quite a while. |
|
Sorry about this. I should have at least left a note here. A quick summary of the current situation: I have some code clean up for the implementation (because I am not entirely satisfied about the current mess), but they are not yet committed/pushed. The second half of this semester was unexpectedly busy for me, so I did not have enough time to finish the rewrite, despite that this PR did occur to me quite a few times. I apologise for keeping you waiting, and I do hope to bring a good ending for this PR. I will try to pick up from where I left with my local edits and push an update here soon. |
|
Hey no worries! I'm also quite busy (hence the fact that I'm popping a message here only now) so it'll take a bit of time to continue reviewing the code anyway. Keep up the good work! |
Closes #11.