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

[Antonio Maiorano via Phabricator via cfe-commits]

amaiorano added inline comments.


================
Comment at: change-namespace/ChangeNamespace.cpp:892
+      llvm::errs() << llvm::toString(Style.takeError()) << "\n";
+      continue;
+    }
----------------
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.


https://reviews.llvm.org/D28315