[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 125267. rwols added a comment. Herald added a subscriber: klimek. - Merge with upstream revision 319613 - Fix FixItHints not getting applied correctly in all cases. The problem is that clang::CharSourceRange can either be a TokenRange or a CharRange. In the ca

[PATCH] D39430: [clangd] formatting: don't ignore style

sammccall added a comment. @ilya-biryukov We should get this landed. I'm happy (my comments were addressed a few rounds ago) - any concerns? https://reviews.llvm.org/D39430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols added inline comments. Comment at: clangd/ClangdUnit.cpp:168 - P.line = Location.getSpellingLineNumber() - 1; - P.character = Location.getSpellingColumnNumber(); Range R = {P, P}; Why subtract 1 from the line here, but not from the column? When subtrac

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 124307. rwols added a comment. - Rebase to latest upstream revision. - Go all-in with TextEdit, even down to ClangdUnit.cpp. - Move FixItsMap to ClangdServer. ClangdLSPServer is much cleaner now. - Remove the cached FixIts when the client closes the document. -

[PATCH] D39430: [clangd] formatting: don't ignore style

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:289 + llvm::Expected> + formatRange(llvm::StringRef Code, PathRef File, Range Rng); + rwols wrote: > rwols wrote: > > ilya-biryukov wrote: > > > rwols wrote: > > > > ilya-biryukov wrote: > >

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols added inline comments. Comment at: clangd/ClangdServer.h:289 + llvm::Expected> + formatRange(llvm::StringRef Code, PathRef File, Range Rng); + rwols wrote: > ilya-biryukov wrote: > > rwols wrote: > > > ilya-biryukov wrote: > > > > Why do we accept `Code`

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols added inline comments. Comment at: clangd/ClangdServer.h:289 + llvm::Expected> + formatRange(llvm::StringRef Code, PathRef File, Range Rng); + ilya-biryukov wrote: > rwols wrote: > > ilya-biryukov wrote: > > > Why do we accept `Code` as a parameter here i

[PATCH] D39430: [clangd] formatting: don't ignore style

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.h:289 + llvm::Expected> + formatRange(llvm::StringRef Code, PathRef File, Range Rng); + rwols wrote: > ilya-biryukov wrote: > > Why do we accept `Code` as a parameter here instead of getting i

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols marked 2 inline comments as done. rwols added inline comments. Comment at: clangd/ClangdServer.h:289 + llvm::Expected> + formatRange(llvm::StringRef Code, PathRef File, Range Rng); + ilya-biryukov wrote: > Why do we accept `Code` as a parameter here inste

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 122904. rwols marked an inline comment as done. rwols added a comment. Removed braces around single statement if-else. https://reviews.llvm.org/D39430 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/JSONRPCDispat

[PATCH] D39430: [clangd] formatting: don't ignore style

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:153 + if (ReplacementsOrError) { +C.reply(json::ary{replacementsToEdits(Code, ReplacementsOrError.get())}); + } else { NIT: remove braces from single-statement branches ==

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols marked 4 inline comments as done. rwols added a comment. Is this ready to merge? I'd like to implement tests in another differential, I'm having trouble referencing a temp dir from `lit` in a JSON request to clangd. Comment at: clangd/ClangdServer.cpp:429 + auto Tagged

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 122606. rwols added a comment. - Merge with upstream, fix merge conflicts - Add a FIXME for a caching strategy for .clang-format files https://reviews.llvm.org/D39430 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clan

[PATCH] D39430: [clangd] formatting: don't ignore style

sammccall added a comment. Thanks! Mostly just comments around the IO (which I'm still learning about myself). Otherwise this LG. I'm afraid you're going to run into some merge conflicts with https://reviews.llvm.org/rL317486 - I think/hope not too bad, though. Comment at: c

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 121605. rwols added a comment. - Rebase to latest SVN revision. - Use `void reply(llvm:Expected Result)` for transparent error handling. - Use `FSProvider` from `ClangdServer` to provide a filesystem for `format::getStyle`. - Organize things around `llvm::Expec

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols marked 2 inline comments as done. rwols added inline comments. Comment at: clangd/ClangdServer.cpp:41 // Call clang-format. - // FIXME: Don't ignore style. - format::FormatStyle Style = format::getLLVMStyle(); - auto Result = format::reformat(Style, Code, Ranges, File

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols updated this revision to Diff 121524. rwols added a comment. Use llvm::Error for beautiful handling of errors. It's perhaps an idea to write a `replyExpected(llvm::Expected)`, that will transparently handle `llvm::Error`s. I'll write a unit test for these changes next. It's probably best

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols added inline comments. Comment at: clangd/ClangdLSPServer.cpp:93 +void ClangdLSPServer::replyWithTextEditsOrError( +Ctx C, std::string Code, sammccall wrote: > This function is pretty hard to understand from a high level - it's a bit of > an odd spli

[PATCH] D39430: [clangd] formatting: don't ignore style

sammccall added a comment. Thanks for this! Comment at: clangd/ClangdLSPServer.cpp:93 +void ClangdLSPServer::replyWithTextEditsOrError( +Ctx C, std::string Code, This function is pretty hard to understand from a high level - it's a bit of an odd split.

[PATCH] D39430: [clangd] formatting: don't ignore style

krasimir added a comment. For testing you could do something similar to how clang-format does it . https://reviews.llvm.org/D39430 ___ cfe-commits mailing list

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols added a comment. Does anyone have an idea how I would write a test for this? Also, I should have commit access after approval now. So that's less work for you guys ;) https://reviews.llvm.org/D39430 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D39430: [clangd] formatting: don't ignore style

rwols created this revision. Look up in parent directories for a .clang-format file. Use "LLVM" as fallback style. https://reviews.llvm.org/D39430 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h Index: clangd/ClangdServer.h