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
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
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
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
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
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
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.
14 matches
Mail list logo