search

markushedvall / plantuml-encoder

PlantUML encoder, works in browser & node.js
MIT License
106 stars 15 forks source link

added decoder #6

Closed mknj closed 5 years ago

mknj commented 6 years ago
alangibson commented 6 years ago

Any chance of merging this? Looks like I'm not the only one that could use it.

markushedvall commented 5 years ago

I haven't really be willing to merge this as this is supposed to just be an encoder, hence the name of the package. Wouldn't it be better to have a separate package for a decoder?

Is there some good reason to prefer having the decoding and encoding packaged together?

alangibson commented 5 years ago

Wouldn't it be better to have a separate package for a decoder?

IMHO, no it wouldn't be better. In this particular case it makes sense to combine them, if only for symmetry. If you want to encode something, it's not unreasonable to expect that you'll want to decode it eventually.

I know there's a strong tendency in the Javascript community of having extremely small packages (take for instance the notorious left-pad package on NPM), so this is just my 2 cents.

markushedvall commented 5 years ago

But in this case this is encoding for Plantuml. In most cases Plantuml itself will act as the decoder. Could you give me some examples of why you need to do both the encoding and decoding?

You also need to consider the frontend code. This is adding the inflate code which will increase the file size. So the decode and encode should be in two seperate files. They could be packaged together though.

markushedvall commented 5 years ago

After giving this some more thought. I would like to add decoding.

However the frontend code need to be dealt with. Preferably generating seperate .js files for decoding and encoding.

Additionally, I would like to use NodeJS zlib for the normal packaging and only use pako for the frontend implementation. (But I guess that's a separate issue)

markushedvall commented 5 years ago

The initial two commits of this pull request has been merged.

I have done some cleanup on master, made sure the decoding uses raw inflate and split decoding and encoding in two separate files for browsers.

This is included in the 1.3.0 release