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

[NETBEANS-5172] Fix anonymous function formatting #2614

Merged

Conversation

KacerCZ
Copy link
Contributor

@KacerCZ KacerCZ commented Dec 29, 2020

https://issues.apache.org/jira/browse/NETBEANS-5172

  • Enabled and fixed tests for spaces around keyword.
  • Fixed formatting of anonymous functions.
  • Updated expected formatting results in rest of tests.

Sample code:

$lambda = function($param) use($parent) {
    echo "$param\n";
};

Formatted after fix:

$lambda = function ($param) use ($parent) {
    echo "$param\n";
};

@junichi11 junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Dec 30, 2020
@junichi11 junichi11 added this to the 12.3 milestone Dec 30, 2020
Copy link
Member

@junichi11 junichi11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that you have run all tests in your local environment (or repository) because GitHub actions are not available. Thanks!

@KacerCZ
Copy link
Contributor Author

KacerCZ commented Dec 30, 2020

Yes, I run all tests in "php.editor.indent" package.

Copy link
Member

@tmysik tmysik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't like this change (yes, I don't use this space in my code) - looking at the number of the changed files in this patch, this will have a big impact; moreover, there was no issue reported for the current behavior so far (it means for the last couple of years), right?

Can we make this configurable? If there was an option for it, I would not be against merging this PR.

@KacerCZ
Copy link
Contributor Author

KacerCZ commented Dec 31, 2020

Configuration is now bound to PHP > Spaces > Other > After Keywords.
I will add dedicated setting as PHP > Spaces > Before Parentheses > Anonymous Function (there is already setting for Anonymous Class).

By default it will be enabled because it is the way how anonymous functions are formatted in PHP manual - https://www.php.net/manual/en/functions.anonymous.php

@tmysik
Copy link
Member

tmysik commented Dec 31, 2020

@KacerCZ Great, thank you!

https://issues.apache.org/jira/browse/NETBEANS-5172

- Enabled and fixed tests for spaces around keyword.
- Fixed formatting of anonymous functions.
- Updated expected formatting results in rest of tests.

Sample code:
```php
$lambda = function($param) use($parent) {
    echo "$param\n";
};
```

Formatted after fix:
```php
$lambda = function ($param) use ($parent) {
    echo "$param\n";
};
```
@KacerCZ KacerCZ force-pushed the netbeans-5172-anonymous-function-format branch from 980e813 to 3a8f505 Compare December 31, 2020 12:43
@KacerCZ
Copy link
Contributor Author

KacerCZ commented Dec 31, 2020

I added new formatting setting and related tests.
Also added anonymous function to code sample in options panel.

@tmysik tmysik self-requested a review December 31, 2020 13:19
@tmysik
Copy link
Member

tmysik commented Dec 31, 2020

@KacerCZ Thanks a lot!

@junichi11 Feel free to merge it if it is OK for you. Thanks!

@junichi11 junichi11 merged commit bad721a into apache:master Dec 31, 2020
@KacerCZ KacerCZ deleted the netbeans-5172-anonymous-function-format branch January 1, 2021 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants