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.

Sign up for GitHub

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

Jump to bottom

Add HeredocIndentationFixer #3915

Merged
merged 1 commit into from Dec 28, 2018
Merged

Add HeredocIndentationFixer #3915

merged 1 commit into from Dec 28, 2018

Conversation

gharlan
Copy link
Contributor

@gharlan gharlan commented Jul 14, 2018 •
edited

#3697 / https://wiki.php.net/rfc/flexible_heredoc_nowdoc_syntaxes

Requires PHP 7.3

Input:

<?php
    $a = [
        <<<'EOD'
abcdef
    ghi
jkl
EOD
    ];

is fixed to:

<?php
    $a = [
        <<<'EOD'
            abcdef
                ghi
            jkl
            EOD
    ];

@gharlan gharlan force-pushed the heredoc-indentation branch 6 times, most recently from 9c50491 to b644ac4 Compare July 14, 2018 12:00
@gharlan gharlan changed the title (WIP) Add HeredocIndentationFixer Add HeredocIndentationFixer Jul 14, 2018
@dmvdbrugge
Copy link
Contributor

dmvdbrugge commented Jul 16, 2018 •
edited

Is it strange that I'd prefer it to be like this? It would be the same as other opening/closing pairs.

<?php
    $a = [
        <<<'EOD'
            abcdef
                ghi
            jkl
        EOD
    ];

Or does the standard not allow that? It indeed doesn't, I should learn to read the provided links before commenting...

@gharlan
Copy link
Contributor Author

gharlan commented Jul 16, 2018

It would change the string content, so it would be risky.

This would be non-risky:

<?php
    $a = [
        <<<'EOD'
        abcdef
            ghi
        jkl
        EOD
    ];

If it is desired, there could be a configuration option for choosing this style, or the style from my first comment.

@gharlan
Copy link
Contributor Author

gharlan commented Jul 16, 2018

But I think the style from my first comment is better, especially in other cases, like function calls:

Input:

<?php
    foo(<<<'EOD'
abcdef
    ghi
jkl
EOD
    );

Style 1 (current indentation + 1):

<?php
    foo(<<<'EOD'
        abcdef
            ghi
        jkl
        EOD
    );

Style 2 (current indentation + 0):

<?php
    foo(<<<'EOD'
    abcdef
        ghi
    jkl
    EOD
    );

@dmvdbrugge
Copy link
Contributor

Oh yeah I agree, definitely current +1.

What I commented goes against the standard unfortunately (but understandably too, how else would they determine what whitespace to strip).

@julienfalque julienfalque mentioned this pull request Aug 8, 2018
Closed
7 tasks
@gharlan gharlan force-pushed the heredoc-indentation branch 2 times, most recently from e50912b to ecd0db7 Compare September 7, 2018 19:49
@gharlan
Copy link
Contributor Author

gharlan commented Nov 27, 2018

Anything to do here?

@SpacePossum
Copy link
Contributor

SpacePossum commented Dec 4, 2018 •
edited

On PHP7.4 the tests fail; https://travis-ci.org/FriendsOfPHP/PHP-CS-Fixer/jobs/460930989 (I can reproduce these local on PHP 7.3.0RC6 (cli) )

@SpacePossum SpacePossum changed the title Add HeredocIndentationFixer HeredocIndentationFixer - Introduction Dec 4, 2018
@gharlan
Copy link
Contributor Author

gharlan commented Dec 4, 2018 •
edited

@SpacePossum it is fixed now.
(the current errors seem not to be not related to this pr)

@SpacePossum SpacePossum added this to the 2.14.0 milestone Dec 11, 2018
@SpacePossum SpacePossum added the RTM Ready To Merge label Dec 27, 2018
@keradus keradus changed the title HeredocIndentationFixer - Introduction Add HeredocIndentationFixer Dec 28, 2018
@keradus keradus removed the RTM Ready To Merge label Dec 28, 2018
@keradus
Copy link
Member

keradus commented Dec 28, 2018

Thank you @gharlan.

@keradus keradus merged commit 61242ca into PHP-CS-Fixer:master Dec 28, 2018
keradus added a commit that referenced this pull request Dec 28, 2018
@keradus
feature #3915 Add HeredocIndentationFixer (gharlan)
a36ee8c 
This PR was merged into the 2.14-dev branch.

Discussion
----------

Add HeredocIndentationFixer

#3697 / https://wiki.php.net/rfc/flexible_heredoc_nowdoc_syntaxes

Requires PHP 7.3

Input:

```php
<?php
    $a = [
        <<<'EOD'
abcdef
    ghi
jkl
EOD
    ];
```

is fixed to:

```php
<?php
    $a = [
        <<<'EOD'
            abcdef
                ghi
            jkl
            EOD
    ];
```

Commits
-------

61242ca Add HeredocIndentationFixer
@SpacePossum
Copy link
Contributor

thanks @gharlan 🎉 :)

@gharlan gharlan deleted the heredoc-indentation branch March 19, 2019 10:59
@gharlan gharlan mentioned this pull request Mar 9, 2020
Merged
SpacePossum added a commit that referenced this pull request Apr 5, 2020
@SpacePossum
feature #4880 HeredocIndentationFixer - config option for indentation…
98e354f 
… level (gharlan)

This PR was merged into the 2.17-dev branch.

Discussion
----------

HeredocIndentationFixer - config option for indentation level

We discussed the best indentation level for the HeredocIndentationFixer [here](#3697 (comment)) and [here](#3915).

But now since I'm actually using it, I changed my mind.
I would like to write code like this:

```php
    public function foo()
    {
        return $this->createQuery(<<<'DQL'
            SELECT foo
            FROM App\Entity\Foo foo
            WHERE ...
        DQL)->getResult();
    }
```

and NOT:

```php
    public function foo()
    {
        return $this->createQuery(<<<'DQL'
            SELECT foo
            FROM App\Entity\Foo foo
            WHERE ...
            DQL)->getResult();
    }
```

So I suggest to add a config option `indentation` with possible values `start_plus_one` (default; same as current behavior) and `same_as_start` (new behavior). (Ideas for better names?)

The fixer will remain non-risky, so it does not change the actual string result.
This input:

```php
    $dql = <<<'DQL'
SELECT foo
FROM App\Entity\Foo foo
DQL;
```

will be changed with the new behavior (`same_as_start`) to:

```php
    $dql = <<<'DQL'
    SELECT foo
    FROM App\Entity\Foo foo
    DQL;
```

Adding one more indentation level inside string content is not part of this fixer (as it would be risky).

Commits
-------

b03554d HeredocIndentationFixer - config option for indentation level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntzm ntzm ntzm requested changes

@SpacePossum SpacePossum SpacePossum approved these changes

@dmvdbrugge dmvdbrugge dmvdbrugge approved these changes

Assignees
No one assigned
Labels
kind/feature
Projects
None yet
Milestone
2.14.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gharlan @dmvdbrugge @SpacePossum @keradus @ntzm