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

Feature suggestion: --space-indent=# command line option #2229

Closed
jrfnl opened this issue Nov 15, 2018 · 3 comments
Closed

Feature suggestion: --space-indent=# command line option #2229

jrfnl opened this issue Nov 15, 2018 · 3 comments

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Nov 15, 2018

PHPCS contains a number of sniffs which have a public $indent property (or similar).

If the standard of 4 space indents is used, all is fine. However, for non-standard space indenting, it is not always clear for people for which sniffs this property needs to be set, no matter that all the properties are outlined in the wiki.

Would it be an idea to add support for a space-indent command line option which would then be respected by all the PHPCS native sniffs which support the property ?

External standards could choose to add support for the command-line option as well if they have sniffs with a similar property.

@gsherwood Does this sound like a good idea ? If so, I'd be willing to have a go at adding this.

@gsherwood
Copy link
Member

I really like the idea of being able to batch-set the value of a sniff property. The bit that worries me is that this relies on a convention rather than a built-in feature, like tab-width is.

Maybe the solution here is to allow sniff properties to be defined on the command line more generally?

@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 16, 2018

Maybe the solution here is to allow sniff properties to be defined on the command line more generally?

Well, within WordPressCS there are a number of sniffs for which we have implemented this using --runtime-set in combination with a specific property name.

We have implemented this based on feature requests, specifically for the recommended properties, such as text_domain and prefixes.
Basically, the sniffs involved (or parts thereof) are disabled without those properties being set, so this was a simple way to allow people who review lots of themes/plugins to set those properties on the fly.

All the same, I think the $indent property is slightly different in that respect, as the sniffs aren't disabled if it's not set, but will run using the default value and if the property has not been adjusted consistently for all sniffs involved, it will automatically lead to fixer conflicts, such as found in #2227.

I like the idea of being able to set properties from the command line as a standard feature though, though implementing that will probably be more involved than what I had in mind originally.

@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 8, 2023

I'm going to close this issue.

People can set the property for all sniffs in one go by using:

<rule ref="Standard">
    <properties>
        <property name="indent" value="4"/>
    </properties>
</rule>

This already worked previously, but will work more cleanly since #3629.

The only caveat is sniffs which have a public property called indent, which isn't intended for the size of the space indentation. Those may run into trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Release
Development

No branches or pull requests

2 participants