[PATCH] D57739: [clangd] Format tweak's replacements.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 8 04:44:33 PST 2019


ilya-biryukov added a comment.

In D57739#1390321 <https://reviews.llvm.org/D57739#1390321>, @sammccall wrote:

> It's not about stability or whether the functionality is desired, but layering.
>  Unit tests having narrow scope is a good thing - if we want system tests that check clangdserver's behavior, they should test clangdserver.
>  Clients that don't go through clangdserver probably want formatting, but an immediate cleanupAndFormat on each generated change isn't necessarily the right way to do that.


My argument is that it's better to provide formatting by default in the public interface for **applying a single tweak**.
I might be missing some use-cases, e.g. one that comes to mind is applying multiple tweaks in a row, in which case we'd want to format once and not for every tweak.

My concerns are code duplication and ease of use for the clients. Having both `apply` and `applyAndFormat` (the latter being a non-virtual or a free-standing utility function) in the public interface of the `Tweak` would totally do it for me.
However, I also think tests should format by default, not sure we agree on this.
WDYT?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739





More information about the cfe-commits mailing list