[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Eric Liu via Phabricator via 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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Alex Lorenz via Phabricator via cfe-commits
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"}}

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Alex Lorenz via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
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"}}

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Alex Lorenz via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Simon Marchi via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Sam McCall via Phabricator via cfe-commits
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 ___

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Simon Marchi via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-01 Thread Alex Lorenz via Phabricator via cfe-commits
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.