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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
