Skip to content

Conversation

@m-burst
Copy link
Contributor

@m-burst m-burst commented May 19, 2019

This allows writing code like f"{[] |> len}".

Fixes #504.

This allows writing code like `f"{[] |> len}"`.

Fixes #504.
@m-burst
Copy link
Contributor Author

m-burst commented May 19, 2019

@evhub TL;DR the implementation is as follows: at the end of f_string_handle create a copy of the current compiler and use it to parse the expressions.

However, there is a problem: since the Forwards in the grammar are defined on the class level, there can only be one active Compiler because Compiler.bind in a new compiler sets the handlers to bound methods of the new compiler. I added a call to self.bind() as soon as the new compiler is no longer needed, but this feels like a terrible hack, so I've opened this as a draft pull request.

Do you have any ideas how to do this in a cleaner way?

Thanks in advance!

@pavelbraginskiy
Copy link

Does this support fstring formats, like f"{[] |> len :#x}"?

@m-burst
Copy link
Contributor Author

m-burst commented May 19, 2019

@pavelbraginskiy Yes, all formatting specifiers are supported.

@evhub evhub added this to the v1.4.1 milestone May 21, 2019
@m-burst
Copy link
Contributor Author

m-burst commented Jun 3, 2019

Hi @evhub, could you please give your feedback on this?

@evhub
Copy link
Owner

evhub commented Jun 8, 2019

@m-burst It looks like this implementation breaks f-strings on non-3.6 versions, which is a big problem. I'd rather keep universal compilation of f-strings and not support Coconut syntax in them than break universal compilation and add Coconut syntax support.

@m-burst
Copy link
Contributor Author

m-burst commented Jun 8, 2019

@evhub That's certainly not what I intended (if it's true, it is a bug and I'll see what I can do to fix that).

See the code here: if the target is 3.6+, we rebuild the f-string with the compiled expressions, otherwise we generate a format call.

Also there are f-string tests in target-agnostic tests and they pass under Python 2 (see the CI run).

@evhub
Copy link
Owner

evhub commented Jun 8, 2019

@m-burst Oh, no, you're right; I'm wrong! In that case, it looks good. I'll try to merge this when I get a chance.

@evhub evhub marked this pull request as ready for review June 11, 2019 23:58
@evhub evhub merged commit b42a019 into evhub:develop Jun 11, 2019
@evhub evhub added the resolved label Nov 24, 2019
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.

3 participants