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

Handling of Encoded in Meta conversions #17

Open
ExpHP opened this issue Mar 7, 2021 · 2 comments
Open

Handling of Encoded in Meta conversions #17

ExpHP opened this issue Mar 7, 2021 · 2 comments
Labels
question Further information is requested

Comments

@ExpHP
Copy link
Owner

ExpHP commented Mar 7, 2021

No description provided.

@ExpHP ExpHP added the question Further information is requested label Mar 7, 2021
@ExpHP
Copy link
Owner Author

ExpHP commented Mar 7, 2021

Option 1: Add Encoding as context to ToMeta/FromMeta

Advantages:

Changes are generally straightforward to make.

Problems with this solution:

  • Big signature changes. ToMeta becomes fallible.
     pub trait FromMeta<'a>: Sized {
    -    fn from_meta(meta: &'a Sp<Meta>) -> Result<Self, FromMetaError<'a>>;
    +    fn from_meta(meta: &'a Sp<Meta>, cfg: &FromMetaContext) -> Result<Self, FromMetaError<'a>>;
     }
     pub trait ToMeta {
    +    fn to_meta(&self, cfg: &ToMetaContext) -> Result<Meta, SimpleError>;
     }
  • Annoyingness of cfg in impls:
    • .get_field(cfg, "field")
    • If we try to eliminate the cfg parameter of get_field by adding &'a FromMetaContext to ParseObject then we end up with annoying lifetime problems due to impl FnOnce(&mut ParseObject) parameters. Either need to make it ParseObject<'a, 'c>, or we need to make FromMeta take &'a FromMetaContext.
  • FromMeta needs a Custom(crate::error::CompileError), variant.

@ExpHP
Copy link
Owner Author

ExpHP commented Mar 7, 2021

Option 2: AnmFile<S> and similar

Summary

  • compile/decompile will use AnmFile<String>
  • write_to_stream/read_from_stream will use AnmFile<Encoded>

Advantages:

  • Changes are made almost exclusively to anm.rs, std.rs, msg.rs, and cli_defs.rs.
  • We might want a separate pass for conversion of args to blobs? This could go hand-in-hand with that. (and then that would be the only pass that needs to know anything about the encoding; nice separation of concerns)

Problems with this solution:

  • How will we do these errors if the "subdir/file.png" is now an Encoded string of unknown encoding?
    error: entry for 'subdir/file.png' is missing required field 'width'!
           (if this data is available in an existing anm file, try using `-i ANM_FILE`)
    

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant