implement error/warning squiggles in stan editor #100
Conversation
|
I think it makes sense to do some munging on the error messages so they look nicer on the hover-over, e.g. removing the 'preview' stanc gives you. See: https://github.com/WardBrian/vscode-stan-extension/blob/main/src/linter.ts#L38 Additionally, with this implemented, do we still see value in the "pop-up" error window? |
Not sure about that. But can we keep it in there for the time being? Not ready to say goodbye :) |
|
Sure, no need to delete it in the same PR -- just speculating for the future! |
I was about to use that code, but saw that it's GPL. Do I have your permission? |
|
Ah, yes. The project I forked from is GPL, but I am the sole author of that file, and I happily grant permission. |
|
Okay, the messages are now reformatted and they look nice! Another comment about the syntax window... I think it's nice to not just rely on seeing a slight yellow squiggle... so even if we do remove that window, we'll want to be able to display something prominently that there are issues (and at that point, I'd like to click on that notification and get the details) |
| } | ||
|
|
||
| if ((lineNumber !== undefined) && (startColumn !== undefined) && (endColumn !== undefined)) { | ||
| const isWarning = x.toLowerCase().startsWith('warning') |
There was a problem hiding this comment.
Thoughts on having a loop over warnings and a loop over errors to avoid needing to back this information out again? I think if the above logic to find the start/end was its own function the two loops wouldn't need to share too much duplicate code
| const toSeverity = (s: 'error' | 'warning' | 'hint' | 'info'): monaco.MarkerSeverity => { | ||
| switch (s) { | ||
| case 'error': return monaco.MarkerSeverity.Error | ||
| case 'warning': return monaco.MarkerSeverity.Warning | ||
| case 'hint': return monaco.MarkerSeverity.Hint | ||
| case 'info': return monaco.MarkerSeverity.Info | ||
| } | ||
| } |
There was a problem hiding this comment.
Mind moving this helper function out of the hook?
Fixes #76