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

PSR-12 compliant function call wrapping #26

Closed
czosel opened this issue Feb 14, 2018 · 11 comments
Closed

PSR-12 compliant function call wrapping #26

czosel opened this issue Feb 14, 2018 · 11 comments
Assignees

Comments

@czosel
Copy link
Collaborator

czosel commented Feb 14, 2018

Quoting @nicoder:

As far as I can tell, both are PSR-2-compliant, and PSR-12 adds precisions because there was debate about that:

https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md#47-method-and-function-calls

for example PSR-12 lists this is valid:

$app->get('/hello/{name}', function ($name) use ($app) {
    return 'Hello ' . $app->escape($name);
});

but this would not be :

$app->get('/hello/{name}',
    function ($name) use ($app) {
        return 'Hello ' . $app->escape($name);
    }
);

I understand it this way:

  • if one argument is defined starting on a new line, then all of the arguments must start on their own line
  • but arguments may take up more than one line (independently of whether they start on their own line or not)

For example it may be nicer to see:

foo([
   $longExpression,
   $prettyLongExpression,   
]);

than:

foo(
    [
       $longExpression,
       $prettyLongExpression,   
    ]
);

Or nicer to see:

$db->Execute($sql, [
    $foo,
    $somewhatLongParameter,
]);

than:

$db->Execute(
    $sql,
    [
        $foo,
        $somewhatLongParameter,
    ]
);
@MichaelDeBoey
Copy link

So the example in #23:

$this->something->method($argument, $this->more->stuff($this->even->more->things->complicatedMethod()));

Should become

$this->something->method($argument, $this->more->stuff(
  $this->even->more->things->complicatedMethod()
));

I think 🙂

@czosel
Copy link
Collaborator Author

czosel commented Feb 14, 2018

@MichaelDeBoey yes, i think so too! Do you want to give this a shot? 😉

@MichaelDeBoey
Copy link

@czosel I'm not really into the codebase 😕

I'm just here because I'd like to use prettier for both my front- and backend in a Laravel project actually... 🙂

@czosel
Copy link
Collaborator Author

czosel commented Feb 14, 2018

Alright, no worries 🙂 If you change your mind, i'd recomment to take a look here and otherwise you can also always ask me!

@mathieutu
Copy link

An other example here. I think it can be related to this issue:

    {
-        return str_replace_array('#', [
-            $this->budget->name,
-            $this->requester->name,
-            'justif',
-        ], '# - # - #');
+        return str_replace_array(
+            '#',
+            [$this->budget->name, $this->requester->name, 'justif'],
+            '# - # - #'
+        );
     }

@mgrip mgrip self-assigned this Feb 26, 2018
@mathieutu
Copy link

So, for me, your examples are:

Do you confirm?

@czosel
Copy link
Collaborator Author

czosel commented Feb 27, 2018

@mgrip i also tried to implement this in #123 based on the JS implementation, but i didn't arrive at a proper solution yet. You might want to take a look 😄

@mgrip
Copy link
Contributor

mgrip commented Feb 27, 2018

Ahhh I was wondering if we would have to use that conditionalGroup - the docs say to use it as a last resort which scared me off haha

@mgrip
Copy link
Contributor

mgrip commented Feb 27, 2018

@mathieutu yes I agree - I'm wondering if its actually going to be possible to pretty-print in the way that PSR12 is dictating. Wanted to just give examples of how prettier-js does it for reference

@mgrip mgrip removed their assignment Feb 27, 2018
@czosel
Copy link
Collaborator Author

czosel commented Mar 5, 2018

Most of the examples listed in this issue should be fixed by #123 - let's open new issues for anything that might be missing.

@czosel czosel closed this as completed Mar 5, 2018
czosel pushed a commit that referenced this issue Jan 31, 2019
Bumps [mem](https://github.com/sindresorhus/mem) from 4.0.0 to 4.1.0.
<details>
<summary>Release notes</summary>

*Sourced from [mem's releases](https://github.com/sindresorhus/mem/releases).*

> ## v4.1.0
> - Work around some host environments throwing on `mimicFn` call ([#26](https://github-redirect.dependabot.com/sindresorhus/mem/issues/26))  e08ad85
> - Bump dependencies  bdfc93a
> 
> sindresorhus/memoize@v4.0.0...v4.1.0
</details>
<details>
<summary>Commits</summary>

- [`a583160`](sindresorhus/memoize@a583160) 4.1.0
- [`e08ad85`](sindresorhus/memoize@e08ad85) Work around some host environments throwing on `mimicFn` call ([#26](https://github-redirect.dependabot.com/sindresorhus/mem/issues/26))
- [`bdfc93a`](sindresorhus/memoize@bdfc93a) Bump dependencies
- [`60f28fd`](sindresorhus/memoize@60f28fd) Add failing tests for RegExp and Symbol arguments ([#21](https://github-redirect.dependabot.com/sindresorhus/mem/issues/21))
- See full diff in [compare view](sindresorhus/memoize@v4.0.0...v4.1.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=mem&package-manager=npm_and_yarn&previous-version=4.0.0&new-version=4.1.0)](https://dependabot.com/compatibility-score.html?dependency-name=mem&package-manager=npm_and_yarn&previous-version=4.0.0&new-version=4.1.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
czosel pushed a commit that referenced this issue Mar 11, 2019
Bumps [strip-ansi](https://github.com/chalk/strip-ansi) from 5.0.0 to 5.1.0.
<details>
<summary>Release notes</summary>

*Sourced from [strip-ansi's releases](https://github.com/chalk/strip-ansi/releases).*

> ## v5.1.0
> - Add support for terminal link escape codes ([#26](https://github-redirect.dependabot.com/chalk/strip-ansi/issues/26))  41b0a8b
> 
> chalk/strip-ansi@v5.0.0...v5.1.0
</details>
<details>
<summary>Commits</summary>

- [`581fd4e`](chalk/strip-ansi@581fd4e) 5.1.0
- [`41b0a8b`](chalk/strip-ansi@41b0a8b) Add support for terminal link ([#26](https://github-redirect.dependabot.com/chalk/strip-ansi/issues/26))
- [`841f0c4`](chalk/strip-ansi@841f0c4) Add security section
- See full diff in [compare view](chalk/strip-ansi@v5.0.0...v5.1.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=strip-ansi&package-manager=npm_and_yarn&previous-version=5.0.0&new-version=5.1.0)](https://dependabot.com/compatibility-score.html?dependency-name=strip-ansi&package-manager=npm_and_yarn&previous-version=5.0.0&new-version=5.1.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants