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

Minor readability fixes #12

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Minor readability fixes #12

wants to merge 4 commits into from

Conversation

Plecra
Copy link
Contributor

@Plecra Plecra commented Feb 16, 2021

I needed the crate for another project and spotted a couple lines that could be simplified. The code should behave the same

This solution uses an extra scope to hide the constants instead
@Lokathor
Copy link
Owner

Does putting the initial const outside the macro cause issues if you use the macro twice in the same scope?

@Plecra
Copy link
Contributor Author

Plecra commented Feb 18, 2021

I doubt it... I'll add a test

@Plecra
Copy link
Contributor Author

Plecra commented Feb 18, 2021

And in case I misunderstood - the mangled __UTF16_LIT_PREFIX_THAT_SHOULD_NEVER_CLASH_WITH_OUTER_SCOPE_UTF8 name is still in a small, internal scope. The actual logic is now two layers deep

@Plecra
Copy link
Contributor Author

Plecra commented Feb 18, 2021

Ah, CI is failing on the checked_sub not being stable in 1.46.0. Bummer. I'll revert that

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

Successfully merging this pull request may close these issues.

2 participants