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

proof of concept for handling expressions such as println_f!("file: {… #1

Closed
wants to merge 6 commits into from

Conversation

rich-murphey
Copy link
Contributor

@rich-murphey rich-murphey commented Sep 22, 2019

this is a proof of concept for handling expressions such as:

    struct Foo { a: i64 }
    let foo = Foo { a: 1234 };
    println_f!("foo.a: {foo.a}");

it works for this simple example, and for certain others as long as ':' doesn't occur in the expression inside {}.

@rich-murphey
Copy link
Contributor Author

the second commit adds support for a trailing '='

println_f!("{foo.a=}");

will output

foo.a=1234

@rich-murphey
Copy link
Contributor Author

the last two commits simplify the format string parsing to make the code easier to read and maintain.

@danielhenrymantilla
Copy link
Owner

Hey, thank you very much for the PR @rich-murphey!

First of all, we must be careful not to allow expressions that are too complex within f-strings, as that leads to mixing code logic and data (string templates), which in turn hinders readability and is thus considered a bad practice. That is, this crate purpose is not the same as ::ifmt's.

That being said, I do agree that being able to have field access within the f-strings is a neat and desirable feature, as I've ended up destructuring many structs to be able to use their fields within an f-string.

As you comment in the code, using ::syn to parse such field accesses might be an idea to explore, and I will do so in the days to come, to see what kind of code it results in (compared, for instance, to yours).


Your PR also includes a magic = token to add code identifier context into the generated strings, and that too is a feature that should come in handy while not hurting readability too much. I will, however, perform that change within a second PR, so as to get each feature clearly linked to distinc PRs each.

@rich-murphey
Copy link
Contributor Author

Thanks so much for taking the time to evaluate and explain. I agree that in the long run, limiting items to terms would be best practice.

Perhaps there's a simple way to limit the "{item}"s to terms, and preclude expressions? I guess prohibiting any brackets within an item would do it.

@rich-murphey
Copy link
Contributor Author

The last commit allows one to use f strings in debug logs, such as:

struct Foo {...}

fn bar( foo :&Foo ) {
debug_f!("bar: received {foo=:?}");

@danielhenrymantilla
Copy link
Owner

I've (auto-)closed this PR since I have implemented both field accesses and the = printing gimmick as of the 0.2.1 release 🎉Thank you @rich-murphey for suggesting it (I haven't used your implementation since I've decided to go "full syn" on the inner parsing; if you still find there are things in the added code that could be improved / you would have done differently, feel free to comment about it).

I will soon add support for the ::log macros as well, although I would like to feature-gate that (to make the dependency on ::log be optional). If you suggest that change in another PR we can get that merged pretty quickly.

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