Skip to content

clang-format: Disabling Formatting on a one line #54334

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

Closed
VasaMM opened this issue Mar 11, 2022 · 27 comments · Fixed by #137577
Closed

clang-format: Disabling Formatting on a one line #54334

VasaMM opened this issue Mar 11, 2022 · 27 comments · Fixed by #137577
Assignees

Comments

@VasaMM
Copy link

VasaMM commented Mar 11, 2022

Often I need disable formatting only on one line. In this case it is annoying write // clang-format off above line and // clang-format on below line. And there is room for error also. It would be nice, if it will be exist some inline command for disabling.

@mkurdej mkurdej added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature clang-format and removed new issue labels Mar 11, 2022
@llvmbot
Copy link
Member

llvmbot commented Mar 11, 2022

@llvm/issue-subscribers-clang-format

@mydeveloperday
Copy link
Contributor

I've seen this requested before, do you have a suggestion?

@VasaMM
Copy link
Author

VasaMM commented Mar 22, 2022

We were thinking about it. What about //< clang-format off?

E.g.:

...
int x = 0;
usnigned int y = 1; //< clang-format off
int z = 2;
...

instead of

...
int x = 0;
//< clang-format off
usnigned int y = 1;
//< clang-format on
int z = 2;
...

@zhouzilong2020
Copy link

Hi, Is there any update on this?

@jamiecook
Copy link

Would also love to see this and the syntax proposed by @VasaMM seems quite tidy

@rockwotj
Copy link
Contributor

Another option similar to other tools (clang tidy) is:

// clang-format off-next-line

@vsolontsov-ll
Copy link

vsolontsov-ll commented Mar 17, 2024

Let me also vote for this extension. Both are really nice to have:

  1. clang-format off-next-line
  2. clang-format off-current-line

I would love to see some shorter magic phrase or maybe allow configuring some acronyms (CFONL / CFOCL)

@mauro-balades
Copy link

anything working on?

@ProExpertProg
Copy link

Should this be marked as a good first issue?

@IHorvalds
Copy link

If no one else is working on this, can I pick it up?

@IHorvalds
Copy link

Sorry I haven't said anything in 3 weeks. I don't think I can work on this very soon.
If anyone else wants to work on this, it's open.

@a-tarasyuk
Copy link
Member

I've opened a draft PR adding support for two options: // clang-format off-line and // clang-format off-next-line. It's still a work in progress as more edge cases need to be covered...

@mydeveloperday
Copy link
Contributor

What are the use cases where you would use this? what is the reason its needed?

@b1ackviking
Copy link

@mydeveloperday one common use case I see is disabling formatting (column limit) for logs. The codebase I currently work on has lots of simple (5-15 lines) functions that are bloated with outputting logs. For example,

int foo(Resources *resources, int i, int j, int k) {
  if (i < 0 && j < 0) {
    myproject::detail::LogErrorPrintf(
        resources->logger,
        "both i and j can not be negative at the same time.\ni = %d, j = %d\n",
        i, j);
    return -1;
  }

  if (i < 0) {
    j *= 10;
  }
  if (j < 0) {
    k += 5;
  }
  return i + j * k;
}
int foo(Resources *resources, int i, int j, int k) {
  if (i < 0 && j < 0) {
    // clang-format off-next-line
    myproject::detail::LogErrorPrintf( resources->logger, "both i and j can not be negative at the same time.\ni = %d, j = %d\n", i, j);
    return -1;
  }

  if (i < 0) {
    j *= 10;
  }
  if (j < 0) {
    k += 5;
  }
  return i + j * k;
}

IMO, the second case is more readable that the first one.

@vsolontsov-ll
Copy link

vsolontsov-ll commented Dec 13, 2024

What are the use cases where you would use this? what is the reason its needed?

It's very helpful to bypass missing features or bugs in clang-format. E.g. I missed this feature when there were no support for C++ concepts. It just did something odd with our style configuration.

Also in C when you need a compile-time constant it could be useful to locally define anonymous enumeration: enum { the_answer = 42 };. For regular enums a team may decide that line-breaks are mandatory. But for defining stand-alone compile time constants it's definitely an overkill stretching it to 2-3 lines.

This is definitely not complete. Just a couple of examples from the top of my head.

@a-tarasyuk
Copy link
Member

@mydeveloperday @owenca @HazardyKnusperkeks Would closing the issue with a wontfix status be appropriate?

@HazardyKnusperkeks
Copy link
Contributor

I think there is some discussion to do.

@owenca owenca removed the enhancement Improving things as opposed to bug fixing, e.g. new or missing feature label Jan 2, 2025
@a-tarasyuk
Copy link
Member

I've closed the PR due to concerns about the usefulness of this feature. If there are beneficial use cases for supporting these new options, please feel free to share them here to highlight their advantages.

@Sedeniono
Copy link
Contributor

Sedeniono commented Apr 11, 2025

We have a use case: We run a separate scanner tool over our source code that extracts string literals for translation into different languages (e.g. Italian, German, etc.). Some string literals do not need any translation because they are not part of any message a user sees. We need to mark lines containing such string literals with some special trailing comment: // NO_TRANSLATION as instruction to the scanner. The problem with clang-format is that if the line is very long and there are additional symbols after the literal, it moves the NO_TRANSLATION to a different line. For example:

std::string const msg = fmt::sprintf("Long string with %s placeholders and %d more.", someStr, someInt); // NO_TRANSLATION

clang-format often ends up breaking the additional arguments and the comment to the next lines, e.g.:

std::string const msg = fmt::sprintf(
   "Long string with %s placeholders and %d more.",
   someStr, someInt); // NO_TRANSLATION

This then causes the string literal to end up in the translation database because the scanner tool incorrectly extracts the literal.

This is one of the major blockers that prevents us from formatting documents on save in the IDE, or running clang-format over the whole code basis. Manually fixing the position of NO_TRANSLATION afterwards would be a lot of work (our code base has several million lines of code). Also, there are some additional different "comment tags" besides NO_TRANSLATION that give specific instructions to the phrases scanner.

If there were a clang-format option to tell clang-format to leave lines alone that contain certain strings/regexes, it would solve the issue for us. So it would be great if the suggested clang-format off-line would be configurable, i.e. if the user could specify multiple strings/regexes.

I have a little bit of familiarity with the clang-format code base and would be willing to invest some work to implement the feature or add to the existing pull request #118566.

Notes:

  • We also thought about replacing the NO_TRANSLATION comments with a user-defined string literal, which would solve the problem in C++, but C# does not offer such a language feature, so we would still have the issue there.
  • The existing option CommentPragmas instructs clang-format to leave just the comments alone, rather than the whole line, so we still have the issue the comment getting moved to another line.

@vsolontsov-ll
Copy link

Another use-case (actually we regularly face with different, they are all relatively minor but very annoying).

In C before C23 the best way to define a compile-time integral constant is enum:

enum { my_magic_constant = 24 };

Meanwhile the general code-style prescribes to put enumerators on its own line and more than this, use "non-Egypt" braces like:

enum 
{ 
    my_magic_constant = 24 
};

Obviously it inflates the code and makes it less readable.

@a-tarasyuk
Copy link
Member

@Sedeniono Thanks for sharing the additional use cases.

I have a little bit of familiarity with the clang-format code base and would be willing to invest some work to implement the feature or add to the existing pull request

The closed PR can be revisited and used as a starting point for this feature. However, it seems that the main concern is whether this feature is necessary, given the existing // clang-format on/off options or the ability to refine formatting rules for specific cases. So, I’m still not confident about it., maybe #54334 (comment) is still relevant...

@Sedeniono
Copy link
Contributor

@a-tarasyuk Yes, I hope that my use case convinces the others that the possibility to disable the formatting via a single-line comment is a good idea. And also, to make the recognized "markers" configurable.
What do you think, @mydeveloperday, @owenca, @HazardyKnusperkeks?

@owenca
Copy link
Contributor

owenca commented Apr 26, 2025

We have a use case: We run a separate scanner tool over our source code that extracts string literals for translation into different languages (e.g. Italian, German, etc.). Some string literals do not need any translation because they are not part of any message a user sees. We need to mark lines containing such string literals with some special trailing comment: // NO_TRANSLATION as instruction to the scanner. The problem with clang-format is that if the line is very long and there are additional symbols after the literal, it moves the NO_TRANSLATION to a different line. For example:

std::string const msg = fmt::sprintf("Long string with %s placeholders and %d more.", someStr, someInt); // NO_TRANSLATION

clang-format often ends up breaking the additional arguments and the comment to the next lines, e.g.:

std::string const msg = fmt::sprintf(
   "Long string with %s placeholders and %d more.",
   someStr, someInt); // NO_TRANSLATION

This then causes the string literal to end up in the translation database because the scanner tool incorrectly extracts the literal.

Have you tried one of the following?

std::string const msg = fmt::sprintf( // NO_TRANSLATION
    "Long string with %s placeholders and %d more.", someStr, someInt);

std::string const msg = /* NO_TRANSLATION */ fmt::sprintf(
    "Long string with %s placeholders and %d more.", someStr, someInt);

std::string const msg = // NO_TRANSLATION
    fmt::sprintf("Long string with %s placeholders and %d more.", someStr,
                 someInt);

@a-tarasyuk Yes, I hope that my use case convinces the others that the possibility to disable the formatting via a single-line comment is a good idea. And also, to make the recognized "markers" configurable. What do you think, @mydeveloperday, @owenca, @HazardyKnusperkeks?

See #118566 (comment), #118566 (comment), and #118566 (comment).

@Sedeniono
Copy link
Contributor

Sedeniono commented Apr 26, 2025

Have you tried one of the following?

std::string const msg = fmt::sprintf( // NO_TRANSLATION
    "Long string with %s placeholders and %d more.", someStr, someInt);
std::string const msg = /* NO_TRANSLATION */ fmt::sprintf(
    "Long string with %s placeholders and %d more.", someStr, someInt);
std::string const msg = // NO_TRANSLATION
    fmt::sprintf("Long string with %s placeholders and %d more.", someStr,
                 someInt);

@owenca Our external phrases scanner tool requires the NO_TRANSLATION comment tag to be on the same line as the string literal "..." (rather than the sprintf function or the msg variable).

We could write a script that rewrites the several million lines of C++/C# code once to put all NO_TRANSLATION markers and the corresponding literals on their own lines, and then running clang-format over them. But future developers also have to take care to not accidentally write too long lines which would get broken without them noticing.
Or the script could introduce the // clang-format off/on markers above and below the NO_TRANSLATION phrases, but apart from blowing up the source code, we again have the issue that future developers need to be aware of the formatting issue.
The script could also rewrite the code to use a user-defined string literal instead of a comment tag, but this won't work in C#.

I do understand the hesitance to add the proposed functionality. It just would have been a simple way for us to really ensure that the introduction of automatic formatting doesn't mess up our translation database, neither for existing nor future code. But there are certainly alternatives as outlined above.

@owenca
Copy link
Contributor

owenca commented Apr 27, 2025

I've come up with the following for maximum flexibility and minimum interference with // clang-format off:

  • A new option OneLineFormatOffRegex.
  • Turns off formatting for the current line if any token matches the regex, except:
  • If a line comment that's the first/ only token of the line matches the regex, turns off formatting for the next line.

@owenca owenca self-assigned this Apr 27, 2025
@owenca
Copy link
Contributor

owenca commented Apr 27, 2025

Sample output:

$ cat a.cc
 int x = 0;
usnigned int y= 1; //< clang-format off
int z =2;
//< clang-format off
float  a = 0.1;
double b = 2.3 ;
myproject::detail::LogErrorPrintf( resources->logger, "both i and j can not be negative at the same time.\ni = %d, j = %d\n", i, j);
std::string const msg = fmt::sprintf("Long string with %s placeholders and %d more.", someStr, someInt); // NO_TRANSLATION
$ cat .clang-format
OneLineFormatOffRegex: //< clang-format off|LogErrorPrintf|// NO_TRANSLATION
$ clang-format a.cc
int x = 0;
usnigned int y= 1; //< clang-format off
int z = 2;
//< clang-format off
float  a = 0.1;
double b = 2.3;
myproject::detail::LogErrorPrintf( resources->logger, "both i and j can not be negative at the same time.\ni = %d, j = %d\n", i, j);
std::string const msg = fmt::sprintf("Long string with %s placeholders and %d more.", someStr, someInt); // NO_TRANSLATION
$ 

@Sedeniono
Copy link
Contributor

I've come up with the following for maximum flexibility and minimum interference with // clang-format off:

  • A new option OneLineFormatOffRegex.
  • Turns off formatting for the current line if any token matches the regex, except:
  • If a line comment that's the first/only token of the line matches the regex, turns off formatting for the next line.

@owenca That sounds great! 👍

owenca added a commit to owenca/llvm-project that referenced this issue Apr 28, 2025
owenca added a commit to owenca/llvm-project that referenced this issue Apr 28, 2025
@owenca owenca closed this as completed in b8bb1cc Apr 30, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this issue May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this issue May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this issue May 6, 2025
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this issue May 7, 2025
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this issue May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment