truth icon indicating copy to clipboard operation
truth copied to clipboard

Handling of Encoded in Meta conversions

Open ExpHP opened this issue 4 years ago • 2 comments

ExpHP avatar Mar 07 '21 18:03 ExpHP

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 avatar Mar 07 '21 18:03 ExpHP

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`)
    

ExpHP avatar Mar 07 '21 19:03 ExpHP