Tightening the Clang-tidy and Clang-format checks

I need more eye balls to verify the automated bulk update results on my current active dev branch.

This is only the initial bulk update. More will come until everything snaps into place; or not if I failed, and these two jobs that use clang-tidy and clang-format will be removed.

This initial bulk update was from clang-format command only. Let me know if you see anything that is not acceptable for the Urho3D project. Just to be clear, I am not asking for your personal setting.

camera->SetZoom(1.2f * Min((float)graphics->GetWidth() / 1280.0f,
                               (float)graphics->GetHeight() /
                                   800.0f)); // Set zoom according to user's resolution to ensure full visibility
                                             // (initial zoom (1.2) is set for full visibility at 1280x800 resolution)

When the long parameters line is wrapped then it will “align” the next line. There is also option to “not align”. I am OK with both. Currently it is “align” I think. Is that what you objecting?

For future, please also provide the name of the file for easy lookup.

DynamicGeometry::CreateScene()

corrupted table float vertexData[]

void L10n::HandleChangeLanguage(StringHash eventType, VariantMap& eventData)
windowTitle->SetText(l10n->Get("title") + " (" + String(l10n->GetLanguageIndex()) + " " + l10n->GetLanguage() +
                         ")");

Source/Samples/40_Localization/L10n.cpp

This is the same issue. It will wrap when the column limit 120 is hit. The number is configurable. Can set to 0 for don’t wrap. I think it was 80 originally and I changed it to 120. But this wrapping can happen on any number we choose. Unless, if we set it to 0.

Source/Samples/47_Typography/Typography.cpp

corrupted tables const char* levels[] thresholds[] limits[]

void Urho2DStretchableSprite::ScaleSprites(float timeStep)

        const auto scale = Vector2{1.0f + (right  ? quantum
                                           : left ? -quantum
                                                  : 0.0f),
                                   1.0f + (up     ? quantum
                                           : down ? -quantum
                                                  : 0.0f)};

Source/Urho3D/LibraryInfo.cpp

wrong indents in GetCompilerDefines()

what wrong about this wrapping? It is kind of nice :slight_smile: If you want may be I could retry without alignment too.

Everywhere, line fragments start with the same indentation, but here it is different

So far the only thing that looks weird and foreign for me is non-multiple-of-4 indentation.
As far as I remember, Urho have never (or almost never) used this kind of formatting in its codebase.

In this particular example it looks good, but in general it tends to waste horizontal line space.
It’s also more annoying to format manually, which may or may not be an issue depending on the editor used by specific person.

Offtopic: I’m forced to use this style of formatting when I’m working with Python code. I don’t know if I like it or not.

TL;DR: I think that multiple-of-4-indentation is more in line with current Urho formatting, and I also personally prefer it due to reasons stated above.

Yes, that the same issue 1vanK is highlighting. It wraps with alignment taking consideration where the previous parameter starts.

OK. Let stop here first. Let me see if I could reconfigure the wrapping setting.

About the column limit, 120 is OK with you guys?

I would do with no limit at all

1 Like

I am contemplating with that option too. I have LG ultra wide screen monitor, so personally I don’t have any issue with that. Anyway, I don’t think we have too many places with extra long lines, so it should be fine.

++ for 120 column limit, we use the same setting on my dayjob. It really helps at diffs review. Although if you really want to keep it as is… well, no limit worked fine for 15 years.
++ for constructor initializer list refactoring
++ for uniform spacing all over the code

Unsure about alignment, I would prefer to keep 1-tab alignment.
I didn’t read whole commit, so maybe I didn’t notice other changes.

Yes, I agree the column limit is good for diffs review, and also for browsing the code through GitHub web interface.

The limit of 120 is large. Lines that exceed this limit do so for some reason. Do you really want to interfere with this? If you really want to set a high limit, then don’t set a limit at all.

120 symbols is not “huge” limit, it is exactly how much fits on Full HD monitor in Beyond Compare for 2-way diff. 120 symbols is also how much code fits on average laptop display with IDE side bar.

120 or 130 symbols is often used as middle ground between unlimited lines and 80-char limit from DOS epoch

80 is too little, especially if you want to apply it to legacy codebase that was written without this limit in mind.

No limit works okay as long as people writing code stay reasonable and keep their lines generally short.