[PATCH] D28315: Update tools to use new getStyle API

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Let me know when broken tests are fixed and this patch (and the corresponding patch) is ready again for review. Also let me know if you need any help.



================
Comment at: change-namespace/ChangeNamespace.cpp:892
+      llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+      continue;
+    }
----------------
amaiorano wrote:
> amaiorano wrote:
> > amaiorano wrote:
> > > ioeric wrote:
> > > > I'd still like to apply replacements that are not cleaned up. Could you add the following line before continue, thanks! :)
> > > > ```
> > > > FileToReplacements[FilePath] = Replaces;
> > > > ```
> > > Will do.
> > Ah darn, just realized that I forgot to make this change you asked for @ioeric. Let me know if everything else is okay, and I'll be sure to add this in before submitting. 
> @ioeric Was looking at this code, and I'm wondering about the change you wanted me to make here. You said that you'd still like to apply replacements that are not cleaned up, so shouldn't that also be the case for when format::cleanupAroundReplacements fails (just below)? In other words, shouldn't it be:
> 
> ```
>     // Clean up old namespaces if there is nothing in it after moving.
>     auto CleanReplacements =
>         format::cleanupAroundReplacements(Code, Replaces, *Style);
>     if (!CleanReplacements) {
>       llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
>       FileToReplacements[FilePath] = Replaces;
>       continue;
>     }
>     FileToReplacements[FilePath] = *CleanReplacements;
>   }
> ```
> 
> If so, I propose instead that we add 
> ```
> FileToReplacements[FilePath] = Replaces;
> ``` 
> before the call to format::getStyle so that if either of the format-related calls below it (getStyle or cleanupAroundReplacements) fail, we get the unclean replacements.
After taking a closer look, I realize that we don't actually need this change since `Replaces` is a reference to `FileToReplacements[FilePath]`.


https://reviews.llvm.org/D28315