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

[DSSv3] Issue 21637: trailing comma in import list #3079

Closed
wants to merge 3 commits into from

Conversation

dorianverna17
Copy link

Added an example of the functionality implemented in the pull request dlang/dmd#12917 at the section regarding selective imports

Added an example of the functionality implemented in the pull request dlang/dmd#12917 at the section regarding selective imports
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dorianverna17! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
21637 enhancement Allow trailing comma in the ImportList

Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

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

I don't think this is worth bringing attention to in the specification. Trailing commas are allowed in general (argument list, array literal, enum members, case statement) and there are no examples there either.

@dorianverna17
Copy link
Author

Should I close this pull request @dkorpel ?

@dkorpel
Copy link
Contributor

dkorpel commented Jul 29, 2021

Should I close this pull request @dkorpel ?

The change should still be reflected in the grammar:

dlang.org/spec/module.dd

Lines 196 to 199 in 0a90480

$(GNAME ImportList):
$(GLINK Import)
$(GLINK ImportBindings)
$(GLINK Import) $(D ,) $(GSELF ImportList)

You can use this PR to do that. For reference, here's how ParameterList is specified:

$(GNAME ParameterList):
$(GLINK Parameter)
$(GLINK Parameter) $(D ,)
$(GLINK Parameter) $(D ,) $(GSELF ParameterList)
$(GLINK VariadicArgumentsAttributes)$(OPT) $(D ...)

@dorianverna17 dorianverna17 changed the title Issue 21637: trailing comma in import list [DSSv3] Issue 21637: trailing comma in import list Jul 29, 2021
Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

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

From the implementation it seems you actually modified ImportBindList, not ImportList, so I pointed you to the wrong rule. Also, your implementation allows an empty bind list, but the grammar does not unless you add $(OPT) behind ImportBindList (similar to ParameterList as well).

@RazvanN7 RazvanN7 closed this Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants