In D85600#2209545, @Typz wrote:

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

Regarding ?:, this is definitely not considered, and should be relatively easy to handle.

Indeed, I saw the emails, but I didn't have time to check or investigate the issues.

In D50078#2055227, @srj wrote:

And I also think it should be on by default instead of modifying many .clang-format files. So IMHO if we add an option, it should be opt-out.

I can live with it being opt-out. My big concern is that (as it currently stands) our project gets different 'canonical' clang-format results depending on whether people are using LLVM10 vs LLVM11 (and both are common and supported by our project), which is a headache. I just want a way to get consistent, predictable formatting regardless of the clang-format version.

In D50078#2050623, @MyDeveloperDay wrote:

I get the point about Opt In, and if we are going to add an option it needs to go in ASAP otherwise too many people will then start complaining it was on by default before.

To be honest I think the question is do we consider this a bug or a feature, because we DO NOT provide Opt In on bug fixes (and there is no Opt Out).

If its a bug, there is no need for us to not have this on by default, its better to drive the improvement than 10,000's of .clang-format files needing to be updated just to get reasonable behavior.

Fix random crash on windows

Still no chance getting a windows or s390x machine, but i could reproduce the/a problem systematically on linux by enabling sanitizer.
In this case, initializing ConditionalsLevel to 0 in constructor does index fix this issue (and initializing to 1 ensures the issue happens, though not in "regular" linux build).

Thanks for trying the patch, too bad it does not work...
I will see how to get the required setup/env, but it will probably take some time (esp. in the current situation).
Thanks again

In D50078#1998578, @hokein wrote:

In D50078#1998520, @Typz wrote:

In D50078#1998398, @dyung wrote:

Hi, this change that you submitted in commit 5daa25fd7a184524759b6ad065a8bd7e95aa149a seems to be causing the test "Clang-Unit :: Format/./FormatTests.exe/FormatTest.ConfigurableUseOfTab" to randomly fail about 50% of the time on Windows.

Take the most recent 5 runs of the llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast buildbot. Builds #31973, 31975 and 31976 all failed because of that test failing.

On my local Windows computer, if I build your commit and run the FormatTest unit test multiple times (the same binary without changing anything or rebuilding), that above test fails roughly 50% of the time.

Can you take a look to figure out why your change might be causing this instability?

Hi,
That is very strange indeed, nothing is supposed to be random here...
I will try to have a look, but I don't have a windows computer to test, and never seen this issue on linux/macos :-/

The linux (s390x arch) buildbot is also failing, http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/31811/steps/ninja%20check%201/logs/stdio.
I reverted it in 47ef09e4848a970c530928496b54085cfdba5a76 to make buildbot happy.

In D50078#1998398, @dyung wrote:

Hi, this change that you submitted in commit 5daa25fd7a184524759b6ad065a8bd7e95aa149a seems to be causing the test "Clang-Unit :: Format/./FormatTests.exe/FormatTest.ConfigurableUseOfTab" to randomly fail about 50% of the time on Windows.

Take the most recent 5 runs of the llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast buildbot. Builds #31973, 31975 and 31976 all failed because of that test failing.

On my local Windows computer, if I build your commit and run the FormatTest unit test multiple times (the same binary without changing anything or rebuilding), that above test fails roughly 50% of the time.

Can you take a look to figure out why your change might be causing this instability?

In D50147#1648786, @sammccall wrote:

First and forehand, I have a problem to solve in my organization : we have many projects, and maintaining the config file in some many repositories is not practical.
In some case (like, LLVM, google or mozilla), this is not an issue because the formatting rules are hard-coded in clang-format.

I think this option is available to anyone with a public style guide that covers a reasonably large body of open-source code, and which can be reasonably well supported by clang-format.
I'd expect QT, KDE etc should be able to use this mechanism.

Rebase on top of D50078

Implement a fallback to regularly indented alignment when the question mark is wrapped.

Sorry, I struggled a bit with git & phabricator, and ended up uploading non-working code :-/

Fix earlier error in patch upload.

Fix corner in previous rebase

Rebased on master, integrating required code from https://reviews.llvm.org/D32478

This actually depends on another Diff : https://reviews.llvm.org/D32478
That other change introduces the ability to "unindent operator", which is required here; however, it also changes AlignOperands to have more than 2 modes, which does not seem to be acceptable.

ping?

Rebase

Rebase

In D50147#1533337, @sammccall wrote:

One thing that's unclear to me is whether your aim is to

1. solve a concrete problem for your organization
2. solve a family of problems for similar organizations
3. add a new way of configuring styles for many types of users/projects

Rebase

ping?

ping ?

ping?

• Rebase
• Fix NamespaceEndCommentsFixerTest to match LLVM indent style

• Rebased
• Adapt styles search path to platform conventions
• Allow customizing search path at compile time
• Allow overriding search path at runtime through CLANG_FORMAT_STYLES_PATH env variable

Ah, wow, I see what confused me now:
In the fixNamespaceEndComment tests you use an indent that clang-format would not produce. Can you please fix that, otherwise I find this super confusing :)

In D50147#1310146, @sammccall wrote:

In D50147#1309880, @Typz wrote:

ping?

Sorry for losing track of this.

I think -style=<filename> is a logical extension of the current options. I'm less sure of supporting it in BasedOnStyle, but happy to go either way.

Actually, looking at all the other options, the same is true for all other macro kind of macros (NamespaceMacros...).

Regarding variable/setting names, one issue I see is that these macros are not really type names IMO : the macro name is just part of the type (like a template), and its invocation is a type declaration. So maybe renaming to Typedecl or something like this would be slightly more representative.

ping ?

The patch goal is indeed to indent the content of namespace-macro blocks like the content of any 'regular' namespace. So it should look like the content of a namespace, possibly depending on the choose style options. To sumarize, here is a detailed summary of the observable differences between macros vs normal namespace.

ping ?

ping ?

ping?

ping?

In D50147#1272742, @sammccall wrote:

Being able to discover the right style from the filesystem is powerful, and if I was going to use this flag, I'd consider symlinking the style-file to subproject/.clang_format instead. That way the setting is persistent and available to all tools, rather than us needing to get it right on each invocation.

In D50147#1272742, @sammccall wrote:

The idea here does seem to be a natural extension of -style, at least for the case when the arg is a filename directly. I'm not opposed, happy to review this.

I do want to probe the use case a bit though: we have found that configuring via -style= doesn't scale that well to lots of codebases and tools. e.g. if someone's running clang-tidy or clangd via some driver, are they going to have a way to plumb through the right -style=?

Being able to discover the right style from the filesystem is powerful, and if I was going to use this flag, I'd consider symlinking the style-file to subproject/.clang_format instead. That way the setting is persistent and available to all tools, rather than us needing to get it right on each invocation.

In D37813#1254865, @klimek wrote:

In D37813#1253813, @Typz wrote:

In D37813#1184051, @klimek wrote:

The problem here is that we have different opinions on how the formatting on namespace macros should behave in the first place- I think they should behave like namespaces, you want them to be formatted differently.
Given that you want full control over the formatting of those macros, and them not actually be formatted exactly like namespaces or classes, I think we need a more generic mechanism for you to express that.

Not sure what you mean here. I want them to behave like namespaces as well, this is actually the use case I have... As implemented, they indeed behave exactly like namespaces :

TESTSUITE(a) {                       namespace a {
} // TESTSUITE(a)                    } // namespace a
                                VS
TESTSUITE(a) { TESTSUITE(b) {        namespace a { namespace b {
} // TESTSUITE(a::b)                 }} // namespace a::b

I thought we had the discussion upthread that they indent differently from namespaces. I'm working on a patch that allows you to configure macros.

In D37813#1184051, @klimek wrote:

The problem here is that we have different opinions on how the formatting on namespace macros should behave in the first place- I think they should behave like namespaces, you want them to be formatted differently.
Given that you want full control over the formatting of those macros, and them not actually be formatted exactly like namespaces or classes, I think we need a more generic mechanism for you to express that.

In D50147#1244237, @krasimir wrote:

I don't understand the use-case this patch is realizing. Suppose I have a bunch of projects that have to share a format style. Then I can checkout them under a common root directory and put an appropriate .clang-format file there.

clang-format does indeed support .clang-format, which is great for *isolated* projects, and which is not affected by this patch.

In D50078#1184159, @krasimir wrote:

Could you clarify how each piece is supposed to be aligned in these examples?
This is what makes me happy:

// column  limit             V
int a = condition1 ? result1
      : conditio2 ? result2
      : loooooooooocondition 
        ? result2
        : dition3 ? resul3
                  : resul4;

// column  limit             V
int a = condition1 
        ? loooooresult1
        : conditio2 ? result2
                    : result4;

In D50078#1184015, @djasper wrote:

• I'd be ok with the suggestion for BreakBeforeTernaryOperators=true

This has been stale for a while now. Last comment indicate this is not the right approach, but some prototyping has been done a more generic approach, with no timeline.
I understand this, but in the meantime the problem is still there, with no solution at the moment...

Regenerate documentation

Regeneration documentation

Regenerate documentation

Notes:

• I choose not to add an option to enable this behavior, as I think of it as just another way clang-format can (better) format the code; but I can one if needed
• Currently, it relies on another patch (D32478), which supports aligning the wrapped operator slightly differently. If/since that other patch does not seem to make it, I can change this patch to either do the alignment in this specific case (e.g. for wrapped ternary operator only) or to keep the 'default' behavior of clang-format (e.g. the wrapped colon would be aligned with the first condition):

rebase

rebase

Add missing block, for consistency

In D42787#1127790, @djasper wrote:

The normal rule for formatting options apply. If you can dig up a public style guide and a project of reasonable size where it is used, we can add an option.

rebase

Would it be acceptable to introduce an option to allow enabling this behavior?
I mean would it have a chance of being integrated, or must I keep maintaining a fork of clang-format...

ping?

ping?