<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, 7 Aug 2018 at 11:38, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 7, 2018 at 11:22 AM Alex L <<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">arphaman added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D50154#1191002" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50154#1191002</a>, @dblaikie wrote:<br>
<br>
> What's the motivation for clangd to differ from clang here? (& if the first<br>
> letter is going to be capitalized, should there be a period at the end? But<br>
> also the phrasing of most/all diagnostic text isn't in the form of complete<br>
> sentences, so this might not make sense)<br>
<br>
<br>
It's mostly for the presentation purposes, to match the needs of our client. I first implemented it as an opt-in feature, but the consensus was to capitalize the messages all the time. <br></blockquote><div><br>Doesn't seem like it'd be any more expensive (amount of code or performance) to do that up in your client code, then, would it? I guess if most users of this API in time ended up preferring capitalized values, it'd make sense to share that implementation - but to me it seems like a strange transformation to happen at this level. (since it depends on what kind of client/how they want to render things - so it seems an odd choice to bake in to the API (or even provide an option for, unless there are lots of users/the code was especially complicated))<br><br>My 2c - I've no vested interest or authority here.<br></div></div></div></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>I think it's more in spirit with Clangd to provide output that's as close to the one presented by the client as possible. </div></div></div></blockquote><div><br>That assumes there's one client though, right? Different clients might reasonably have different needs for how they'd want the text rendered, I'd imagine.<br></div></div></div></blockquote><div><br></div><div>True. This transformation is lossless though, so the clients will still be able to get back the original text if needed. And if the consensus about this particular text transformation changes I'm willing to revert this change for sure.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>I would argue there's already a precedence for this kind of transformations, for example, Clangd merges the diagnostic messages of notes and the main diagnostics into one, to make it a better presentation experience in the client:</div><div><br></div><div><a href="https://github.com/llvm-mirror/clang-tools-extra/blob/55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161" target="_blank">https://github.com/llvm-mirror/clang-tools-extra/blob/55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161</a></div></div></div></blockquote><div><br>I'm assuming that's because the API/protocol only supports a single message, so the multi-message/location nuance of LLVM's diagnostics are necessarily lost when converting to that format?<br><br>If that's not the case, and the API/protocol does support a multiple-message diagnostic & ClangD is collapsing them early - that also seems like an unfortunate loss in fidelity.<br></div></div></div></blockquote><div><br></div><div>Clangd sends out both the main diagnostic, and the attached notes (that don't have fixits) in a list (i.e. the hierarchy isn't directly preserved, although it can be recreated by the client).</div><div>So it looks like they're collapsed early, but at the same time the client has enough information to do this transformation itself if desired.</div><div>I'm planning to work on an option to remove this behavior if desired by the client.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
I don't think it would make sense to insert the period at the end, because, as you said, not all diagnostics are complete sentences<br>
<br>
<br>
Repository:<br>
rCTE Clang Tools Extra<br>
<br>
<a href="https://reviews.llvm.org/D50154" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50154</a><br>
<br>
<br>
<br>
</blockquote></div></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></blockquote></div></div>
</blockquote></div></div>