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

Allow using pear sniffs with tabs #1362

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

wilsonge
Copy link

In @joomla we want to use these sniffs, but we use tabs instead of spaces. This has left us with a handful of (not so nice options). Copy and keep our own copies of the sniffs with these changes in, changing the message - except the message shows the number of spaces instead of tabs required (even through the autofixer works as expected), so basically this is less about the autofixing and more about displaying a user friendly message.

This is basically the same code taken from the Generic.Whitespace.ScopeIndentSniff

@photodude
Copy link
Contributor

@gsherwood any thoughts on merging this?

@wilsonge
Copy link
Author

Rebased to fix conflicts

@gsherwood gsherwood added this to the 3.1.0 milestone Jun 13, 2017
@gsherwood gsherwood modified the milestones: 3.2.0, 3.1.0 Aug 6, 2017
@gsherwood
Copy link
Member

I've decided to remove this from the 3.2.0 milestone because I'd like to find a better way of allowing any sniff to support tab indents without copy/pasting code and tests.

The fact that PHPCS will replace tabs with spaces while checking (assuming you use --tab-width) already allows for consistent checks, but there isn't any easy way to know if this happened and change error messages accordingly.

In addition, auto fixing should be able to apply whitespace indents and have the fixer automatically insert the correct type of indent based on the tab width setting and if the indent was already using tabs or not.

Obviously, if you've included a sniff like Generic.WhiteSpace.DisallowSpaceIndent then your indents are going to be replaced by tabs no matter what the other sniffs do during auto-fixing. So this really only applies when you are running a single fixer over your code base. Still, it's not that much effort to also include the indent sniff that you want at the same time to ensure the fixes use the correct indent, so I feel like there is an easy workaround for this already. The message is where no workaround currently exists.

@gsherwood gsherwood removed this from the 3.2.0 milestone Nov 22, 2017
@gsherwood gsherwood added this to Backlog in PHPCS v3 Development May 25, 2018
@wilsonge
Copy link
Author

wilsonge commented Oct 2, 2018

@gsherwood it's been a year since the last comment here - have you had any thoughts about how you'd like to progress with this. I'm willing to implement something else if you have an idea on how to achieve it. Of course your comment is totally on the money.

@gsherwood
Copy link
Member

It's been a year since the last comment here - have you had any thoughts about how you'd like to progress with this.

I have not had time to look into this and I'm not sure when I will.

@wilsonge
Copy link
Author

wilsonge commented Oct 3, 2018

In which case can I persuade you to reconsider this. I know it's not the most optimal thing ever - but it saves us having to create 4 sniffs just to work around this in Joomla (because too many end users got confused 🤷‍♂️ )

@gsherwood
Copy link
Member

In which case can I persuade you to reconsider this.

I will, when I get time to do some PHPCS dev again.

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

Successfully merging this pull request may close these issues.

None yet

3 participants