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

Replace <?php echo with <?= #2596

Merged
merged 2 commits into from Jan 3, 2014

Conversation

Projects
None yet
8 participants
@AD7six
Member

AD7six commented Jan 3, 2014

Adopt this convenience since 5.4 makes it possible on all supported installs.

Should not be merged before #2588 (at which point this will need regenerating).

Replace <?php echo with <?=
Generated with:

    find . -type f -print0 | xargs -0 sed -i s"/<.php echo /<?= /g"
@ADmad

This comment has been minimized.

Member

ADmad commented Jan 3, 2014

👍

@dereuromark

This comment has been minimized.

Member

dereuromark commented Jan 3, 2014

👎 I like <?php echo to keep things consistent across all files.

@renan

This comment has been minimized.

Member

renan commented Jan 3, 2014

👍

@lorenzo

This comment has been minimized.

Member

lorenzo commented Jan 3, 2014

I think the less reasons we have to put "<?php" the better. We should identify the common cases where we use control structures and minimize them

@lorenzo

This comment has been minimized.

Member

lorenzo commented Jan 3, 2014

👍

markstory added a commit that referenced this pull request Jan 3, 2014

Merge pull request #2596 from AD7six/3.0-shortecho
Replace <?php echo with <?=

@markstory markstory merged commit ba8d3ff into cakephp:3.0 Jan 3, 2014

1 check failed

default The Travis CI build failed
Details
@markstory

This comment has been minimized.

Member

markstory commented Jan 3, 2014

I like this as well. Less typing is nice.

@AD7six

This comment has been minimized.

Member

AD7six commented Jan 3, 2014

#2588 Is so much merge fun now.. =D

@markstory

This comment has been minimized.

Member

markstory commented Jan 3, 2014

Such conflicts.

@AD7six AD7six deleted the AD7six:3.0-shortecho branch Jan 3, 2014

@jippi

This comment has been minimized.

Member

jippi commented Jan 3, 2014

👍

@ionas

This comment has been minimized.

Contributor

ionas commented Jan 7, 2014

Related/Questions:
a.) Is there a reason to keep the leading and tailing white spaces:
<?= $this->Form->input(); ?>
vs:
<?=$this->Form->input();?>

b.) As of now there seems to be an inconsistency for the closing semi-colon
<?= $this->Form->input(); ?>
vs:
<?= $this->Form->input() ?>

I suggest to keep things as dense as possible cause PHP tends to be verbose anyway:
<?=$this->Form->input()?>
over:
<?=$this->Form->input(); ?>

As a related question: Has it been discussed when to use <?php if($statement):?><?php endif?> over <?php if ($statement) { ?><?php } ?> for views, layouts and bake results?
The same question applies to <?php foreach($collection):?><?php endforeach?> vs <?php foreach ($collection) { ?><? } ?>

@dereuromark

This comment has been minimized.

Member

dereuromark commented Jan 7, 2014

It would be <?php if ($statement): ?><?php endif ?> (note the space after if and :).
In general I like the space as it makes it a little bit easier to read.

So I'd go for <?= $this->Form->input(); ?>

@lorenzo

This comment has been minimized.

Member

lorenzo commented Jan 7, 2014

@ionas <?php if ($statement) :?> has been out standard for quite a while. Also having the ; with a space is part of the PHP language. Cannot remember what it does., though :P

I think it trims new lines when outputting, or adds the newline

@ionas

This comment has been minimized.

Contributor

ionas commented Jan 7, 2014

So there is an agreement for the white spaces (to keep them in) for both short echos and alternative syntax for control structures.

Currently (also in this commit) though there is still a mix of adding the last tailing ; and omitting it.

Examples:
<?= $admin ?>index method
$this->set('<?= $pluralName ?>', $this->paginate());
VS:
if (!$this-><?php echo $currentModelName; ?>->exists($id)) {
if (!$this-><?= $currentModelName; ?>->exists($id)) {

I'd purpose to omit the last tailing semicolon within layouts, views and bake results.

@lorenzo
Omitting that semicolon makes no difference AFAIK in terms of new lines. Omitting a space after ?> removes the carriage return if the new line starts with <?php or <? again, AFAIK.

Related: When to use Alternate Controle Syntax?
I'd not had replaced <?php echo from bake files themselves only from the baked results to keep things different and easier to read and had not used alternative syntax for control structures anywhere but in layouts, views and bake results, again to keep things easier to read/differentiate. E.g. bake files use <?php echo and standard control structures while the layouts, views and bake results use <?=, endif, endwhile, endfor, endforeach and endswitch.

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