Page MenuHomePhabricator

rwols (Raoul Wols)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 18 2017, 5:03 AM (91 w, 4 d)

Recent Activity

Jul 29 2018

rwols closed D49967: [clangd] Add command-line option to suppress the space and the circular dot prepended in a completion label..

Oh... I should have used arc land instead of svn commit. Here's the commit: https://reviews.llvm.org/rCTE338223

Jul 29 2018, 12:21 PM
rwols committed rCTE338223: [clangd] Add command-line option.
[clangd] Add command-line option
Jul 29 2018, 12:15 PM
rwols committed rL338223: [clangd] Add command-line option.
[clangd] Add command-line option
Jul 29 2018, 12:13 PM
rwols updated the diff for D49967: [clangd] Add command-line option to suppress the space and the circular dot prepended in a completion label..

Avoid double negative for command line option

Jul 29 2018, 12:08 PM
rwols created D49967: [clangd] Add command-line option to suppress the space and the circular dot prepended in a completion label..
Jul 29 2018, 11:23 AM

May 11 2018

rwols added a comment to D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type.

Can we get this merged soon? I'm experiencing a lot of false positive diagnostics with this library at work.

May 11 2018, 10:58 AM

Dec 13 2017

rwols abandoned D39430: [clangd] various fixes.
Dec 13 2017, 12:28 AM
rwols closed D40860: [clangd] Fix diagnostic positions.

D41118 fixes both the start positions as well as the ranges, let's merge that.

Dec 13 2017, 12:28 AM

Dec 12 2017

rwols committed rCTE320524: [clangd] (Attempt to) read clang-format file for document formatting.
[clangd] (Attempt to) read clang-format file for document formatting
Dec 12 2017, 12:26 PM
rwols committed rL320524: [clangd] (Attempt to) read clang-format file for document formatting.
[clangd] (Attempt to) read clang-format file for document formatting
Dec 12 2017, 12:26 PM
rwols closed D41031: [clangd] (Attempt to) read clang-format file for document formatting.
Dec 12 2017, 12:25 PM
rwols accepted D41118: [clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions.

LGTM, you can go ahead and land this, I'll drop D40860. I'm happy we got to the bottom of this :)

Dec 12 2017, 11:48 AM

Dec 8 2017

rwols added a comment to D39430: [clangd] various fixes.

This differential is now split into https://reviews.llvm.org/D41031 and https://reviews.llvm.org/D40860.

Dec 8 2017, 1:53 PM
rwols created D41031: [clangd] (Attempt to) read clang-format file for document formatting.
Dec 8 2017, 1:51 PM

Dec 5 2017

rwols updated the summary of D40860: [clangd] Fix diagnostic positions.
Dec 5 2017, 2:52 PM
rwols created D40860: [clangd] Fix diagnostic positions.
Dec 5 2017, 2:47 PM
rwols added a comment to D39430: [clangd] various fixes.

I will split this diff up into separate parts.

Dec 5 2017, 11:17 AM

Dec 2 2017

rwols retitled D39430: [clangd] various fixes from [clangd] formatting: don't ignore style to [clangd] various fixes.
Dec 2 2017, 6:43 AM
rwols updated the diff for D39430: [clangd] various fixes.
  • 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 case of a TokenRange, we have to determine the end of the token ourselves (this would occur, for example, in things like if (i=1) { ... }; the fixit hint to replace = with == would be a TokenRange for some reason).
Dec 2 2017, 6:41 AM

Nov 26 2017

rwols added inline comments to D39430: [clangd] various fixes.
Nov 26 2017, 8:22 AM
rwols updated the diff for D39430: [clangd] various fixes.
  • 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.
  • Fix (or introduce bug?) wrong conversion between clang line/columns and LSP line/columns. From what I understand, clang line/cols are 1-based, and LSP line/cols are 0-based. The line numbers were already converted correctly (subtracting 1 when going from clang line numbers to LSP line numbers), but the column numbers were not converted correctly. Consequently, a bunch of tests had to be adjusted for the correct (or wrong?) column numbers.
Nov 26 2017, 8:16 AM

Nov 18 2017

rwols added inline comments to D39430: [clangd] various fixes.
Nov 18 2017, 4:05 AM
rwols added inline comments to D39430: [clangd] various fixes.
Nov 18 2017, 2:16 AM

Nov 14 2017

rwols added inline comments to D39430: [clangd] various fixes.
Nov 14 2017, 1:33 PM
rwols updated the diff for D39430: [clangd] various fixes.

Removed braces around single statement if-else.

Nov 14 2017, 1:26 PM

Nov 12 2017

rwols added a comment to D39430: [clangd] various fixes.

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.

Nov 12 2017, 2:08 PM
rwols updated the diff for D39430: [clangd] various fixes.
  • Merge with upstream, fix merge conflicts
  • Add a FIXME for a caching strategy for .clang-format files
Nov 12 2017, 2:07 PM

Nov 4 2017

rwols updated the diff for D39430: [clangd] various fixes.
  • Rebase to latest SVN revision.
  • Use void reply(llvm:Expected<Twine> Result) for transparent error handling.
  • Use FSProvider from ClangdServer to provide a filesystem for format::getStyle.
  • Organize things around llvm::Expected, I've become a fan of that class :-)
Nov 4 2017, 4:12 PM
rwols added inline comments to D39435: Adds a json::Expr type to represent intermediate JSON expressions..
Nov 4 2017, 8:03 AM

Nov 3 2017

rwols added inline comments to D39430: [clangd] various fixes.
Nov 3 2017, 12:29 PM
rwols updated the diff for D39430: [clangd] various fixes.

Use llvm::Error for beautiful handling of errors.

Nov 3 2017, 12:27 PM

Nov 2 2017

rwols added inline comments to D39430: [clangd] various fixes.
Nov 2 2017, 3:38 PM

Oct 30 2017

rwols added a comment to D39430: [clangd] various fixes.

Does anyone have an idea how I would write a test for this?

Oct 30 2017, 1:52 PM
rwols created D39430: [clangd] various fixes.
Oct 30 2017, 1:49 PM

Oct 23 2017

rwols added a comment to D38939: [clangd] Handle exit notification (proper shutdown).

you have commit access now, right?

Oct 23 2017, 3:00 PM
rwols updated the diff for D38939: [clangd] Handle exit notification (proper shutdown).
  • Clarify why we invert the tests in protocol.test,
  • Use a variable name to clarify why we exit with status code 1 (when run() returns true),
  • Reword misleading comment in onShutdown method.
Oct 23 2017, 2:49 PM

Oct 17 2017

rwols added a comment to D38939: [clangd] Handle exit notification (proper shutdown).

So I'm not totally convinced that treating an abrupt exit or EOF as an error is worth the hassle

Oct 17 2017, 12:30 PM
rwols updated the diff for D38939: [clangd] Handle exit notification (proper shutdown).
  • Add test shutdown-with-exit.test
  • Add test shutdown-without-exit.test
  • Make protocol.test pass by inverting exit code ("# RUN: not ...")
Oct 17 2017, 12:29 PM

Oct 16 2017

rwols added a comment to D38939: [clangd] Handle exit notification (proper shutdown).

I couldn't get the protocol.test to succeed; clangd returns 1 now and that trips up lit. I think I have to use XFAIL from lit somehow but didn't figure it out.

Oct 16 2017, 12:57 PM
rwols updated the diff for D38939: [clangd] Handle exit notification (proper shutdown).

Return 0 if shutdown was received before exit, otherwise return 1

Oct 16 2017, 12:54 PM
rwols added inline comments to D38939: [clangd] Handle exit notification (proper shutdown).
Oct 16 2017, 11:22 AM

Oct 15 2017

rwols created D38939: [clangd] Handle exit notification (proper shutdown).
Oct 15 2017, 2:59 PM

Oct 11 2017

rwols accepted D38720: [clangd] Report proper kinds for 'Keyword' and 'Snippet' completion items..

LGTM

Oct 11 2017, 8:31 AM

Oct 5 2017

rwols added a comment to D38048: [clangd] Add textDocument/signatureHelp.

LGTM. Do you need my help in landing this?

Oct 5 2017, 12:21 PM

Oct 3 2017

rwols added a comment to D38048: [clangd] Add textDocument/signatureHelp.

Thinking ahead, it's probably best if we change the behavior of SnippetCompletionItemCollector as follows:

Oct 3 2017, 2:40 PM
rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.

Reword the documentation of the signature help structures in Protocol.h

Oct 3 2017, 2:11 PM

Oct 2 2017

rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.
  • Update invokeCodeComplete function to also take a CodeCompleteOptions, we assign it to the CompilerInstance's FrontendOpts.CodeCompleteOpts.
  • Remove unused variable in SignatureHelpCollector::ProcessOverloadCandidate.
Oct 2 2017, 2:57 PM

Oct 1 2017

rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.
  • Fix fillSortText: used the old version, now using new version from upstream
  • Fix initialize-params.test and initialize-params-invalid.test to account for signature help
  • All tests pass now
Oct 1 2017, 12:33 PM
rwols added inline comments to D38048: [clangd] Add textDocument/signatureHelp.
Oct 1 2017, 12:23 PM

Sep 30 2017

rwols added inline comments to D38048: [clangd] Add textDocument/signatureHelp.
Sep 30 2017, 2:15 PM
rwols added a comment to D38048: [clangd] Add textDocument/signatureHelp.
Sep 30 2017, 1:59 PM
rwols added a comment to D38048: [clangd] Add textDocument/signatureHelp.

There were some failing tests, probably because we use an extra digit for the sortText property now. I haven't touched those tests.

Sep 30 2017, 12:14 PM
rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.
  • Updated to latest upstream revision.
  • Fix optional/default parameter handling: each default parameter now has its own label (thanks, ilya!).
  • Remove the templated "invokeClangAction" function and use polymorphism with CodeCompleteConsumer instead. The function is now called "invokeCodeComplete".
Sep 30 2017, 12:10 PM

Sep 25 2017

rwols added a comment to D38225: [clangd] Fix missing "message" key when responding with unsupported method.

No, I don't have commit access. Feel free to merge.

Sep 25 2017, 10:01 AM
rwols created D38225: [clangd] Fix missing "message" key when responding with unsupported method.
Sep 25 2017, 5:03 AM
rwols accepted D38083: [clangd] Skip informative qualifier chunks..

LGTM

Sep 25 2017, 5:02 AM
rwols added inline comments to D38048: [clangd] Add textDocument/signatureHelp.
Sep 25 2017, 5:02 AM
rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.
  • Add signature-help.test
  • Modify formatting.test to make the test pass. The test failed because clangd's response to the initialize request was changed to advertise signature help trigger characters, so the expected content length changed.
Sep 25 2017, 5:02 AM
rwols updated the diff for D38048: [clangd] Add textDocument/signatureHelp.
  • Update doxygen comment to say "signature help" instead of "code complete",
  • Refactor: put the FillDocumentation member function outside of CompletionItemsCollector so that SignatureHelpCollector can also use it.
  • Refactor: put the FillSortText member function outside of CompletionItemsCollector because it has nothing to do with CompletionItemsCollector.
  • Refactor: Make a templated function called invokeClangAction that will invoke the given CodeCompleteConsumer type. Both clangd::codeComplete and clangd::signatureHelp now call this function.
  • Present optional/defaulted parameters too (using CodeCompletionString::CK_Optional chunks).
Sep 25 2017, 5:02 AM

Sep 20 2017

rwols added inline comments to D38083: [clangd] Skip informative qualifier chunks..
Sep 20 2017, 12:33 PM

Sep 19 2017

rwols created D38048: [clangd] Add textDocument/signatureHelp.
Sep 19 2017, 11:44 AM

Sep 12 2017

rwols added a comment to D37101: [clangd] Add support for snippet completions.

Thanks! Thinking ahead now so we're on the same page, you will be implementing the client's initialize request, and I'll start work on textDocument/signatureHelp.

Sep 12 2017, 9:45 AM · Restricted Project

Sep 11 2017

rwols added a comment to D37101: [clangd] Add support for snippet completions.

No, I don't have access to the repo, sorry forgot to mention this. Go ahead and merge.

Sep 11 2017, 10:52 AM · Restricted Project

Sep 8 2017

rwols updated the diff for D37101: [clangd] Add support for snippet completions.

Update the description for the "-enable-snippets" option.

Sep 8 2017, 12:32 PM · Restricted Project

Sep 7 2017

rwols updated the diff for D37101: [clangd] Add support for snippet completions.

Change command-line option back to "-enable-snippets", disable snippets by default. This is a temporary solution and we should instead inspect the "initialize" request from the client to check wether the client supports snippets.

Sep 7 2017, 12:53 PM · Restricted Project

Sep 6 2017

rwols added inline comments to D37101: [clangd] Add support for snippet completions.
Sep 6 2017, 8:03 AM · Restricted Project

Sep 5 2017

rwols added inline comments to D37101: [clangd] Add support for snippet completions.
Sep 5 2017, 12:55 PM · Restricted Project
rwols updated the diff for D37101: [clangd] Add support for snippet completions.
  • Fix failing clangd tests
Sep 5 2017, 12:50 PM · Restricted Project
rwols updated the diff for D37101: [clangd] Add support for snippet completions.
  • Don't use const for non-reference parameters
  • lowerCamelCase for functions, verb phrase
  • Field definitions at the bottom of the class
  • Make ProcessChunks virtual
  • Add comment that fallthrough to the next case is intended
  • Revert new constructors for CompletionItem, they are pointless
  • Bump minimum version for vscode-languageclient and vscode-languageserver to 3.3.0
Sep 5 2017, 12:50 PM · Restricted Project

Sep 3 2017

rwols retitled D37101: [clangd] Add support for snippet completions from [clangd] [WIP] Add support for snippet completions to [clangd] Add support for snippet completions.
Sep 3 2017, 3:44 AM · Restricted Project

Sep 2 2017

rwols updated the diff for D37101: [clangd] Add support for snippet completions.
  • Adjust SnippetCompletionItemCollector such that it only sets insertTextFormat to InsertTextFormat::Snippet when it's actually needed (i.e. we encounter a CK_Placeholder chunk).
  • Fix failing tests in test/clangd/{completion.test,authority-less-uri.test} because of the new changes.
  • Add new tests in test/clangd/completion-snippet.test that tests the snippet functionality.
Sep 2 2017, 1:41 PM · Restricted Project
rwols added inline comments to D37101: [clangd] Add support for snippet completions.
Sep 2 2017, 2:51 AM · Restricted Project
rwols added a comment to D37101: [clangd] Add support for snippet completions.

Forgot to mention I've also made sure the filterText field is taken care of now (both for snippet and for plaintext items).

Sep 2 2017, 2:21 AM · Restricted Project
rwols updated the diff for D37101: [clangd] Add support for snippet completions.
  • Split up the CompletionItemsCollector into two classes called PlainTextCompletionItemsCollector and SnippetCompletionItemsCollector to allow collecting both types of items.
  • Add a command-line setting to clangd called "--enable-snippets" that, when set, will make clangd use the SnippetCompletionItemsCollector. The default is to use the PlainTextCompletionItemsCollector.
  • Enable "code pattern" completions when "--enable-snippets" is true.
  • Do not add a space in the completion label when we encounter a CK_VerticalSpace in the SnippetCompletionItemsCollector. A space looks weird.
Sep 2 2017, 2:16 AM · Restricted Project

Aug 30 2017

rwols updated the diff for D37101: [clangd] Add support for snippet completions.

Some more tweaks

Aug 30 2017, 6:50 AM · Restricted Project
rwols added a comment to D37101: [clangd] Add support for snippet completions.

So at this point the C++ changes are basically done, save for some minor things I guess. The problem is still that the VSCode extension doesn't do anything clever with the snippets. I have zero experience with TypeScript let alone extension development for VSCode, so it can take a while for me to investigate. I'd appreciate if someone else could take a look.

Aug 30 2017, 3:33 AM · Restricted Project
rwols added a comment to D37101: [clangd] Add support for snippet completions.

I followed your advice and kept the snippet functionality. We'll do the SignatureHelp stuff in another review.

Aug 30 2017, 3:09 AM · Restricted Project
rwols updated the diff for D37101: [clangd] Add support for snippet completions.

Tidy up snippet handling

Aug 30 2017, 3:04 AM · Restricted Project

Aug 27 2017

rwols added a comment to D37101: [clangd] Add support for snippet completions.

After digging into VSCode, I now think that presenting snippets is the wrong way forward, and I believe the right way is to implement the signatureHelpProvider protocol for method/function parameters. See: https://github.com/Microsoft/vscode-docs/blob/master/docs/extensionAPI/language-support.md#help-with-function-and-method-signatures.

Aug 27 2017, 4:55 AM · Restricted Project

Aug 24 2017

rwols added inline comments to D37101: [clangd] Add support for snippet completions.
Aug 24 2017, 2:09 PM · Restricted Project
rwols updated the diff for D37101: [clangd] Add support for snippet completions.

[clangd] [WIP] Add support for snippet completions

Aug 24 2017, 1:58 PM · Restricted Project
rwols added a comment to D37101: [clangd] Add support for snippet completions.

Thanks for the quick review! I'm new to Phabricator and the arc CLI tool. Is the workflow like this: "address a comment, change a few lines, do arc diff, do this multiple times", or is it like this: "address all the comments, change lots of lines, do arc diff, do this once"?

Aug 24 2017, 6:24 AM · Restricted Project
rwols created D37101: [clangd] Add support for snippet completions.
Aug 24 2017, 3:05 AM · Restricted Project