krakenjs / spud

A content store parser, reading a java .properties-like format
Other
14 stars 9 forks source link

Customizable State-Based Serialization/Deserialization #12

Closed kkleidal closed 9 years ago

kkleidal commented 10 years ago

allows users to define functions in an 'options' object passed to the SerializerFactory which allows the user to customize parsing/serialization behavior.

An example application would be for the L10N team to add comment-embedded commands (such as "# translate: false" marking a block of key/value pairs which shouldn't be translated). An example implementation of this can be found in the two test suites added to test/properties.js

Documentation for the "options" object has not yet been added; this pull request is intended to show the idea and receive feedback. If we decide to pursue this direction, I will work on adding documentation.

aredridel commented 10 years ago

Hm. The actual code is pretty good, but I have my doubts about the design -- it opens a lot of internal surface area to consumers, which fixes the implementation in place. How much customization would you expect of these things? What are the use cases? (I guess some detail to the use-cases is what we really need.)

kkleidal commented 10 years ago

I'll have to think more about specific use cases and talk more with my team, but for L10N, the advantage is the following: SPUD does a great job standardizing the parsing of properties files, but if we want to tweak the process in the slightest (like by looking for translate/no translate commands in the comments), we have to write an entirely new deserializer/serializer and maintain it ourselves, which means we have to constantly be updating it to keep up with SPUD. If, instead, we could write a state-based pluggin for the existing deserializer/serializer, we could get the additional functionality we need without having to maintain the trivial details of properties file serialization which SPUD already handles.

The nice part about the pluggin approach is that it doesn't add any extra data for users which don't care about things like comments (like the meta approach previously proposed did).

aredridel commented 10 years ago

Yeah. I'm not sure that's a good trade-off for plugins -- enlarging the API surface dramatically just to decouple a common use-case?

It'd make more sense to embed the function in spud -- or to redesign the format to more clearly support metadata, rather than overloading arbitrary comments.