Enforce "elseif" standard

Here is a simple find and replace command to track down all occurences of "else if" excluding Javascript files (execute in a Unix-style environment in your Drupal 8 root folder):
find . -path './.git' -prune -o -not \( -name '*.js' \) -type f -exec sed -i -e "s/else if/elseif/g" {} \;
This also changes core/lib/Drupal/Component/Archiver/ArchiveTar.php, which is a third party library. You can revert that change with
git checkout core/lib/Drupal/Component/Archiver/ArchiveTar.php
Patch attached.

And I think it is not necessary to backport this minor change to Drupal 7, so removing tag.

Adding "find and replace" tag, so that I remember where I put those useful find&sed commands. It can be quite tedious to get them actually right.

Looks pretty straight forward.

Wow, really, only 6 places in all of Core? So why were we thinking of going in the other direction? :)

Yep, that seams to be all ;)

(Except tons of javascript that needs to stay else if and third party PHP code.)

update version of Archive_Tar #1414508: update system.tar.inc to latest Archive_Tar (Pear)

Wow, excellent!

RE #6 -- um, why is that relevant here?

@jhodgdon

the 3rd party script contains "else if" (comment #5)

The new ArchiveTar.php in /core/lib/Drupal/Component/Archiver adds 20 more instances of this issue. Preparing a patch.

Here's a patch that includes those changes. In reading previous comments it seems that these changes were intended to be included but were somehow.

We should not be doing anything to third-party scripts at all?

Also, isn't it a standard to scope elseif's no matter what?

for example:

if ($this->_compress_type == 'gz')
  @gzclose($this->_file);
elseif ($this->_compress_type == 'bz2')
  @bzclose($this->_file);
elseif ($this->_compress_type == 'none')
  @fclose($this->_file);
else
  $this->_error('Unknown or missing compression type ('.$this->_compress_type.')');

Should the previous code be this instead?

if ($this->_compress_type == 'gz') {
  @gzclose($this->_file);
}
elseif ($this->_compress_type == 'bz2') {
  @bzclose($this->_file);
}
elseif ($this->_compress_type == 'none') {
  @fclose($this->_file);
}
else {
  $this->_error('Unknown or missing compression type ('.$this->_compress_type.')');
}

Do not try to "fix" external third party files, because this will be overwritten anytime the file is updated from upstream.

Reuploading patch from #1.

RTBC seconded. #14 is exactly what we need.

Tossing to Jennifer since it's coding standards related.

The patch seems to be fine, although I haven't verified that it catches all the spots...

But I also don't see anywhere on http://drupal.org/coding-standards where it says we should be using elseif as opposed to else if. So I think we need to draft some wording, probably for the section "control structures", before we do this patch?

Hm. Is that not already documented by virtue of the code sample directly beneath the "Coding standards" heading? I'm not sure we need to spell it out more than that, do we?

I think it should be spelled out in a note below, if it is a standard, especially given that PHP language supports both.

If everyone already agrees that it's a de-facto standard (as evidenced by most of core following it), then I can put a note there. Does it need to be in Coder too?

Coder already has Use "elseif" in place of "else if".

I added a brief note to the coding standards. I'll get the patch committed after I verify independently that it gets all the else if spots (except in 3rd-party libraries that we aren't modifying).

Patch above needs a reroll -- someone has already removed the else if lines from locale.admin.inc.

I have also just verified (I think) that the only places outside of Archiver and .js files that have elseif are:

core/modules/field/modules/options/options.module:211:        else if (!$has_value) {
core/modules/taxonomy/taxonomy.module:1331:        else if ($item['tid'] == 'autocreate') {
core/includes/utility.inc:49:  else if (is_object($var) && get_class($var) === 'stdClass') {

using

grep -R -n "else if" * | grep -v Archiver | grep -v ".js:"

Unassigning in hopes that someone will reroll this patch.

Here's a the reroll

Thank you, cosmicdreams. Assuming tests pass.

Committed to 8.x, thanks!

Patch needs reroll for 7.x, where the following command

grep -R -n "else if" * | grep -v system.tar.inc | grep -v ".js:"

says there are only two places to patch:

modules/field/modules/options/options.module:211:        else if (!$has_value) {
modules/taxonomy/taxonomy.module:1579:        else if ($item['tid'] == 'autocreate') {

Note that system.tar.inc in d7 I think is the same as Archiver.inc in D8 -- it's a 3rd-party file so we leave it alone.

Okay.

Thanks!

Committed to 7.x. Thanks!