Skip to content
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

Add echo_tag_syntax fixer #4493

Open
wants to merge 27 commits into
base: master
from
Open

Conversation

@mlocati
Copy link
Contributor

mlocati commented Aug 6, 2019

We have a fixer (no_short_echo_tags) that expands short echo tags (<?= $foo ?>) to long echo tags (<?php echo $foo ?>).

What about adding a new fixer (short_echo_tags) that can also do the reverse?

Fix #2990

@Big-Shark

This comment has been minimized.

Copy link

Big-Shark commented Nov 19, 2019

@kubawerlos Any update about this?

Copy link
Contributor

kubawerlos left a comment

Still needs some work 👍

README.rst Outdated Show resolved Hide resolved
dev-tools/trigger-website.sh Outdated Show resolved Hide resolved
src/Fixer/PhpTag/NoShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
src/Fixer/PhpTag/ShortEchoTagFixer.php Outdated Show resolved Hide resolved
tests/Fixer/PhpTag/NoShortEchoTagFixerTest.php Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@mlocati mlocati changed the title Add short_echo_tag fixer Add echo_tag_syntax fixer Nov 29, 2019
src/RuleSet.php Outdated Show resolved Hide resolved
@mlocati

This comment has been minimized.

Copy link
Contributor Author

mlocati commented Dec 2, 2019

There's still this issue to be fixed.

How should these cases be translated from long-echo to short-echo tags?

  1. <?php /*comment*/ echo
      1
    ?>
  2. <?php
      /*comment*/ echo 1
    ?>
  3. <?php
      /*comment*/
      echo
      1
    ?>

Right now, they are fixed as:

  1. <?=/*comment*/
      1
    ?>
  2. <?=/*comment*/ 1
    ?>
  3. <?=/*comment*/
      1
    ?>
@julienfalque

This comment has been minimized.

Copy link
Member

julienfalque commented Dec 2, 2019

IMO keeping current behavior is fine.

@mlocati

This comment has been minimized.

Copy link
Contributor Author

mlocati commented Dec 2, 2019

IMO keeping current behavior is fine.

Ok, so I'll update the code as per @kubawerlos suggestion.

@mlocati

This comment has been minimized.

Copy link
Contributor Author

mlocati commented Dec 2, 2019

Ok, so I'll update the code as per @kubawerlos suggestion.

Nope, I can't find a way to implement #4493 (comment) in a faster/simpler way.

So, unless @kubawerlos sees a way to implement it, IMHO this PR is ready to be merged...

@julienfalque

This comment has been minimized.

Copy link
Member

julienfalque commented Dec 2, 2019

Could you please rebase onto latest master to get your branch up-to-date?

@mlocati mlocati force-pushed the mlocati:short_echo_tag branch from 3417e70 to 378a3ae Dec 2, 2019
@mlocati

This comment has been minimized.

Copy link
Contributor Author

mlocati commented Dec 2, 2019

Could you please rebase onto latest master to get your branch up-to-date?

Done.

src/Fixer/PhpTag/EchoTagSyntaxFixer.php Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
mlocati added 12 commits Aug 6, 2019
mlocati and others added 14 commits Nov 21, 2019
@mlocati mlocati force-pushed the mlocati:short_echo_tag branch from dd9c367 to 88134f3 Dec 10, 2019
@mlocati mlocati force-pushed the mlocati:short_echo_tag branch from 88134f3 to 034546b Dec 10, 2019
@julienfalque julienfalque added this to the 2.17.0 milestone Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.