8000 Allow trailing comma for function use and parameter lists by mkornaukhov03 · Pull Request #664 · VKCOM/kphp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow trailing comma for function use and parameter lists #664

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
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Allow trailing comma for function use and parameter lists #664

wants to merge 5 commits into from

Conversation

mkornaukhov03
Copy link
Contributor
@mkornaukhov03 mkornaukhov03 commented Oct 31, 2022 •
edited
Loading

Support PHP8 feature.
Main issue: #290
Previous attempt: #308
I didn't rebase because of merge conflict.

@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch from 1be3414 to 6c012c8 Compare October 31, 2022 22:30
@quasilyte
Copy link
Contributor
quasilyte commented Nov 2, 2022 •
edited
Loading

Hi!

I would suggest adding the tests.

  1. One test should fail for the incorrect syntax combinations. Two trailing commas, etc. See @kphp-should-fail annotations in tests/phpt/
  2. Another test should check that trailing commas work as expected (@ok test).

Basically, this feature should work when it works in PHP and fail to compile when it's not a valid syntax for PHP. You may look at the PHP pull request for an inspiration. It probably includes some tests.

See the docs for more info about contributing (how to run tests, etc.)

Or you can ask the questions here.
For Russian-speaking contributors I would suggest a Telegram chat: https://t.me/kphp_chat

@Danil42Russia Danil42Russia added the PHP8 PHP8 feature label Nov 3, 2022
@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch from 6c012c8 to 1f5cfa1 Compare November 28, 2022 08:05
@mkornaukhov03
Copy link
Contributor Author

Please check

@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch from a7fe9c3 to f452192 Compare February 8, 2023 09:56
@mkornaukhov03 mkornaukhov03 requested review from tolk-vm and quasilyte and removed request for tolk-vm February 8, 2023 16:11
@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch 3 times, most recently from f5937ec to 8f78498 Compare February 15, 2023 16:53
@mkornaukhov03 mkornaukhov03 force-pushed the mkornaukhov03_trailing_comma branch from 8f78498 to 19abded Compare February 21, 2023 12:23
@mkornaukhov03 mkornaukhov03 requested a review from drdzyk February 27, 2023 12:50
@Tsygankov-Slava Tsygankov-Slava mentioned this pull request Jul 3, 2023
Open
44 tasks
@mkornaukhov03 mkornaukhov03 mentioned this pull request Mar 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quasilyte quasilyte Awaiting requested review from quasilyte

@tolk-vm tolk-vm Awaiting requested review from tolk-vm

@drdzyk drdzyk Awaiting requested review from drdzyk

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
PHP8 PHP8 feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mkornaukhov03 @quasilyte @Danil42Russia
0