ioeric added inline comments.
================
Comment at: lib/Format/Format.cpp:1597
@@ +1596,3 @@
+ SmallVector<CharSourceRange, 8>(Ranges.begin(),
Ranges.end())),
+ UnwrappedLines(1),
+ Encoding(encoding::detectEncoding(SourceMgr.getBufferData(ID))),
----------------
djasper wrote:
> Why are you doing this?
I am not quite sure here actually. This was copied from Formatter. And since
`consumeUnwrappedLine` requires at lease one element in `UnwrappedLines`, I
figured this is necessary initialization?
================
Comment at: lib/Format/Format.cpp:1818
@@ +1817,3 @@
+
+ // Merge multiple continuous token deletions into one big deletion.
+ unsigned Idx = 0;
----------------
djasper wrote:
> Can you explain why you are doing this?
I want to reduce the number of replacements so that when we do `reformat` on
the fixed code, there could be fewer changed code ranges considering that the
current implementation of `computeAffectedLines` is not that efficient when we
have many ranges.
================
Comment at: lib/Format/Format.cpp:2286
@@ +2285,3 @@
+ const FormatStyle &, StringRef, std::vector<tooling::Range>, StringRef)>
+ ReplacementsProcessFunctionType;
+
----------------
djasper wrote:
> Don't define a typedef for something that is only used once. Also, this is an
> internal function, how about writing this as:
>
> template <typename T>
> static tooling::Replacements
> processReplacements(StringRef Code, const tooling::Replacements &Replaces,
> T ProcessFunc,
> const FormatStyle &Style) {
>
> }
>
> No need to spell out the function type (I think).
Wow, this is really helpful! Thanks!
================
Comment at: lib/Format/Format.cpp:1590
@@ +1589,3 @@
+
+class CodeProcesser : public UnwrappedLineConsumer {
+public:
----------------
I'm not quite sure if the name is accurate. Do you have a better name?
http://reviews.llvm.org/D18551
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
