Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] More on unbreakable strings in TypeScript #66321

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

sstwcw
Copy link
Contributor

@sstwcw sstwcw commented Sep 14, 2023

Now. string literals in lines beginning with export type will not be broken.

The case was missed in 5db201f. I don't know TypeScript. And merging GitHub pull requests seems to be a little too easy. So it got committed before the reviewers had a chance to find edge cases.

@sstwcw sstwcw requested a review from a team as a code owner September 14, 2023 03:39
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-format labels Sep 14, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2023

@llvm/pr-subscribers-clang-format

@llvm/pr-subscribers-clang

Changes Now. string literals in lines beginning with `export type` will not be broken.

The case was missed in 5db201f. I don't know TypeScript. And merging GitHub pull requests seems to be a little too easy. So it got committed before the reviewers had a chance to find edge cases.

Full diff: https://github.com/llvm/llvm-project/pull/66321.diff

2 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+4-2)
  • (modified) clang/unittests/Format/FormatTestJS.cpp (+3)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 75ab08de42ea0e8..6673b5c703b835f 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -2242,8 +2242,10 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
       return nullptr;
 
     // Strings in TypeScript types and dictionary keys can not be broken.
-    if (Style.isJavaScript() && (Current.is(TT_SelectorName) ||
-                                 State.Line->startsWith(Keywords.kw_type))) {
+    if (Style.isJavaScript() &&
+        (Current.is(TT_SelectorName) ||
+         State.Line->startsWith(Keywords.kw_type) ||
+         State.Line->startsWith(tok::kw_export, Keywords.kw_type))) {
       return nullptr;
     }
 
diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp
index 309326569143df6..51543d0a54d8561 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -1604,6 +1604,9 @@ TEST_F(FormatTestJS, StringLiteralConcatenation) {
   verifyFormat("/* type */ type x =\n"
                "    'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
                getGoogleJSStyleWithColumns(20));
+  verifyFormat("export type x =\n"
+               "    'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
+               getGoogleJSStyleWithColumns(20));
   // Dictionary keys can't be broken. Values can be broken.
   verifyFormat("var w = {\n"
                "  'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':\n"

@alexfh
Copy link
Contributor

alexfh commented Sep 14, 2023

LGTM

@alexfh alexfh self-requested a review September 14, 2023 09:00
Now. string literals in lines beginning with `export type` will not be
broken.

The case was missed in 5db201f.  I don't know TypeScript.  And
merging GitHub pull requests seems to be a little too easy.  So it got
committed before the reviewers had a chance to find edge cases.
@eywdck2l eywdck2l merged commit cb479e7 into llvm:main Sep 14, 2023
@sstwcw sstwcw deleted the format-js-strings branch September 14, 2023 12:43
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Sep 14, 2023
Now. string literals in lines beginning with `export type` will not be
broken.

The case was missed in 5db201f.  I don't know TypeScript.  And
merging GitHub pull requests seems to be a little too easy.  So it got
committed before the reviewers had a chance to find edge cases.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Now. string literals in lines beginning with `export type` will not be
broken.

The case was missed in 5db201f.  I don't know TypeScript.  And
merging GitHub pull requests seems to be a little too easy.  So it got
committed before the reviewers had a chance to find edge cases.
@owenca owenca removed the clang Clang issues not falling into any other category label May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants