Skip to content

Use tabs for indentation in embedded JSON#6103

Merged
qwerty287 merged 1 commit into
woodpecker-ci:mainfrom
qwerty287:indent-embed
Feb 18, 2026
Merged

Use tabs for indentation in embedded JSON#6103
qwerty287 merged 1 commit into
woodpecker-ci:mainfrom
qwerty287:indent-embed

Conversation

@qwerty287
Copy link
Copy Markdown
Contributor

My editor (and I guess any other also) is not working properly otherwise.

@qwerty287 qwerty287 added the tests related to tests or other things CI check before merge label Feb 10, 2026
@qwerty287 qwerty287 requested a review from a team February 10, 2026 16:00
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.66%. Comparing base (2ca6f58) to head (f1d154b).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6103   +/-   ##
=======================================
  Coverage   31.66%   31.66%           
=======================================
  Files         420      420           
  Lines       28413    28413           
=======================================
  Hits         8996     8996           
  Misses      18596    18596           
  Partials      821      821           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xoxys
Copy link
Copy Markdown
Member

xoxys commented Feb 10, 2026

This shouldnt be any problem. Embedded templates should use the common indentation of the target language in case of json this shoulf stay as spaces.

@6543
Copy link
Copy Markdown
Member

6543 commented Feb 10, 2026

what IDE do you use?

@qwerty287
Copy link
Copy Markdown
Contributor Author

I use Kate (KDE's text editor), does this work with different editors?

The editor can't know which file type this if it does not analyze the content itself. and this would also only work for common types.

@xoxys
Copy link
Copy Markdown
Member

xoxys commented Feb 11, 2026

I use Kate (KDE's text editor), does this work with different editors?

What does "work" means here? I can open the files in vscode/codium and auto-formatting on safe also works.

@qwerty287
Copy link
Copy Markdown
Contributor Author

qwerty287 commented Feb 11, 2026

By "work" I mean what happens when editing this embedded JSON data. Kate uses tabs then (obviously because the rest of the file uses tabs), while most lines use spaces. Yes, it works correctly for the Go code in the file. There already were many inconsistencies in the JSON, likely happened due to this issue. I don't think any editor handles this.

@xoxys
Copy link
Copy Markdown
Member

xoxys commented Feb 12, 2026

Correct you have to manually indent it. Feel free to switch to tabs for this particular case but thats not a global solution. In the autoscaler for example we had the same issue with the embedded cloudinit config but cloudinit is picky and fails on tab indened files. The best approach would be to use //go:embed instead of embedded multuiline data.

@qwerty287
Copy link
Copy Markdown
Contributor Author

For larger files yes, embedding is best, but having a new file for every 3-line json seems a bit overkill to me. I can change it though if you prefer that way

@xoxys
Copy link
Copy Markdown
Member

xoxys commented Feb 16, 2026

Also fine for me to go with this PR. Just wanted to say, this will not work in all cases and will not be enforced by linters etc. so we will end up with a mixed result again.

@qwerty287
Copy link
Copy Markdown
Contributor Author

So can we merge this then?

@qwerty287 qwerty287 merged commit b7c9429 into woodpecker-ci:main Feb 18, 2026
10 checks passed
@qwerty287 qwerty287 deleted the indent-embed branch February 18, 2026 12:20
@woodpecker-bot woodpecker-bot mentioned this pull request Feb 17, 2026
1 task
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 1, 2026
1 task
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 15, 2026
1 task
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 27, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests related to tests or other things CI check before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants