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

fix issue 19442 - multiple argument string mixin dont support char literals #9491

Merged
merged 1 commit into from Mar 30, 2019
Merged

fix issue 19442 - multiple argument string mixin dont support char literals #9491

merged 1 commit into from Mar 30, 2019

Conversation

ghost
Copy link

@ghost ghost commented Mar 25, 2019

To support a character, an array literal containing this character is created, which then support the .toStringExp method used later.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Basile-z! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
  • 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
19442 minor multiple argument string mixin dont support char literals

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9491"

@thewilsonator
Copy link
Contributor

Love the idea, but D source code is defined to be UTF-8, do we even support wstring and dstring mixins?

@ghost
Copy link
Author

ghost commented Mar 25, 2019

Yes, that works, it looks like it's réencoded, for example this is okay

enum1 = 42;
enum x2 = mixin('x', wchar('é'), 1);  

@thewilsonator
Copy link
Contributor

Huh. I'll put a 72h-> merge on this in case anyone else wants to chime in.

@thewilsonator thewilsonator added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Mar 26, 2019
@ghost
Copy link
Author

ghost commented Mar 30, 2019

Addressed the remaining suggestion.
Ping for auto-merge @thewilsonator.

@thewilsonator thewilsonator added Merge:auto-merge and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels Mar 30, 2019
@dlang-bot dlang-bot merged commit f297107 into dlang:master Mar 30, 2019
@ghost ghost deleted the issue-19442 branch March 30, 2019 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants