[PATCH] D57739: [clangd] Format tweak's replacements.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 8 02:22:41 PST 2019
sammccall added a comment.
In D57739#1385144 <https://reviews.llvm.org/D57739#1385144>, @hokein wrote:
> In D57739#1384844 <https://reviews.llvm.org/D57739#1384844>, @ilya-biryukov wrote:
>
> > Could we move the code to the `Tweak` itself, so that all the callers would get the formatting? I.e.
> >
> > class Tweak {
> > Replacements apply() { // This is now non-virtual.
> > auto Replacements = doApply();
> > return cleanupAndFormat(Replacements);
> > }
> >
> > protected:
> > virtual Replacements doApply() = 0; // inheritors should implement this now. Note: the name is terrible, suggestions are welcome.
> > };
> >
> >
> > This would ensure the callers don't need to deal with the formatting on their own.
>
>
> This seems introduce intrusive changes to the Tweak interface (Tweak will need to know the `FormatStyle` somehow), I think this might be done in another patch.
I agree with this concern, and don't think this is an appropriate layer to be aware of styling or failing on format errors. Can we consider moving the formatting to clangdserver as originally planned?
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