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

[1.x] Add indent and line-ending options #230

Closed
wants to merge 3 commits into from

Conversation

aidanthewiz
Copy link

Laravel Pint, as an opinionated formatter, closely follows Laravel's internal formatting rules. However, the essence of Laravel is its configurability, a feature that should ideally extend to Pint. This update proposes the addition of customizable indentation and line-ending options, designed to integrate seamlessly and require minimal maintenance.

Key reasons for introducing these options include:

  • Accessibility: Adjustable indentation improves readability for developers with visual impairments or those using screen readers, making Pint more inclusive.

  • Focus on Development, Not Formatting Debates: Issues like ->setIndent("\t") #24 and Add Ability to Set Indentation Level #90 show how formatting can become a point of contention. Configurable options can minimize these distractions for the Laravel maintainers.

  • Integration with Legacy Code: For projects transitioning to Laravel, pre-existing code standards can be maintained without extensive reformatting, easing the integration process.

  • Adherence to Regional or Organizational Standards: Flexibility in formatting helps developers meet specific coding standards required in different regions or by various organizations.

Laravel Pint is already heavily configurable by allowing you to adjust rules, which is appreciated by many. But these last two options would make it a lot easier for teams to integrate the tool with existing workflows. Developers could of course create their own workflows with PhpCsFixer manually, but most of them probably also want the rules specified by this project to stay in line with most of the Laravel maintainer's opinions. Allowing more configurability will allow more Laravel projects to adopt this tool, keeping Laravel projects between organizations mostly in sync with the Laravel style guidelines with some organizations making some custom changes here and there.

I hope you will consider merging this PR and let me know if there are any additional changes you would like me to make. I can also update the Laravel documentation with these options if you would like in another PR.

@taylorotwell
Copy link
Member

Keeping Pint opinionated.

@josh-brainbox
Copy link

Keeping Pint opinionated.

Hey @taylorotwell and @nunomaduro,

Could you please provide more insight into the reasoning behind this decision, especially concerning its impact on accessibility?

While the Laravel community aims for inclusivity, this choice seems to disproportionately affect developers with visual impairments and those utilizing specialized input devices for code navigation. Pint offers extensive configuration options, but is the resistance to adding (~10) extra lines of code for accessibility support related to some unapparent cost? Would accommodating this additional configuration outweigh the blatant downside of making the 'out-of-the-box' Laravel experience worse for developers facing challenges?

Given that Pint allows for nearly all configurations from php-cs-fixer and comes bundled with Laravel, I'm curious about the specific 'opinion' the Laravel team intends to maintain here. If Pint didn't allow any configuration or wasn't bundled with Laravel by default, I could understand avoiding scope expansion, but considering its flexibility, it raises questions about the underlying rationale.

At the end of the day, the default configuration should be the Laravel 'opinion', but this seems like an easy win to promote inclusivity and accessibility, not only in the scope of the Laravel community, but for the industry as a whole.

@ndf
Copy link

ndf commented Dec 30, 2023

It is not ideal if the pr won’t be merged into pint. But it still has value to improve this pr, so that anyone can patch pint for projects that need it, without having to create and maintain a fork.

The test coverage could be improved; the pr now has a test for the configuration itself, but not for the actual formatting results.

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.

4 participants