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

Format macro #897

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Format macro #897

wants to merge 2 commits into from

Conversation

bch29
Copy link

@bch29 bch29 commented Dec 14, 2020

Includes changes from #896

Addresses #664

This is a preliminary PR to help diagnose why this isn't working. I am happy to accept discussion/changes to how the macro works.

src/format_macro.rs Outdated Show resolved Hide resolved
src/format_macro.rs Outdated Show resolved Hide resolved
@bch29
Copy link
Author

bch29 commented Dec 15, 2020

One question I had: this works somewhat unpleasantly with strings at the moment, because show adds quotes.

let x = "hello"
format! "x is {x}"

becomes x is "hello".

I could look up type information and handle strings differently, but that feels ugly. Perhaps we should add a Display typeclass for this purpose, which would render strings without quotes?

@bch29 bch29 force-pushed the format-macro branch 2 times, most recently from 4ea2798 to 728e9d9 Compare December 15, 2020 10:55
@Marwes
Copy link
Member

Marwes commented Dec 15, 2020

I could look up type information and handle strings differently, but that feels ugly. Perhaps we should add a Display typeclass for this purpose, which would render strings without quotes?

Type information isn't available at macro expansion (though I have thought about adding a second macro expansion step which does have that capability). Having a Debug and Display typeclass/trait like rust is probably the way to go (with Show being changed to Debug) 👍

@bch29
Copy link
Author

bch29 commented Dec 15, 2020

Added a Display typeclass. I have left Show as-is: I think if it is changed to Debug it should be done in a separate PR.

I now support {x} for display and {x:?} for show, similar to rust.

Now, arbitrary expressions that do not contain braces or : are allowed in the format subexpressions. This was actually simpler for me to implement than restricting to single variable names and it allows some nice flexibility, for example:

let foo = { x = 1, y = 2}
format! "x is {foo.x}, y is {foo.y}"

Let me know whether you agree that this is a good idea.

Copy link
Member

@Marwes Marwes left a comment

Choose a reason for hiding this comment

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

Some test failures related to display not being imported but otherwise LGTM 👍

@scarf005
Copy link

Hi, could there be any chance this PR could get traction? being able to interpolate strings sounds really useful

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.

3 participants