On Tue, Aug 7, 2018 at 11:22 AM Alex L <arpha...@gmail.com> wrote: > On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> >> On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator < >> revi...@reviews.llvm.org> wrote: >> >>> arphaman added a comment. >>> >>> In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote: >>> >>> > What's the motivation for clangd to differ from clang here? (& if the >>> first >>> > letter is going to be capitalized, should there be a period at the >>> end? But >>> > also the phrasing of most/all diagnostic text isn't in the form of >>> complete >>> > sentences, so this might not make sense) >>> >>> >>> 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. >>> >> >> 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)) >> >> My 2c - I've no vested interest or authority here. >> > > 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. >
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. > 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: > > > https://github.com/llvm-mirror/clang-tools-extra/blob/55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161 > 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? 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. > > > > > >> >> >>> 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 >>> >>> >>> Repository: >>> rCTE Clang Tools Extra >>> >>> https://reviews.llvm.org/D50154 >>> >>> >>> >>> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
