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

Implement testing of to-string implementation via KST #114

Closed
wants to merge 1 commit into from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Apr 7, 2024

  • Added a new key to KST -- to-string which should contain the string which is expected from to-string implementation.
  • Added missed tests for Nim and Rust

Depends on kaitai-io/kaitai_struct_compiler#297 which introduces CppTranslator.doRawStringLiteral method to get a plain old C string literal instead of std::string object.

@generalmimon
Copy link
Member

I don't see any benefit in implementing this on the side of the KST translator when we'll eventually need this in KSC anyway - see kaitai-io/kaitai_struct#980 (-webide-representation already has this feature, so if we want to-string to be a visualizer-independent full-fledged replacement for -webide-representation, we're going to need it).

So instead of introducing this unnecessary to-string key to KST specs, it would be better to implement .to_s for structs with to-string on the KSC's side, and then we'll just do actual: struct.to_s or something.

Needless to say that your proposed to-string key only works on the top-level object (there is no way to specify which object we want to convert to string using actual: ...), which is an unnecessary limitation.

@Mingun Mingun deleted the to-string-via-kst branch April 8, 2024 16:29
@Mingun
Copy link
Contributor Author

Mingun commented Apr 13, 2024

@generalmimon, I'm not sure, how it would be possible to implement in non-ambiguous way.

id: to_string_custom
data: term_strz.bin
asserts:
  - actual: to_s
    expected: '"s1 = foo, s2 = bar"'

would mean that object should have to_s seq field or instance. What the syntax you'll propose for calling to-string?

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