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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits