[PATCH] D50154: [clangd] capitalize diagnostic messages

[David Blaikie via cfe-commits]

On Wed, Aug 8, 2018 at 5:00 AM Ilya Biryukov via Phabricator <
reviews at reviews.llvm.org> wrote:

> ilya-biryukov added a comment.
>
> In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote:
>
> > What's the motivation for clangd to differ from clang here?
>
>
> The presentation of diagnostics to the users is different between clang
> and clangd:
>
> - clang diagnostics are always prefixed with the file, location and
> severity of the diagnostic, e.g. `main.cpp:1:2: error: expected '}'`
>

*nod* That's a fairly natural one - and I assume the information is
provided semantically - source, line, column, and text. A client can stitch
them together as clang does or it can render the information some other way
(by placing the text as a mouseover/popup at the source location). I don't
see that as a divergence - but providing the full fidelity of the
information (rather than prematurely stringifying it all together) &
leaving it up to the client to decide how to render it to the user.


> - In LSP clients (VSCode in particular), the diagnostic message is the
> first thing the user sees and it looks a little nicer (subjectively) if the
> first letter is capitalized.
>

It looks nicer in VSCode because that's the UI convention for mouseover
text (in Windows, at least) - first letter capitalized. But it seems hard
to me to generalize from there to all or most LSP clients. (& I also worry
that such a mechanical transformation might be erroneous in some messages)

(well, there's at least one case where reversing this would fail - there's
a diagnostic in Clang that starts with "ISO" - so uppercasing does nothing,
but lowercasing it to try to get back to the original would result in "iSO"
- several others like "Objective-C", "Neon", "GCC", etc)

(also, there's one that this will mangle: "ms_struct may not produce
Microsoft-compatible layouts" - I assume capitalizing "ms_struct" would be
incorrect (though arguably/mostly identifiers like that are quoted in error
messages - and that may be reasonable/correct/better to do that here).
Similar use of 'sizeof' at the start of a diagnostic, 'os_log()', -
those'll be an issue no matter where the implementation is (in clangd or
its client(s)) though - short of having a more customized thing in the
diagnostics table itself to record which things can be capitalized/how they
should be capitalized - but, yeah, arguably quoting might be the right
thing in many, maybe all, of those cases)




>
> See the screenshots from VSCode on how diagnostics are presented:
> F6901986: image.png <https://reviews.llvm.org/F6901986> F6901990:
> image.png <https://reviews.llvm.org/F6901990>
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D50154
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180808/55218915/attachment.html>