improve the lsp diagnostic message format #2425
Conversation
* replace the line breaks with spaces
autoload/ale/lsp/response.vim
Outdated
| @@ -27,6 +27,7 @@ function! ale#lsp#response#ReadDiagnostics(response) abort | |||
|
|
|||
| for l:diagnostic in a:response.params.diagnostics | |||
| let l:severity = get(l:diagnostic, 'severity', 0) | |||
| let l:diagnostic.message = substitute(l:diagnostic.message, "\n", ' ', 'g') | |||
There was a problem hiding this comment.
Instead of modifying the incoming Dictionary, set the value with newlines in the detail key, and the value with newlines replaced in the text key. Add tests for this.
|
I believe the code actually works but I am still failing the test. Am I missing something here? |
autoload/ale/lsp/response.vim
Outdated
| " we want to preserve the line breaks for `detail` but replace with | ||
| " spaces for text | ||
| let l:diagnostic.detail = l:diagnostic.message | ||
| let l:diagnostic.text = substitute(l:diagnostic.message, "\n", ' ', 'g') |
There was a problem hiding this comment.
Don't modify the incoming object here, just adjust the objects we're creating.
There was a problem hiding this comment.
Do you mean something like:
let l:diagnostic.text = l:diagnostic.message
let l:diagnostic.text = substitute(l:diagnostic.text, "\n", ' ', 'g')
I don't think this passes the unit test. I am not very good at Vimscript so sorry about these problems.
There was a problem hiding this comment.
You can pass substitute(l:diagnostic.message, "\n", ' ', 'g') to the text key below without modifying the object, and use l:diagnostic.message below that as before.
|
I just convert this PR to use the simple approach as suggested but the test is still failing. Is there something wrong with the test I wrote? |
| \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ | ||
| \ { | ||
| \ 'range': Range(9, 14, 11, 22), | ||
| \ 'message': 'cannot borrow `cap` as mutable more than once at a time\n\nmutable borrow starts here in previous iteration of loop', |
There was a problem hiding this comment.
| \ 'message': 'cannot borrow `cap` as mutable more than once at a time\n\nmutable borrow starts here in previous iteration of loop', | |
| \ 'message': "cannot borrow `cap` as mutable more than once at a time\n\nmutable borrow starts here in previous iteration of loop", |
Here's the problem with your test. Escapes like \n are only applied for double quoted strings.
autoload/ale/lsp/response.vim
Outdated
| @@ -28,7 +28,7 @@ function! ale#lsp#response#ReadDiagnostics(response) abort | |||
| for l:diagnostic in a:response.params.diagnostics | |||
| let l:severity = get(l:diagnostic, 'severity', 0) | |||
| let l:loclist_item = { | |||
| \ 'text': l:diagnostic.message, | |||
| \ 'text': substitute(l:diagnostic.message, "\n", ' ', 'g'), | |||
There was a problem hiding this comment.
| \ 'text': substitute(l:diagnostic.message, "\n", ' ', 'g'), | |
| \ 'text': substitute(l:diagnostic.message, '\(\r\n\|\n\|\r\)', ' ', 'g'), |
I think you should handle Windows style CRLF newlines too. For example:
:echo substitute("one\ntwo\r\nthree\rfour", '\(\r\n\|\n\|\r\)', ' ', 'g')The string for the regex has to be single quoted here. Add tests for \r by itself and \r\n.
|
Cheers! 🍻 |
Here I am trying to improve the LSP diagnostic message format via replacing the line breaks with spaces (see #2301). This fix will affect the message in the status line, virtual text and the
ALEDetail.However, I don't think this is what we desire: now the
ALEDetailno longer has the line breaks. I assume the virtual text region is somehow binded with theALEDetail? Any input on it?Regards,