On Wed, Aug 8, 2018 at 5:00 AM Ilya Biryukov via Phabricator <
revi...@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
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,
On 7 August 2018 at 17:40, Fangrui Song wrote:
>
> On 2018-08-07, David Blaikie wrote:
>
>>
>> On Tue, Aug 7, 2018 at 4:02 PM Alex L wrote:
>>
>>On Tue, 7 Aug 2018 at 11:38, David Blaikie wrote:
>>
>>On Tue, Aug 7, 2018 at 11:22 AM Alex L wrote:
>>
>>On Tue, 7 Aug 2018
On 2018-08-07, David Blaikie wrote:
On Tue, Aug 7, 2018 at 4:02 PM Alex L wrote:
On Tue, 7 Aug 2018 at 11:38, David Blaikie wrote:
On Tue, Aug 7, 2018 at 11:22 AM Alex L wrote:
On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
cfe-commits@lists.l
On Tue, Aug 7, 2018 at 4:02 PM Alex L wrote:
> On Tue, 7 Aug 2018 at 11:38, David Blaikie wrote:
>
>>
>>
>> On Tue, Aug 7, 2018 at 11:22 AM Alex L wrote:
>>
>>> On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
On Tue, Aug 7,
On Tue, 7 Aug 2018 at 11:38, David Blaikie wrote:
>
>
> On Tue, Aug 7, 2018 at 11:22 AM Alex L 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...@r
It might be useful to know what other editors do here to know whether
this should be a client transformation or baked into the server.
Vim syntastic/ale, Emacs flycheck do not seem to do such transformation. What's
the editor you are using?
On 2018-08-07, Alex L wrote:
On Tue, 7 Aug 2018 at 10
On Tue, Aug 7, 2018 at 11:22 AM Alex L 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.
>>>
>>> I
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:
>
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
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'
dblaikie added subscribers: ilya-biryukov, dblaikie.
dblaikie added a comment.
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
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)
On Fri, Aug 3, 2018 at 1:44 P
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE338919: [clangd] capitalize diagnostic messages (authored
by arphaman, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50154?vs=159059&id=159080#toc
Repository:
rCTE Clang Tools
hokein added a comment.
nit: The patch description needs to be updated.
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/Diagnostics.cpp:149
+/// Capitalizes the first word in the diagnostic's message.
+std::string capitalizeDiagnosticText(std::string Message) {
+ if (!
arphaman added inline comments.
Comment at: test/clangd/capitalize-diagnostics.test:1
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
---
arphaman updated this revision to Diff 159059.
arphaman marked an inline comment as done.
arphaman added a comment.
Remove test and update unit test.
https://reviews.llvm.org/D50154
Files:
clangd/Diagnostics.cpp
test/clangd/diagnostics.test
test/clangd/did-change-configuration-params.test
ioeric added inline comments.
Comment at: test/clangd/capitalize-diagnostics.test:1
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-
arphaman updated this revision to Diff 159036.
arphaman added a comment.
Always capitalize the diagnostic's message.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
Files:
clangd/Diagnostics.cpp
test/clangd/capitalize-diagnostics.test
test/clangd/diagnostics.test
t
ilya-biryukov added a comment.
Seems we have consensus here.
Let's remove the option and just always capitalize the messages.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
simark added a comment.
In https://reviews.llvm.org/D50154#1185605, @sammccall wrote:
> Capitalizing sounds nice. +1 to just do it without an option.
>
> My favorite essay on this is
> http://neugierig.org/software/blog/2018/07/options.html
Agreed.
Repository:
rCTE Clang Tools Extra
https
sammccall added a comment.
Capitalizing sounds nice. +1 to just do it without an option.
My favorite essay on this is
http://neugierig.org/software/blog/2018/07/options.html
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
___
simark added a comment.
That's fine with me. I'll try it and time will tell if I like it or not :).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
ioeric added a comment.
In https://reviews.llvm.org/D50154#1185545, @ilya-biryukov wrote:
> Maybe we could simply always capitalize the messages? Any cons to
> capitalizing error messages?
> @simark, @malaperle, @ioeric, @hokein, WDYT?
Oh sorry, I thought `-capitialize-diagnostic-text` was an
ilya-biryukov added subscribers: simark, malaperle.
ilya-biryukov added a comment.
Maybe we could simply always capitalize the messages? Any cons to capitalizing
error messages?
@simark, @malaperle, @ioeric, @hokein, WDYT?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
ioeric added a comment.
Is it possible for clangd to always capitalize diagnostics if
`-capitialize-diagnostic-text` is found in the compile comand?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
arphaman created this revision.
arphaman added reviewers: ilya-biryukov, ioeric, hokein, jkorous.
Herald added subscribers: dexonsmith, MaskRay.
This patch adds support for diagnostic message capitalization to Clangd. This
is enabled when '-capitialize-diagnostic-text' is provided to Clangd.
R
28 matches
Mail list logo
