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

Faulty core fixes: mixed tab/space indentation #983

Closed
jrfnl opened this Issue Jun 18, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@jrfnl
Contributor

jrfnl commented Jun 18, 2017

There are some instances where spaces are used instead of tabs or a mixture of spaces and tabs.
While the auto-fixer can correct most of this, if the spaces used is not a multitude of 4, it will - correctly - assume that the extra spaces are used for precision alignment.

I don't think we can fix this as there is no way for the sniffer to know whether this "precision alignment" was intentional or accidental.

What we could do however, is to throw a warning when precision alignment is found to alert someone to manual inspect the code.

Opinions ?

Example:

File: ./src/wp-admin/post.php line 51 (and various other files)

Source: (five spaces)

if ( ! $sendback ||
     strpos( $sendback, 'post.php' ) !== false ||
     strpos( $sendback, 'post-new.php' ) !== false ) {
 	// something
}

Fixed as: (tab + space)

if ( ! $sendback ||
	 strpos( $sendback, 'post.php' ) !== false ||
	 strpos( $sendback, 'post-new.php' ) !== false ) {
 	// something
}
@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Jun 18, 2017

Member

Why the consideration for alignment, when considering indentation? I don't think there should be any "precision alignment for the first character" allowed; all spaces in indentation should be considered to be invalid.

(Here, I'd expect ! $sendback || to be on a new line too.)

Member

GaryJones commented Jun 18, 2017

Why the consideration for alignment, when considering indentation? I don't think there should be any "precision alignment for the first character" allowed; all spaces in indentation should be considered to be invalid.

(Here, I'd expect ! $sendback || to be on a new line too.)

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Jun 18, 2017

Contributor

Why the consideration for alignment, when considering indentation? I don't think there should be any "precision alignment for the first character" allowed; all spaces in indentation should be considered to be invalid.

Eh.. what about docblocks ?

	/**
	 * <- Before the asterix is a space for precision alignment...
	 */
	function something() {}

There are also other examples with things like XML documents being concatenated together using precision spaces (including in Core!).

(Here, I'd expect ! $sendback || to be on a new line too.)

That is a different issue and AFAIK not covered in the handbook, so not checked for at all.
If you think that should change, please discuss it in the Slack channel and/or open an issue for this.

Contributor

jrfnl commented Jun 18, 2017

Why the consideration for alignment, when considering indentation? I don't think there should be any "precision alignment for the first character" allowed; all spaces in indentation should be considered to be invalid.

Eh.. what about docblocks ?

	/**
	 * <- Before the asterix is a space for precision alignment...
	 */
	function something() {}

There are also other examples with things like XML documents being concatenated together using precision spaces (including in Core!).

(Here, I'd expect ! $sendback || to be on a new line too.)

That is a different issue and AFAIK not covered in the handbook, so not checked for at all.
If you think that should change, please discuss it in the Slack channel and/or open an issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment