[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2025-01-02 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk closed 
https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2025-01-01 Thread Oleksandr T. via cfe-commits

a-tarasyuk wrote:

> I’m fine with continuing the discussion in the GitHub issue

Ok, let's move the discussion to the issue, and I'll delete the PR. From there, 
we can summarize all the concerns to proceed with a decision on this proposal. 
I think it would be useful to add comments or update the status of the issue to 
avoid anyone spending time on development because before I started working on 
it proposal seemed to have no concerns and was ready for development.


https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2025-01-01 Thread Owen Pan via cfe-commits

owenca wrote:

> > If we were to add // clang-format off-next-line, would "next line" mean the 
> > next physical or logical/unwrapped line?
> 
> I would expect it to apply only to the physical line, similar to how other 
> formatters work. However, the main concern doesn’t seem to be about its 
> behavior but rather about extending clang-format directive with new options 
> at all.

This is an essential detail missing from the description of the proposed option 
for us to accept a new option and review the patch.

> From my perspective, these options are just additional ways to control 
> formatting and give users more flexibility. These options aren’t intended to 
> eliminate `clang-format` usage, and based on user feedback on the issue, it 
> seems there are cases in which these options are useful. For 
> [instance](https://github.com/athomps/lammps/blob/ceb9466172398e9a20cb510528b4b17f719c7cf2/src/set.h#L15-L17),
> 
> ```c++
> // clang-format off
> CommandStyle(set,Set);
> // clang-format on
> ```
> 
> vs
> 
> ```c++
> CommandStyle(set,Set); // clang-format off-line
> ```

For simplicity and clarify, I might accept a lone comment above the (physical) 
line to be skipped by clang-format, but not a trailing comment.

> If these options introduce significant complexity that could lead to 
> regressions without improving the development experience, I believe the 
> discussion should be wrapped up, and both the PR and issue should be closed. 
> Otherwise, it would be helpful to continue the discussion in the issue to 
> gather more insights into why users need these changes. Does that make sense?

I’m fine with continuing the discussion in the GitHub issue, but I’m still with 
@mydeveloperday on this one.

https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2025-01-01 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/6] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-30 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/6] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-30 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 70c9152f99818ffd0342260ae12d709268031235 
2215fe06429f4d7238eb9b778ad33cefdb9a7b89 --extensions cpp,h -- 
clang/include/clang/Format/Format.h 
clang/lib/Format/DefinitionBlockSeparator.cpp clang/lib/Format/Format.cpp 
clang/lib/Format/FormatTokenLexer.cpp clang/lib/Format/FormatTokenLexer.h 
clang/lib/Format/IntegerLiteralSeparatorFixer.cpp 
clang/lib/Format/SortJavaScriptImports.cpp clang/lib/Format/TokenAnnotator.cpp 
clang/unittests/Format/FormatTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 9bf70a8ccb..185f4fbcb5 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -28320,7 +28320,7 @@ TEST_F(FormatTest, KeepFormFeed) {
"void f();",
Style);
 }
-  
+
 TEST_F(FormatTest, DisableLine) {
   verifyFormat("int  a  =  1; // clang-format off-line\n"
"int b = 1;",

``




https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-30 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/5] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-24 Thread Oleksandr T. via cfe-commits

a-tarasyuk wrote:

> If we were to add // clang-format off-next-line, would "next line" mean the 
> next physical or logical/unwrapped line?

I would expect it to apply only to the physical line, similar to how other 
formatters work. However, the main concern doesn’t seem to be about its 
behavior but rather about extending clang-format directive with new options at 
all.

> To me the current // clang-format off/on is enough, for everything else log 
> an issue, or better still submit a pull request.

In most cases where users find the formatting unacceptable, they tend to 
disable it, while only a narrow group take the initiative to address the root 
cause by submitting pull requests or suggesting new rules. For instance, a 
[search for `clang-format` usage](https://shorturl.at/SFNGZ) on GitHub, even 
just for C++, shows hundreds of uses across various scenarios. Analyzing all of 
them and covering them with new rules would take a significant amount of time,. 
anyway, we can assume that these options are not without value.

>From my perspective, these options are just additional ways to control 
>formatting and give users more flexibility. These options aren’t intended to 
>eliminate `clang-format` usage, and based on user feedback on the issue, it 
>seems there are cases in which these options are useful. For 
>[instance](https://github.com/athomps/lammps/blob/ceb9466172398e9a20cb510528b4b17f719c7cf2/src/set.h#L15-L17),

```cpp
// clang-format off
CommandStyle(set,Set);
// clang-format on
```

vs

```cpp
CommandStyle(set,Set); // clang-format off-line
```

We don't know the context of why it was disabled. However, I would use `// 
clang-format off-line` in a similar way to `// NOLINT`., it's up to personal 
preference.

If these options introduce significant complexity that could lead to 
regressions without improving the development experience, I believe the 
discussion should be wrapped up, and both the PR and issue should be closed. 
Otherwise, it would be helpful to continue the discussion in the issue to 
gather more insights into why users need these changes. Does that make sense?

















https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-23 Thread Owen Pan via cfe-commits

owenca wrote:

> A nice multiline example I stumbled upon is:
> 
> ```c++
> Tree tree[]
> {
> {'D', tree + 1, tree + 2},
> //│
> //┌───┴┐
> //││
> {'B', tree + 3, tree + 4},   {'F', tree + 5, tree + 
> 6},
> //││
> //  ┌─┴─┐  ┌───┴─┐
> //  │   │  │ │
>   {'A'},  {'C'}, {'E'},{'G'}
> };
> ```

Which line(s) do you want clang-format to skip here?

> And [#54334 
> (comment)](https://github.com/llvm/llvm-project/issues/54334#issuecomment-2531049984)
>  makes a good example on why only one line disabling would be desired.

The current way to skip the line in that example is as follows:
```cpp
int foo(Resources *resources, int i, int j, int k) {
  if (i < 0 && j < 0) {
// clang-format off
myproject::detail::LogErrorPrintf( resources->logger, "both i and j can not 
be negative at the same time.\ni = %d, j = %d\n", i, j);
// clang-format on
return -1;
  }

  if (i < 0) {
j *= 10;
  }
  if (j < 0) {
k += 5;
  }
  return i + j * k;
}
```
What's wrong with that other than it may be less convenient? Actually, I prefer 
the current way as it makes the skipped line stand out. If we were to add `// 
clang-format off-next-line`, would "next line" mean the next physical or 
logical/unwrapped line?


https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-23 Thread Björn Schäpers via cfe-commits

HazardyKnusperkeks wrote:

> > > I struggle with changes that encourage people to not be fully 
> > > clang-formatted, I would prefer to ask why we need this feature, can we 
> > > have some examples of where this would be used?
> > 
> > 
> > This makes it so only one line isn't formatted instead of the current 
> > solution which is 2 lines unformatted. Additionally, if you start writing 
> > code in between the unformatted region you inherit the unformatted scope. 
> > Being able to disable the formatter for only a single line means the 
> > formatter will be disabled for the shortest amount of code possible.
> 
> I understand what is can be used for, I'm asking why its needed? I don't 
> understand why people are needing to unformat just one line, what is broken?
> 
> The implementation IMHO just complicates the code (I much prefer the 
> isClangFormatOn() function than the parse())
> 
> I don't even deny it might be a nice to have, my concern is why do we 
> continue to appease the people who don't want to use clang-format warts and 
> all. I would prefer we put the effort into fixing the formatting issues which 
> mean people are having to use this rather than marking hundreds of lines as 
> // clang-format off.
> 
> Rather than complicating the code with a myriad of rules that effectively 
> give half a dozen ways of doing the same thing. To me the current // 
> clang-format off/on is enough, for everything else log an issue, or better 
> still submit a pull request.

I think there are multiple reasons to disable formatting for a range, and be 
the range only one line. A nice multiline example I stumbled upon is:
``` c++
Tree tree[]
{
{'D', tree + 1, tree + 2},
//│
//┌───┴┐
//││
{'B', tree + 3, tree + 4},   {'F', tree + 5, tree + 6},
//││
//  ┌─┴─┐  ┌───┴─┐
//  │   │  │ │
  {'A'},  {'C'}, {'E'},{'G'}
};
```
(Kindly borrowed from https://en.cppreference.com/w/cpp/coroutine/generator)
We will never be able to format something like this with clang-format.

I myself have a few places where automatic formatting would be possible, but 
kind of hard, they all have to do with some aligning.

And https://github.com/llvm/llvm-project/issues/54334#issuecomment-2531049984 
makes a good example on why only one line disabling would be desired.

Also this would bring us in par with clang-tidy which has `NOLINT` and 
`NOLINTNEXTLINE`.

https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-18 Thread Oleksandr T. via cfe-commits


@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {

a-tarasyuk wrote:

@HazardyKnusperkeks Thank you for the review. I've made the changes. I extended 
the parsing of the directive because handling only On/Off doesn't fully address 
cases where we need to determine what the directive does to apply the 
appropriate formatting—such as disabling the next line, disabling the current 
line, or toggling an entire block. Does that make sense? I'd be happy to 
discuss alternative solutions if there's a better way to handle this.


Based on the discussions on the issue and this PR, where a clear consensus on 
these features hasn’t been reached. I’ve added a comment 
https://github.com/llvm/llvm-project/pull/118566#issuecomment-2531572587 - it 
might be best to hold off on these changes until a decision is made, on whether 
to move forward or close the issue entirely.


https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-18 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/5] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-18 Thread via cfe-commits

mydeveloperday wrote:

> > I struggle with changes that encourage people to not be fully 
> > clang-formatted, I would prefer to ask why we need this feature, can we 
> > have some examples of where this would be used?
> 
> This makes it so only one line isn't formatted instead of the current 
> solution which is 2 lines unformatted. Additionally, if you start writing 
> code in between the unformatted region you inherit the unformatted scope. 
> Being able to disable the formatter for only a single line means the 
> formatter will be disabled for the shortest amount of code possible.

I understand what is can be used for, I'm asking why its needed? I don't 
understand why people are needing to unformat just one line, what is broken?

The implementation IMHO just complicates the code (I much prefer the 
isClangFormatOn() function than the parse()) 

I don't even deny it might be a nice to have, my concern is why do we continue 
to appease the people who don't want to use clang-format warts and all. I would 
prefer we put the effort into fixing the formatting issues which mean people 
are having to use this rather than marking hundreds of lines as // clang-format 
off.

Rather than complicating the code with a myriad of rules that effectively give 
half a dozen ways of doing the same thing. To me the current // clang-format 
off/on is enough, for everything else log an issue, or better still submit a 
pull request.


https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-18 Thread Björn Schäpers via cfe-commits


@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {

HazardyKnusperkeks wrote:

Wouldn't it have to be
```suggestion
  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) !=
   ClangFormatDirective::On) {
```

Otherwise you change the meaning, right?

https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-17 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/4] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-17 Thread Paul Hoad via cfe-commits

phoad107185 wrote:

> > I struggle with changes that encourage people to not be fully 
> > clang-formatted, I would prefer to ask why we need this feature, can we 
> > have some examples of where this would be used?
> 
> This makes it so only one line isn't formatted instead of the current 
> solution which is 2 lines unformatted. Additionally, if you start writing 
> code in between the unformatted region you inherit the unformatted scope. 
> Being able to disable the formatter for only a single line means the 
> formatter will be disabled for the shortest amount of code possible.

While I understand the reason I would ask again, what exactly are you disabling 
on a single line and why? Does the clang-format team really want to maintain 
more reasons to "not" format code? I would rather put the effort into 
understanding why you are having to use the // clang-format off in the first 
place

I work on a project (in my day job) with millions of lines of code, EVERY file 
is 100% clang-formatted, without any // clang-format off statements. When I hit 
a problem I report it against clang-format or even submit a fix. 

So again to reiterate my question why do we need this feature? what is the 
reason and why doesn't the current capabilities support what you need.


https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-16 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/4] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-11 Thread Brandon Staab via cfe-commits

BrandonStaab wrote:

> I struggle with changes that encourage people to not be fully 
> clang-formatted, I would prefer to ask why we need this feature, can we have 
> some examples of where this would be used?

This makes it so only one line isn't formatted instead of the current solution 
which is 2 lines unformatted. Additionally, if you start writing code in 
between the unformatted region you inherit the unformatted scope. Being able to 
disable the formatter for only a single line means the formatter will be 
disabled for the shortest amount of code possible.

https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-10 Thread Oleksandr T. via cfe-commits

a-tarasyuk wrote:

@mydeveloperday would it make sense to close the PR until a final decision 
about the feature is made, to avoid unnecessary review effort?

https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-09 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/4] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-05 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/4] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/4] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/2] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 75da343b0bd6e3b0f3219b349f6be4c882947820 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH 1/3] [clang-format] extend clang-format directive with options
 to prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 59 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 +---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 .../unittests/Format/SortImportsTestJava.cpp  |  9 +++
 10 files changed, 165 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDi

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-format

Author: Oleksandr T. (a-tarasyuk)


Changes

Fixes #54334 

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


11 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/include/clang/Format/Format.h (+9-2) 
- (modified) clang/lib/Format/DefinitionBlockSeparator.cpp (+2-1) 
- (modified) clang/lib/Format/Format.cpp (+41-18) 
- (modified) clang/lib/Format/FormatTokenLexer.cpp (+32-7) 
- (modified) clang/lib/Format/FormatTokenLexer.h (+7-1) 
- (modified) clang/lib/Format/IntegerLiteralSeparatorFixer.cpp (+2-2) 
- (modified) clang/lib/Format/SortJavaScriptImports.cpp (+7-3) 
- (modified) clang/lib/Format/TokenAnnotator.cpp (+3-1) 
- (modified) clang/unittests/Format/FormatTest.cpp (+53) 
- (modified) clang/unittests/Format/SortImportsTestJava.cpp (+9) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 755418e9550cf4..1d7fddb2236740 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -997,6 +997,7 @@ clang-format
   ``Never``, and ``true`` to ``Always``.
 - Adds ``RemoveEmptyLinesInUnwrappedLines`` option.
 - Adds ``KeepFormFeed`` option and set it to ``true`` for ``GNU`` style.
+- Adds ``off-line`` and ``off-next-line`` options to the ``clang-format`` 
directive
 
 libclang
 
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comme

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)


Changes

Fixes #54334 

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


11 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/include/clang/Format/Format.h (+9-2) 
- (modified) clang/lib/Format/DefinitionBlockSeparator.cpp (+2-1) 
- (modified) clang/lib/Format/Format.cpp (+41-18) 
- (modified) clang/lib/Format/FormatTokenLexer.cpp (+32-7) 
- (modified) clang/lib/Format/FormatTokenLexer.h (+7-1) 
- (modified) clang/lib/Format/IntegerLiteralSeparatorFixer.cpp (+2-2) 
- (modified) clang/lib/Format/SortJavaScriptImports.cpp (+7-3) 
- (modified) clang/lib/Format/TokenAnnotator.cpp (+3-1) 
- (modified) clang/unittests/Format/FormatTest.cpp (+53) 
- (modified) clang/unittests/Format/SortImportsTestJava.cpp (+9) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 755418e9550cf4..1d7fddb2236740 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -997,6 +997,7 @@ clang-format
   ``Never``, and ``true`` to ``Always``.
 - Adds ``RemoveEmptyLinesInUnwrappedLines`` option.
 - Adds ``KeepFormFeed`` option and set it to ``true`` for ``GNU`` style.
+- Adds ``off-line`` and ``off-next-line`` options to the ``clang-format`` 
directive
 
 libclang
 
diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..11802e8f5b3738 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3266,10 +3266,11 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
   FormattingOff = false;
 
 bool IsBlockComment = false;
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
 
-if (isClangFormatOff(Trimmed)) {
+if (CFD == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (CFD == ClangFormatDirective::On) {
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3453,10 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+ClangFormatDirective CFD = parseClangFormatDirective(Trimmed);
+if (CFD == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (CFD == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4192,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Len

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk ready_for_review 
https://github.com/llvm/llvm-project/pull/118566
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-04 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk updated 
https://github.com/llvm/llvm-project/pull/118566

>From 0badd09a2c77c94446b3ba352e48a54ff8fbc0b7 Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH] [clang-format] extend clang-format directive with options to
 prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 57 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 ++---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 9 files changed, 154 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..5b7659306696c5 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3267,9 +3267,9 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
 
 bool IsBlockComment = false;
 
-if (isClangFormatOff(Trimmed)) {
+if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::On) 
{
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3452,9 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4190,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDirectiveValuePos = Pos;
+while (EndDirectiveValuePos < Length) {
+  char Char = Comment[EndDirect

[clang] [clang-format] extend clang-format directive with options to prevent formatting for one line (PR #118566)

2024-12-03 Thread Oleksandr T. via cfe-commits

https://github.com/a-tarasyuk created 
https://github.com/llvm/llvm-project/pull/118566

Fixes #54334 

>From 8bae39299284dffd592bbf32374929e4a6f2d3ff Mon Sep 17 00:00:00 2001
From: Oleksandr T 
Date: Wed, 4 Dec 2024 02:24:12 +0200
Subject: [PATCH] [clang-format] extend clang-format directive with options to
 prevent formatting for one line

---
 clang/include/clang/Format/Format.h   | 11 +++-
 clang/lib/Format/DefinitionBlockSeparator.cpp |  3 +-
 clang/lib/Format/Format.cpp   | 57 +--
 clang/lib/Format/FormatTokenLexer.cpp | 39 ++---
 clang/lib/Format/FormatTokenLexer.h   |  8 ++-
 .../Format/IntegerLiteralSeparatorFixer.cpp   |  4 +-
 clang/lib/Format/SortJavaScriptImports.cpp| 10 +++-
 clang/lib/Format/TokenAnnotator.cpp   |  4 +-
 clang/unittests/Format/FormatTest.cpp | 53 +
 9 files changed, 154 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6383934afa2c40..b25d190178247d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -5620,8 +5620,15 @@ inline StringRef 
getLanguageName(FormatStyle::LanguageKind Language) {
   }
 }
 
-bool isClangFormatOn(StringRef Comment);
-bool isClangFormatOff(StringRef Comment);
+enum class ClangFormatDirective {
+  None,
+  Off,
+  On,
+  OffLine,
+  OffNextLine,
+};
+
+ClangFormatDirective parseClangFormatDirective(StringRef Comment);
 
 } // end namespace format
 } // end namespace clang
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp 
b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 319236d3bd618c..709ad5e776cc5a 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -144,7 +144,8 @@ void DefinitionBlockSeparator::separateBlocks(
 return false;
 
   if (const auto *Tok = OperateLine->First;
-  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
+  Tok->is(tok::comment) && parseClangFormatDirective(Tok->TokenText) ==
+   ClangFormatDirective::None) {
 return true;
   }
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index dcaac4b0d42cc5..e82ac41e652839 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -3267,9 +3267,9 @@ tooling::Replacements sortCppIncludes(const FormatStyle 
&Style, StringRef Code,
 
 bool IsBlockComment = false;
 
-if (isClangFormatOff(Trimmed)) {
+if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::Off) {
   FormattingOff = true;
-} else if (isClangFormatOn(Trimmed)) {
+} else if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::On) 
{
   FormattingOff = false;
 } else if (Trimmed.starts_with("/*")) {
   IsBlockComment = true;
@@ -3452,9 +3452,9 @@ tooling::Replacements sortJavaImports(const FormatStyle 
&Style, StringRef Code,
 Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
 StringRef Trimmed = Line.trim();
-if (isClangFormatOff(Trimmed))
+if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::Off)
   FormattingOff = true;
-else if (isClangFormatOn(Trimmed))
+else if (parseClangFormatDirective(Trimmed) == ClangFormatDirective::On)
   FormattingOff = false;
 
 if (ImportRegex.match(Line, &Matches)) {
@@ -4190,24 +4190,45 @@ Expected getStyle(StringRef StyleName, 
StringRef FileName,
   return FallbackStyle;
 }
 
-static bool isClangFormatOnOff(StringRef Comment, bool On) {
-  if (Comment == (On ? "/* clang-format on */" : "/* clang-format off */"))
-return true;
+static unsigned skipWhitespace(unsigned Pos, StringRef Str, unsigned Length) {
+  while (Pos < Length && isspace(Str[Pos]))
+++Pos;
+  return Pos;
+}
 
-  static const char ClangFormatOn[] = "// clang-format on";
-  static const char ClangFormatOff[] = "// clang-format off";
-  const unsigned Size = (On ? sizeof ClangFormatOn : sizeof ClangFormatOff) - 
1;
+ClangFormatDirective parseClangFormatDirective(StringRef Comment) {
+  size_t Pos = std::min(Comment.find("/*"), Comment.find("//"));
+  unsigned Length = Comment.size();
+  if (Pos == StringRef::npos)
+return ClangFormatDirective::None;
 
-  return Comment.starts_with(On ? ClangFormatOn : ClangFormatOff) &&
- (Comment.size() == Size || Comment[Size] == ':');
-}
+  Pos = skipWhitespace(Pos + 2, Comment, Length);
+  StringRef ClangFormatDirectiveName = "clang-format";
 
-bool isClangFormatOn(StringRef Comment) {
-  return isClangFormatOnOff(Comment, /*On=*/true);
-}
+  if (Comment.substr(Pos, ClangFormatDirectiveName.size()) ==
+  ClangFormatDirectiveName) {
+Pos =
+skipWhitespace(Pos + ClangFormatDirectiveName.size(), Comment, Length);
+
+unsigned EndDirectiveValuePos = Pos;
+while (EndDirectiveValuePos < Length) {
+  char Char = Co