rism-digital / verovio

🎵 Music notation engraving library for MEI with MusicXML and Humdrum support and various toolkits (JavaScript, Python)
https://www.verovio.org
GNU Lesser General Public License v3.0
687 stars 185 forks source link

engravingDefaults not working in toolkit #1706

Closed rettinghaus closed 3 years ago

rettinghaus commented 4 years ago

The newly added support for engravingDefaults is not working in the toolkit. I don't know how to fix this, and the toolkit documentation is still under development.

lpugin commented 4 years ago

@rettinghaus is this issue different than #1708 ? Please elaborate a bit if so. Otherwise can close it.

rettinghaus commented 4 years ago

Yes, it is different. Passing engravingDefaults: "non-existent-file" does not throw an error. Even if you pass a correct file, the function Options::Sync isn't called.

And wouldn't it make sense for the toolkit to pass complete engravingDefaults JSON within the JSON object containing the options?

lpugin commented 4 years ago

How are you passing the file? My understanding is that, as it stands, it cannot work.

The way I think you can make it work would be to:

The problem I can see with this is that it would then not work with a file with other bindings (e.g., the Python one). So other people will complain, and I agree it would be confusing. So to make it clear, we could have a --engraving-defaults-file but only for the C++ toolkit, which would work as described above, and then have --engraving-defaults only supporting JSON with JSON for other bindings. (It should actually also work for the C++ toolkit)

rettinghaus commented 3 years ago

This is also now implemented properly.