-
Notifications
You must be signed in to change notification settings - Fork 155
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
Basic f-strings (intepolated strings) #258
Conversation
shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala
Outdated
Show resolved
Hide resolved
shared/src/main/scala/io/kaitai/struct/translators/CommonLiterals.scala
Outdated
Show resolved
Hide resolved
shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala
Outdated
Show resolved
Hide resolved
shared/src/main/scala/io/kaitai/struct/translators/GoTranslator.scala
Outdated
Show resolved
Hide resolved
shared/src/main/scala/io/kaitai/struct/translators/CommonLiterals.scala
Outdated
Show resolved
Hide resolved
shared/src/main/scala/io/kaitai/struct/translators/BaseTranslator.scala
Outdated
Show resolved
Hide resolved
@GreyCat I also have a question - is there a way to get a literal In Python, for example, you can escape a >>> f"1 + 2 = {1 + 2} {{basic math}}"
'1 + 2 = 3 {basic math}' |
commit 1dbafe3 Author: Mikhail Yakshin <[email protected]> Date: Thu Feb 29 15:08:24 2024 +0000 ExpressionsSpec: f-strings: added test with newline in the middle, fixed name of test with double quote in the middle commit 8605f3f Author: Mikhail Yakshin <[email protected]> Date: Thu Feb 29 15:04:40 2024 +0000 Lexical: moved fstringItem + fstringChar from Expressions; Expressions: fixed rep -> repX to avoid whitespace problem commit 17f9e40 Author: Mikhail Yakshin <[email protected]> Date: Sat Oct 14 21:07:58 2023 +0100 Added similar escaped quote+space test for double-quoted string commit c0083fb Author: Mikhail Yakshin <[email protected]> Date: Sat Oct 14 20:48:19 2023 +0100 Added tests suggested in PR review: quote in f-string, regular string in f-string, f-string in f-string commit 1b22258 Author: Mikhail Yakshin <[email protected]> Date: Sat Oct 14 20:38:19 2023 +0100 Apply suggestions from code review Co-authored-by: Petr Pučil <[email protected]> commit 97ffceb Author: Mikhail Yakshin <[email protected]> Date: Sat Oct 14 08:50:41 2023 +0100 Added basic interpolated string (f-string) translation into target languages, mostly working off concatenation and existing other-types-to-string conversions. Supports only integers and strings now. Added basic unit tests. * GoTranslator: given non-string nature, added custom implementation using `fmt.Sprintf` * CommonLiterals: split generation of string "body" (without quotes) and adding quotes to it * CommonMethods: param name cleanup commit 1c7c759 Author: Mikhail Yakshin <[email protected]> Date: Sat Oct 14 08:23:12 2023 +0100 Implemented basic f-string parsing in expression language
To be honest, I don't want to overcomplicate it, nor I see a lot of value in having this escaping behavior. In Python itself, it looks pretty weird per se — e.g.:
Therefore, if I want to include a set in f-string, I'm down to unpleasant surprise:
It's possible to work around with extra spaces:
... and then you try some other combinations and it gets progressively weirder, e.g. this throws a very weird error:
while adding initial space makes it work as expected:
So, my proposal is basically avoiding going into that rabbit hole, at least yet. If anybody wants to have literal |
…wance, added lots of tests to ensure that it works as intended
…ings; modified common test to accommodate this pattern
@generalmimon If you'll have some time, can I ask you to take another look? It looks like I've accommodated all your feedback, implemented all the fixes, and rebased it off the latest master — so it looks reasonably good as a first step to merge in. Meanwhile I'll start adding .kst tests for this and once this is good to go, I'll start stage 2, which will hopefully give us better formatting options. Then, finally, we can probably deprecate |
Interpreting thumbs up as all good — therefore merging. Thanks! |
Implements step 1 out of formatting strings proposal.
fmt.Sprintf
Both are supported by unit tests.