search

Mesabloo / diagnose

A simple library for reporting compiler/interpreter errors
https://hackage.haskell.org/package/diagnose
BSD 3-Clause "New" or "Revised" License
258 stars 19 forks source link

`codespan-reporting` layout #14

Open ruifengx opened 2 years ago

ruifengx commented 2 years ago

Closes #11.

ruifengx commented 2 years ago
For completeness, here are two "screenshots" [^1] of the test suite: With Unicode Without Unicode
unicode ascii

[^1]: I used term-transcript-cli for the SVG screenshot. The command line is the following: cat {unicode,ascii}.txt | term-transcript capture "stack test # with[out] Unicode" -o {unicode,ascii}.svg --palette powershell --no-wrap.

Mesabloo commented 1 year ago

There's a lot to unpack here. Let's check bit by bit. I'll comment directly here and there (and maybe request some chnages) while reviewing the code.

Mesabloo commented 1 year ago

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. I personally find it easier to read if there is a bit of space here and there (see how spaces out my code is), but this may only be a matter of taste.

Would you be willing to maintain this new package? I'd be grateful. :D

ruifengx commented 1 year ago

The code is however quite "blocky", as in there are big chunks of code with no empty lines. I personally find it easier to read if there is a bit of space here and there (see how spaces out my code is), but this may only be a matter of taste.

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.

Would you be willing to maintain this new package? I'd be grateful. :D

My pleasure. Should any issue arises, ping me and I will try to address them ASAP.

Mesabloo commented 1 year ago

Hello @ruifengx, it's been quite a while. Do you have any update on this, any work left in commits not pushed?

ruifengx commented 1 year ago

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.

Mesabloo commented 1 year ago

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!