Everything

php

Consistent indentation for test files

php.internals

Nikita Popov

8 years ago

Hi internals,

Right now we do not have a consistent standard for the indentation of PHPT
files. Some people create space-indented files, others create tab-indented
files. Over time, indentation invariably starts to mix, because developers
with different indentation settings work on one file.

Here are the current statistics for code in .phpt files:

total: 15515
prefer tabs: 4273 (27.5%)
prefer spaces: 6307 (40.7%)
draw: 77 (0.5%)
no indentation: 4858 (31.3%)

There are 1824 (11.8%) files that contain mixed tab and space indentation.
The indentation was determined based on the first character of a line.

I would like to propose that we establish a common standard by
a) using space indentation for all future tests (as they currently form the
majority), and
b) reindenting existing test files to use space indentation.

Thanks,
Nikita

PS: Motivated by https://github.com/php/php-src/pull/2879. PRs regularly
have WS issues.

Christoph Becker

8 years ago

On 28.10.2017 at 13:59, Nikita Popov wrote:

> Right now we do not have a consistent standard for the indentation of PHPT
> files. Some people create space-indented files, others create tab-indented
> files. Over time, indentation invariably starts to mix, because developers
> with different indentation settings work on one file.
>
> Here are the current statistics for code in .phpt files:
>
> total: 15515
> prefer tabs: 4273 (27.5%)
> prefer spaces: 6307 (40.7%)
> draw: 77 (0.5%)
> no indentation: 4858 (31.3%)
>
> There are 1824 (11.8%) files that contain mixed tab and space indentation.
> The indentation was determined based on the first character of a line.

Thanks for bringing this up!

> I would like to propose that we establish a common standard by
> a) using space indentation for all future tests (as they currently form the
> majority), and
> b) reindenting existing test files to use space indentation.

+1

I'm rather baffled that <http://qa.php.net/write-test.php> doesn't
address this issue at all.

--
Christoph M. Becker

Kris Craig

8 years ago

On Sat, Oct 28, 2017 at 5:42 AM, Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> On 28.10.2017 at 13:59, Nikita Popov wrote:
>
> > Right now we do not have a consistent standard for the indentation of
> PHPT
> > files. Some people create space-indented files, others create
> tab-indented
> > files. Over time, indentation invariably starts to mix, because
> developers
> > with different indentation settings work on one file.
> >
> > Here are the current statistics for code in .phpt files:
> >
> > total: 15515
> > prefer tabs: 4273 (27.5%)
> > prefer spaces: 6307 (40.7%)
> > draw: 77 (0.5%)
> > no indentation: 4858 (31.3%)
> >
> > There are 1824 (11.8%) files that contain mixed tab and space
> indentation.
> > The indentation was determined based on the first character of a line.
>
> Thanks for bringing this up!
>
> > I would like to propose that we establish a common standard by
> > a) using space indentation for all future tests (as they currently form
> the
> > majority), and
> > b) reindenting existing test files to use space indentation.
>
> +1
>
> I'm rather baffled that <http://qa.php.net/write-test.php> doesn't
> address this issue at all.
>
> --
> Christoph M. Becker
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Those stats can be a bit misleading, though, as I've observed that many (if
not most) IDEs are configured to automatically convert tab characters to
spaces.  Therefore, it stands to reason that at least some of the "prefer
spaces" category would actually fall under "prefer tabs".  I suspect the
tabs and spaces groups are much closer together, though it's impossible to
say how much since there's no way to control for that variable.  Still, I
would caution people not to read too much into those statistics.

A common standard would be nice, but I fear it might discourage some from
contributing tests.  Also, how would we actually enforce this?  Would we
reject any tests that don't conform to this or fix them, ourselves?  Either
way, it would require someone to actually do the work.  Another possible
option would be to use some kind of automated conversion process.  But I
personally think these solutions are more trouble than they're worth.  So
long as the spacing is consistent within each file, I can live with it.

--Kris

Jakub Zelenka

8 years ago

On Sat, Oct 28, 2017 at 12:59 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi internals,
>
> Right now we do not have a consistent standard for the indentation of PHPT
> files. Some people create space-indented files, others create tab-indented
> files. Over time, indentation invariably starts to mix, because developers
> with different indentation settings work on one file.
>
> Here are the current statistics for code in .phpt files:
>
> total: 15515
> prefer tabs: 4273 (27.5%)
> prefer spaces: 6307 (40.7%)
> draw: 77 (0.5%)
> no indentation: 4858 (31.3%)
>
> There are 1824 (11.8%) files that contain mixed tab and space indentation.
> The indentation was determined based on the first character of a line.
>
> I would like to propose that we establish a common standard by
> a) using space indentation for all future tests (as they currently form the
> majority), and
>

I usually use tabs for phpt as it's a bit simpler with my IDE that I use
for PHP core dev (QtCreator) and mostly the core exts that I work with had
tabs. That said I wouldn't be against making it the same and use spaces
everywhere if we also start using PSR-2 for tests - it's not just an
indentation that is a mess...

> b) reindenting existing test files to use space indentation.
>
>

+1 but it needs to be done in the lowest active branch otherwise it will
result in conflicts when changing existing tests (it is sometimes useful
for extending tests)...

Cheers

Jakub

Anatoliy Belsky

8 years ago

Hi,

> -----Original Message-----
> From: jakub.php@gmail.com [mailto:jakub.php@gmail.com] On Behalf Of Jakub
> Zelenka
> Sent: Sunday, October 29, 2017 7:41 PM
> To: Nikita Popov <nikita.ppv@gmail.com>
> Cc: PHP internals <internals@lists.php.net>
> Subject: Re: [PHP-DEV] Consistent indentation for test files
>
> On Sat, Oct 28, 2017 at 12:59 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> > Hi internals,
> >
> > Right now we do not have a consistent standard for the indentation of
> > PHPT files. Some people create space-indented files, others create
> > tab-indented files. Over time, indentation invariably starts to mix,
> > because developers with different indentation settings work on one file.
> >
> > Here are the current statistics for code in .phpt files:
> >
> > total: 15515
> > prefer tabs: 4273 (27.5%)
> > prefer spaces: 6307 (40.7%)
> > draw: 77 (0.5%)
> > no indentation: 4858 (31.3%)
> >
> > There are 1824 (11.8%) files that contain mixed tab and space indentation.
> > The indentation was determined based on the first character of a line.
> >
> > I would like to propose that we establish a common standard by
> > a) using space indentation for all future tests (as they currently
> > form the majority), and
> >
>
> I usually use tabs for phpt as it's a bit simpler with my IDE that I use for PHP core
> dev (QtCreator) and mostly the core exts that I work with had tabs. That said I
> wouldn't be against making it the same and use spaces everywhere if we also
> start using PSR-2 for tests - it's not just an indentation that is a mess...
>
>
> > b) reindenting existing test files to use space indentation.
> >
> >
> +1 but it needs to be done in the lowest active branch otherwise it will
> result in conflicts when changing existing tests (it is sometimes useful for
> extending tests)...
>

Whichever way would be chosen, implementing a pre commit hook would make sense to be done. To mention, tabs were easier for me, too, but not a big deal anyway.

Regards

Anatol

Kalle Sommer Nielsen

8 years ago

2017-10-29 22:30 GMT+01:00 Anatol Belski <ab@php.net>:

> Whichever way would be chosen, implementing a pre commit hook would make sense to be done. To mention, tabs were easier for me, too, but not a big deal anyway.

I'm on the same boat as Anatol here, whatever is choosen is fine with
me, tho I generally tend to use tabs unless the test I copy/pasted had
spaces, I would follow that.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

Derick Rethans

8 years ago

On Sun, 29 Oct 2017, Jakub Zelenka wrote:

> On Sat, Oct 28, 2017 at 12:59 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

<snip>

> > Here are the current statistics for code in .phpt files:
> >
> > total: 15515
> > prefer tabs: 4273 (27.5%)
> > prefer spaces: 6307 (40.7%)
> > draw: 77 (0.5%)
> > no indentation: 4858 (31.3%)
> >
> > There are 1824 (11.8%) files that contain mixed tab and space indentation.
> > The indentation was determined based on the first character of a line.
> >
> > I would like to propose that we establish a common standard by
> > a) using space indentation for all future tests (as they currently form the
> > majority), and
>
> I usually use tabs for phpt as it's a bit simpler with my IDE that I use
> for PHP core dev (QtCreator) and mostly the core exts that I work with had
> tabs. That said I wouldn't be against making it the same and use spaces
> everywhere if we also start using PSR-2 for tests - it's not just an
> indentation that is a mess...
>
> > b) reindenting existing test files to use space indentation.
> >
> +1 but it needs to be done in the lowest active branch otherwise it will
> result in conflicts when changing existing tests (it is sometimes useful
> for extending tests)...

I'm -1 on the whole exercise, but this comment by Jakub is what worries
me most.

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Like Xdebug? Consider a donation: https://xdebug.org/donate.php
twitter: @derickr and @xdebug

Sara Golemon

8 years ago

On Sat, Oct 28, 2017 at 7:59 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> Right now we do not have a consistent standard for the indentation of
> PHPT files.
>
> I would like to propose that we establish a common standard
>

Not for nothin', but PSR-2 exists.  If we adopt *any* standard, I
think it should be this one* (which is a spiritual successor to PEAR's
CS fwiw).

> +1 but it needs to be done in the lowest active branch
> otherwise it will result in conflicts when changing
> existing tests (it is sometimes useful for extending tests)...
>

I'm not convinced that test files are in such an active state of churn
that this would net us much benefit for the cost.

-Sara

* PSR-12 is in the works as an update to PSR-2, but the items it
addresses are largely orthogonal to the kind of code you find in PHPT
files.