Code style (endif endforeach endfor and more)

[luigifab]

Description

This PR updates code style of PHP code (#1027).

For express review, perhaps you can:

• git clone 1.9.4.x in base/
• git clone endif-endwhile-endforeach in changes/

Then, open all modified files of base/ directory in your text editor, search and replace (case sensitive) in all files (from bottom to top):

<?php endfor; ?>      by  <?php endfor ?>
<?php endfor;?>       by  <?php endfor ?>
<?php else :?>        by  <?php else: ?>
<?php else : ?>       by  <?php else: ?>
<?php else:?>         by  <?php else: ?>
<?php endforeach; ?>  by  <?php endforeach ?>
<?php endforeach;?>   by  <?php endforeach ?>
<?php endforeach?>    by  <?php endforeach ?>
<?php endif; //       by  <?php endif //
<?php //endif; ?>     by  <?php //endif ?>
<?php endif; ?>       by  <?php endif ?>
<?php endif;?>        by  <?php endif ?>
<?php endif?>         by  <?php endif ?>

Then, diff -r -U0 base/ changes/

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)

[kkrieger85]

After 300 files I'd say: LGTM

Few whitespace only changes, but looks good.

Need to discuss: We should change the review workflow for this kind of PR. Nobody want's to review 800 files manually and it's very time consuming (for me: 2 1/2hours for 300 files => estimated 6-7hours for all files)

[colinmollenhour]

Need to discuss: We should change the review workflow for this kind of PR. Nobody want's to review 800 files manually and it's very time consuming (for me: 2 1/2hours for 300 files => estimated 6-7hours for all files)

@luigifab If you applied the change with a command line tool perhaps you could supply the command used so we could easily reproduce it?

[luigifab]

Sorry, no command used, I search all files, open all files, search/replace string with my text editor, save and commit.

[kkrieger85]

509/805 files reviewed

still LGTM

[kkrieger85]

Finally: LGTW

thx @luigifab for this effort

[colinmollenhour]

@luigifab Sorry about the conflicts, can you merge the latest to resolve?

[luigifab]

Oops, my bad, I rebased the branch from 1.9.4.x instead of merging it :/
I don't understand why there are xmlconnect files in Files changed tab.

[kkrieger85]

@luigifab Please stop to add more commits here ;)

I think it would be better to create separate PR for different Code Style changes.
That would be improve the code review process.

[kkrieger85]

See discussion about PHPCSFIXER in #947

[luigifab]

Yes, so sorry, I rebased and repushed only endxyz changes to get a diff.

[luigifab]

I try to apply the diff but no... so I made a script to do the job:

<?php

$todo = [
	'<?php endfor; ?>'     => '<?php endfor ?>',
	'<?php endfor;?>'      => '<?php endfor ?>',
	'<?php else :?>'       => '<?php else: ?>',
	'<?php else : ?>'      => '<?php else: ?>',
	'<?php else:?>'        => '<?php else: ?>',
	'<?php endforeach; ?>' => '<?php endforeach ?>',
	'<?php endforeach;?>'  => '<?php endforeach ?>',
	'<?php endforeach?>'   => '<?php endforeach ?>',
	'<?php endif; //'      => '<?php endif //',
	'<?php //endif; ?>'    => '<?php //endif ?>',
	'<?php endif; ?>'      => '<?php endif ?>',
	'<?php endif;?>'       => '<?php endif ?>',
	'<?php endif?>'        => '<?php endif ?>',
];

foreach ($todo as $search => $replace) {

	echo $search,"\n";

	$files = [];
	exec('grep -Hlrn "'.$search.'" app/', $files);

	foreach ($files as $file) {
		echo '  ',$file,"\n";
		file_put_contents($file, str_replace($search, $replace, file_get_contents($file)));
		// system(git add app/ ; git commit -m "")
	}
}

[fballiano]

@luigifab loving this PR as I really didn't like the old syntax :-)

[colinmollenhour]

I used the PHP script on the same base commit and the result was slightly different but no material differences so I approve so this can get merged sooner than later. Attached is the diff that was generated.
diff.patch.txt

[github-actions]

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
6 runs  +4  4 ✔️ +2  0 💤 ±0  2 ❌ +2 

Results for commit 5a0b657. ± Comparison against base commit e4c2c4c.

[addison74]

Once this will be released with a new OM version I recommend a revision of the code looking for some parts unchanged. I will do that too.

A script (e.g. PHP, bash) will be useful to change any custom theme by everyone interested. I guess the one provided by @luigifab is just fine.

[addison74]

What is your opinion about <?php break; ?> ?

Should it be <?php break ?>?

[fballiano]

@addison74 you've a great eye, I added "break" as a new PR #1577