[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few NITs and a few questions about the possibility of moving this to `clang-format` at some point in the future.
>From what I can to simplify the calling, we need to:

1. teach `clang-format` about a cursor (by making it a special token or somehow else), this would allow to avoid some book-keeping;
2. teach `clang-format` to format in presence of unmatched parentheses. Again, would simplify things. Although not sure it's easy to do in `clang-format` either;
3. teach `clang-format` to respect indentation before the cursor (added by the editor in our case).

After that we don't seem to need the complicated multi-pass replacements.

- Am I missing something that we also need from `clang-format` here?
- How would you estimate the amount of work needed to move this functionality to `clang-format`?
- In general, do you think it's worth moving it there or not?



================
Comment at: clangd/Format.cpp:83
+
+llvm::StringRef Filename = "<stdin>";
+
----------------
NIT:  put a comment about filename being required but ignored here to explain why we need this variable.


================
Comment at: clangd/Format.h:25
+/// Applies limited formatting around new \p InsertedText.
+/// The \p Code already contains the updated text near \p Cursor, and may have
+/// had additional / characters (such as indentation) inserted by the editor.
----------------
NIT: maybe be more concrete? `near \p Cursor` means `right before \p Cursor`? Or can it have other characters (e.g. whitespace in your example)?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60605/new/

https://reviews.llvm.org/D60605