Skip to content

[Style] stylua format entire codebase#7199

Open
keithharvey wants to merge 2 commits into
beyond-all-reason:masterfrom
keithharvey:fmt
Open

[Style] stylua format entire codebase#7199
keithharvey wants to merge 2 commits into
beyond-all-reason:masterfrom
keithharvey:fmt

Conversation

@keithharvey

@keithharvey keithharvey commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

Part of BAR type-error cleanup. Rebuilds idempotently from master via just bar::fmt-mig-generate.

What it does: Runs stylua over the entire Lua tree, applying the repo style (indentation, quotes, call parens, line width). Formatting only — no behavioral change.

# fmt - run stylua across the entire Lua codebase
stylua .

Branch Topology

All branches in the BAR type-error cleanup stack. Regenerated deterministically by just bar::fmt-mig-generate. Generated 2026-06-11 10:24:44 UTC.

Leaves — each targets master, mergeable independently:

Branch Command Diff vs parent Units
fmt stylua 1450 files, +183025 −187662 ✅ pass
mig-bracket bar-lua-codemod bracket-to-dot 342 files, +7653 −7653 ✅ pass
mig-rename-aliases bar-lua-codemod rename-aliases 173 files, +362 −362 ✅ pass
mig-detach-bar-modules bar-lua-codemod detach-bar-modules 178 files, +1694 −1562 ✅ pass
mig-i18n bar-lua-codemod i18n-kikito 20 files, +121 −1198 ✅ pass
mig-spring-split bar-lua-codemod spring-split 782 files, +10019 −10010 ✅ pass
mig-integration-tests <hand curated> 22 files, +140 −90 ✅ pass
mig-busted-types <hand curated> 12 files, +1501 −0 ✅ pass

Rollups — composite branches stacking the leaves and (for fmt-llm) the env + LLM layers:

Branch Diff vs master Diff vs parent Units
mig 1480 files, +193205 −197222 897 files, +21353 −20733 ✅ pass
fmt-llm-source 1486 files, +193710 −197492 55 files, +575 −340 ✅ pass
fmt-llm 1487 files, +193804 −197514 51 files, +117 −45 ✅ pass

@github-actions

github-actions Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Integration Test Results

14 tests  ±0   6 ✅ ±0   3s ⏱️ ±0s
 1 suites ±0   8 💤 ±0 
 1 files   ±0   0 ❌ ±0 

Results for commit 9e54b33. ± Comparison against base commit afaebb3.

♻️ This comment has been updated with latest results.

Comment thread spec/builder_specs/spring_synced_builder_spec.lua Outdated
Comment thread README.md Outdated
Comment thread luarules/gadgets/map_atmosphere_cegs.lua Outdated
Comment thread luaui/configs/badwords.lua
Comment thread luaui/Widgets/gfx_deferred_rendering_GL4.lua
Comment thread luaui/Widgets/gfx_distortion_gl4.lua
Comment thread luaui/Widgets/gui_attackrange_gl4.lua Outdated
Comment thread modules/lava.lua Outdated
Comment thread .stylua.toml
@keithharvey

keithharvey commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator Author

Problem

lx fmt is only formatting src/, spec/ directories because that is what it expects a lua package to look like (kind of dumb imo).

Notably lx --lua-version 5.1 lint does not have this problem and passes the entire directory.

Solution

lx --lua-version 5.1 exec stylua . {{args}} is what we have to do instead to run against the world

or
just bar::fmt over in BAR-Devtools

Problem here is that introduced a LOT of changes. I was surprised how few changes there were the first time so that explains it.

just bar::lint passes now

...
Checking /var/home/daniel/code/Beyond-All-Reason/weapons/spybombx.lua OK
Checking /var/home/daniel/code/Beyond-All-Reason/weapons/treefire.lua OK

Total: 0 warnings / 0 errors in 2199 files

@WatchTheFort WatchTheFort marked this pull request as draft March 24, 2026 05:59
@WatchTheFort WatchTheFort changed the title Fmt Initial linting of entire codebase Mar 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't linting something we want to run on the dev's machine before they push, rather than as an action after?

@keithharvey keithharvey Mar 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The README contains recommendations to set up a github hook. We can also add that to the setup script over in BAR-Devtools. This is more to catch people that didn't do that.

  • I should probably add a recommendation for vscode setup and extensions to auto format as well
  • Another option would be to auto-format when it hits the CI, that's less contrib friction but potentially surprising. I would just force the hook by catching them with the linter for now but I'm open to just doing it in the CI too. Maybe for now adding a github workflow that CAN format for us, so we can just nuke PRs from orbit as a one-off with a stylua pass wouldn't be a terrible migration strategy here.

Edit:
@WatchTheFort I did both of these things, but most relevant to you is that I added a workflow that you can run against arbitrary PRs (untested).

Comment thread .github/workflows/lint.yml Outdated
Comment thread .stylua.toml Outdated
@@ -1,13 +1,11 @@
syntax = "All"
column_width = 2000
column_width = 120

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this number come from? The solution for excessively long lines is better formatting, or better code architecture, not arbitrarily splitting across multiple lines.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this number come from?

120 is stylua's own default. Industry standard range is 80-160; 120 sits in the middle.

The solution for excessively long lines is better formatting

This is better formatting -- it's automated formatting.

or better code architecture

That's a stylistic judgment call, which is exactly what we're trying to get out of the business of making manually. A formatter with a column width handles it consistently so developers don't have to debate it in review.

Having a max line length beats not having one. Without it, lines grow unchecked, diffs get wider (more merge conflicts), and side-by-side review becomes painful. 120 gives a concrete answer to "how long is too long" so we stop relitigating it per-PR. If a specific line reads better long, -- stylua: ignore above it opts out.

Comment thread common/configs/unit_restrictions_config.lua Outdated
Comment thread common/luaUtilities/base64.lua
Comment thread common/springUtilities/tableFunctions.lua Outdated
Comment thread common/springUtilities/teamFunctions.lua
Comment thread common/testing/assertions.lua Outdated

@efrec efrec left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nibbles

Comment thread common/configs/unit_restrictions_config.lua Outdated
Comment thread common/configs/unit_restrictions_config.lua Outdated
Comment thread common/springUtilities/color.lua Outdated
Comment thread common/springUtilities/color.lua
Comment thread common/springUtilities/debug.lua
Comment thread common/springUtilities/debug.lua
Comment thread common/springUtilities/image_tga.lua
@keithharvey

keithharvey commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator Author

Related PR Comments

@WatchTheFort @efrac -- closing thoughts on #7199 for tonight.

Why we're not hand-editing this PR

We don't want to deviate from the generated output any more than we already have. Right now this branch is: CI changes, a format command, and a few targeted manual fixes (the -- stylua: ignore comments for those unreachable-code test blocks, the empty do/end header). The further we drift from that, the faster we have to merge it, because this thing will be merge conflict central for every in-flight branch. But if we keep it close to a pure stylua . pass, anyone can re-run the formatter on their branch and conflicts just go away. Comment cleanup, bracket-to-dot refactors, engine globals -- all good ideas, all follow-up PRs after this one lands.

PR comment threads

I've resolved the threads I consider addressed. If you disagree on any particular closure, please reopen the ones you think still need discussion. Otherwise I'll take silence as agreement.

Migration guide for contributors with in-flight branches

Here's the playbook. I just did this with sharing_tab (3 commits, 100+ files, ~37 conflicts) and it took about 2 minutes.

Step 1: rebase onto master (after #7199 merges)

git fetch origin
git rebase origin/master

Step 2: at each conflict, take your version and re-format

# for pure formatting conflicts (most of them), take yours:
git checkout --theirs <conflicted-files>

# for files your branch deletes:
git rm <file>

# then re-run stylua on everything you touched:
stylua <conflicted-files>
# or if you use BAR-Devtools:
just bar::fmt

# stage and continue:
git add -A
git rebase --continue

If you're confident all your conflicts are formatting-only (no overlapping logic changes with master), you can batch the whole thing:

# ⚠️  only safe when ALL conflicts are pure formatting, not semantic
git diff --name-only --diff-filter=U | xargs git checkout --theirs --
stylua .
git add -A
git rebase --continue

That's it. The formatter is idempotent -- your logical changes survive and the formatting matches master.

If you use BAR-Devtools: just bar::fmt wraps the same stylua . pass, and just bar::setup-hooks installs a pre-commit hook so you never push unformatted code again. If you want to set up stylua on your host machine directly instead, see the "Manual Tooling Setup" section in the BAR README.

Caveat: the BAR-Devtools recipes (bar::fmt, bar::setup-hooks) exist locally but haven't been pushed to thvl3's upstream yet -- @thvl3 has been AFK. Until those land, run lx fmt or stylua . directly.

@keithharvey

keithharvey commented Mar 25, 2026

Copy link
Copy Markdown
Collaborator Author

Also added .git-blame-ignore-revs so this commit is transparent in git blame. GitHub picks it up automatically. Locally it's one config line (git config blame.ignoreRevsFile .git-blame-ignore-revs).

@keithharvey keithharvey force-pushed the fmt branch 5 times, most recently from 2df6fa9 to 8c537e4 Compare March 27, 2026 05:28
@Ruwetuin

Copy link
Copy Markdown
Member

does #7229 now supercede this PR?

@keithharvey

keithharvey commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator Author

does #7229 now supercede this PR?

Kind of. That PR is a superset of all changes. This is a generative step in the migration process. 7229 is everything. It would be easier if we roll them out together probably, but each PR represents a different step. This PR is the style base.

fmt - 'stylua .'
^
Mig1 - dots conversion
^
Mig2 - spring split
<< mig branch

There's also transform leaf branches.

Fmt > mig1
> mig2

This allows you to preview each transform independent of the style changes.

All of this is kicked out automatically by a script for me so it shouldn't drift and we can add more transforms easily and see previews.

edit: I modified the script to kick out a topology so all of this is more clear. Also split the transform leaf PRs up into completely individual (I had put spring and the global rename together for convenience before all this was scripted)

{ 250, 3, 5*60 },
{ 415, 1.5, 3*30 },
{ 250, 3, 10*60 } },
tideRhythm = { { 250, 3, 15 }, { 415, 1.5, 30 }, { 250, 3, 5 * 60 }, { 415, 1.5, 30 }, { 250, 3, 5 * 60 }, { 415, 1.5, 3 * 30 }, { 250, 3, 10 * 60 } },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tables should still be formatted with entries on separate lines

@keithharvey keithharvey force-pushed the fmt branch 4 times, most recently from 0e99a58 to b427120 Compare April 22, 2026 18:12
@keithharvey keithharvey force-pushed the fmt branch 4 times, most recently from 888b0a1 to 6ecfd7b Compare April 29, 2026 00:17
@keithharvey keithharvey marked this pull request as ready for review April 29, 2026 18:08
@keithharvey keithharvey force-pushed the fmt branch 8 times, most recently from c6c8713 to 078f9f8 Compare April 30, 2026 07:27
@keithharvey keithharvey force-pushed the fmt branch 3 times, most recently from 9f46d38 to c2fa433 Compare June 10, 2026 03:09
…ring

Renames spec/builders/{spring->engine}_{,un}synced_builder.lua + their
builder_specs, Builders.Spring->Builders.EngineSynced,
Builders.SpringUnsynced->Builders.EngineUnsynced, @Class names, and all
call sites. The renamed builders own every Engine-related test edit so no
two prereqs touch one file:
  - engine_synced_builder.lua: gamedata/system.lua defs mock aliases
    Engine.{Shared,Synced,Unsynced} + BAR to the _G.Spring/_G.BAR mocks
  - engine_unsynced_builder.lua: widget sandbox gets env.Engine; capture
    spies install on Engine.Shared.*

Prefix branch -> lands in fmt once, every leaf+mig inherits it.

Recovered from origin/mig-spring-split@4e25bfe82a (branch lost in a rename);
files transplanted verbatim (formatting is normalized by run_fmt).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants