sensiblecodeio / pdf2msgpack

Efficiently export PDF content in an easily machine readable format
GNU General Public License v2.0
11 stars 1 forks source link

Review use of msgpack-cpp floats/doubles workaround #147

Open StevenMaude opened 1 year ago

StevenMaude commented 1 year ago

This was introduced in #146 to retain the behaviour of msgpack-cpp v4.1.1 where floats and doubles never get converted to ints. This allows us to upgrade to the latest msgpack-cpp and closes #142.

The code seems seldom touched, so this might be a reasonably sustainable workaround for now. It will need reviewing on the next msgpack-cpp upgrade, and become problematic if the code upstream starts to diverge more.

A better workaround might be one of:

StevenMaude commented 1 year ago

https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_packer

msgpack::packer packs any data to msgpack format. Currently, the following formats are supported:

https://github.com/msgpack/msgpack-c/tree/cpp_master/include/msgpack/adaptor

In addition, you can pack msgpack::object.

You can add your adaptor class template specialization to support packing objects that have the type you want to pack.

Overload declaration:

#include <msgpack.hpp>

namespace msgpack {
MSGPACK_API_VERSION_NAMESPACE(MSGPACK_DEFAULT_API_NS) {
namespace adaptor {

template <>
struct pack<the_type_you_want_to_pack> {
   template <typename Stream>
   msgpack::packer<Stream>& operator()(msgpack::packer<Stream>& o, the_type_you_want_to_pack const& v) const {
       // packing implementation.
       // o.pack_???(v.???);
       return o;
   }
};
} // namespace adaptor
} // MSGPACK_API_VERSION_NAMESPACE(MSGPACK_DEFAULT_API_NS)
} // namespace msgpack

You can call packing member functions of packer at your packing implementation. See pack manually.

See also adaptor

StevenMaude commented 11 months ago

This applies again now #219 is merged and this change is restored. For now, it's fine, but we should consider if there's a better workaround that removes the need for a fork.