kadircet added a comment.
implementation lgtm with a few nits.
main concern is about the new getlangopts helper
================
Comment at: clang-tools-extra/clangd/ParsedAST.h:80
+ const LangOptions &getLangOpts() const {
+ return getASTContext().getLangOpts();
----------------
can we introduce this helper in a new patch, while changing other occurrences
in clangd?
================
Comment at:
clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp:34
+/// After:
+/// NSLocalizedString(@"description", "")
+class ObjCLocalizeStringLiteral : public Tweak {
----------------
NSLocalizedString(@"description", `@`"")
================
Comment at:
clang-tools-extra/clangd/refactor/tweaks/ObjCLocalizeStringLiteral.cpp:69
+ SM, CharSourceRange::getCharRange(Str->getBeginLoc()),
+ "NSLocalizedString(", Inputs.AST.getASTContext().getLangOpts()));
+ SourceLocation EndLoc = Lexer::getLocForEndOfToken(
----------------
maybe extract `Inputs.AST.getASTContext().getLangOpts()` into a variable and
make use of it in the following places as well? (line 72 and 75)
================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:131
+ // Ensure the the action can be initiated in the string literal.
+ EXPECT_AVAILABLE(R"(id x = ^@^"^t^est^";)");
+ EXPECT_AVAILABLE(R"(id x = [[@"test"]];)");
----------------
nit: you can combine all of this into a single test
================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:136
+ // Ensure that the action can't be initiated in other places.
+ EXPECT_UNAVAILABLE(R"(i^d ^x ^= @"test";^)");
+ EXPECT_UNAVAILABLE(R"(id [[x]] = @"test";)");
----------------
nit: you can combine all of this into a single test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69543/new/
https://reviews.llvm.org/D69543
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
