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