Skip to content
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

Preserve indentation when writing JSON files #11390

Merged
merged 3 commits into from Jul 19, 2023
Merged

Preserve indentation when writing JSON files #11390

merged 3 commits into from Jul 19, 2023

Conversation

maximal
Copy link
Contributor

@maximal maximal commented Mar 21, 2023

Refs #11341, doesn’t resolve specifically it though, but has something to move on.

UPD: I’ll fix PHPStan warnings if the PR is OK in general.

@Seldaek
Copy link
Member

Seldaek commented Mar 22, 2023

Yeah that seems reasonable to me

@Seldaek Seldaek added this to the 2.6 milestone Mar 22, 2023
Copy link
Contributor

@stof stof left a comment

Choose a reason for hiding this comment

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

it would be great to add tests covering that behavior

return Preg::replaceCallback(
'#^ {4,}#m',
static function ($match) use ($indent): string {
return str_repeat($indent, (int)(strlen($match[0] ?? '') / 4));
Copy link
Contributor

Choose a reason for hiding this comment

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

if the number of spaces is not a multiple of 4, we should probably keep the remaining spaces (as those would not be about indentation but about alignment)

Copy link
Contributor Author

@maximal maximal Mar 22, 2023

Choose a reason for hiding this comment

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

Mathematically speaking, the number of tabs must always be equal to the indent level, and then any number of spaces for alignment (aka SmartTabs approach), but I think here we can simplify the process as we are dealing with json_encode() result which can have either 4 or 0 spaces for indentation and have no alignment at all. (Also, JSON cannot have multiline strings and comments, so the regex-based replacing is somewhat reasonable.)

@Seldaek Seldaek merged commit 1c9fbeb into composer:main Jul 19, 2023
17 of 18 checks passed
@Seldaek
Copy link
Member

Seldaek commented Jul 19, 2023

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants