craigsapp / humlib

Humdrum data parsing library in C++
http://humlib.humdrum.org
BSD 2-Clause "Simplified" License
32 stars 8 forks source link

Improve option names #64

Closed WolfgangDrescher closed 1 year ago

WolfgangDrescher commented 1 year ago

Reference: https://github.com/humdrum-tools/vhv-documentation/issues/20

Should I keep --recip as an alternative? It could be confusing since it's something different than e.g. in the deg filter. So maybe not.

Note that verovio on the fb documentation page will crash because of this change (at least it did on localhost). I will wait with merging this PR until humlib is updated in the verovio repo.

craigsapp commented 1 year ago

Should I keep --recip as an alternative? It could be confusing since it's something different than e.g. in the deg filter. So maybe not.

Yes, I think it would be best to remove it because in the future, it might be useful to add a similar option to output only fb data without the score, but with some rhythmic information still available in **recip form.

Note that verovio on the fb documentation page will crash because of this change (at least it did on localhost). I will wait with merging this PR until humlib is updated in the verovio repo.

Yes, I think the problem is that the current VHV version of fb has --frequency, and if you change it to --rate before it is available in verovio, verovio will complain (on the command-line the program will print an error about an unknown option before quitting without doing anything.

craigsapp commented 1 year ago

define("r|abbreviate=b",

You can give multiple aliases for an option, so it is good to have abbr and abbreviate:

define("r|abbreviate|abbr=b",

What does the r for the single-letter alias mean? It is unusual to have a short name that is unrelated to the long name somehow. I would expect the long name for -r to be --reduce, which has a similar meaning to abbreviate, but is linked to the short form since they both start with r. --reduce also has the advantage of being shorter than --abbreviate (and easier to spell).

So you could have three aliases for the long form:

define("r|reduce|abbreviate|abbr=b",
WolfgangDrescher commented 1 year ago

Yes, I think it would be best to remove it because in the future, it might be useful to add a similar option to output only fb data without the score, but with some rhythmic information still available in **recip form.

Okay, I removed it.

Yes, I think the problem is that the current VHV version of fb has --frequency, and if you change it to --rate before it is available in verovio, verovio will complain (on the command-line the program will print an error about an unknown option before quitting without doing anything.

Exactly, but no idea why the site crashes. Probably something with the WASM. Does https://github.com/humdrum-tools/verovio-script need to be triggered to update it in VHV? Or is there a hook when verovio#develpop-humdrum is updated?

You can give multiple aliases for an option, so it is good to have abbr and abbreviate:

Okay, I readded it.

What does the r for the single-letter alias mean?

-a was taken by --accidentals next letter in the word was -b which was taken by --base so -r is simply the third letter of the word.

I would expect the long name for -r to be --reduce

This sounds good. I added it as well.

craigsapp commented 1 year ago

Does https://github.com/humdrum-tools/verovio-scrip need to be triggered to update it in VHV? Or is there a hook when verovio#develpop-humdrum is updated?

There is no automation in the update of VHV. The repository that would actually update automatically would be https://github.com/humdrum-tools/verovio-script (VHV loads the Humdrum-enabled version of verovio from that repository). This also applies to the Humdrum Notation Plugin repository https://github.com/humdrum-tools/humdrum-notation-plugin. So neither of those repositories deal directly with humlib or verovio, and they just load the WASM javascript file from the humdrum-tools/verovio-script repository.

You could set it up to be automated 😉 That would be complicated because first humlimb need to be committed to the humdrum-develop branch of verovio, and then that update would trigger a recompiling in the humdrum-tools/verovio-script repository (but only for changes in the develop-humdrum branch of verovio).

In the past I have preferred to monitor the compiling manually in case of some problem (but that rarely happens).

WolfgangDrescher commented 1 year ago

I can setup a GitHub action and dockerize it if you want. It would be inside of https://github.com/humdrum-tools/verovio-script. The action can be configured to run regularly as a cronjob (let's say once a day or so) and additional it can be triggered manually.

Have a look at this setup.

This Dockerfile will create a custom verovio build (customized for my project in this case):

https://github.com/WolfgangDrescher/verovio-humlib-docker-image/blob/071934e17db5038e4008d87abcf6fdc26683b0d4/Dockerfile#L30-L39

This workflow will publish a docker image on ghcr.io:

https://github.com/WolfgangDrescher/verovio-humlib-docker-image/blob/master/.github/workflows/ghcr.yaml

The image is now available on:

https://github.com/WolfgangDrescher/verovio-humlib-docker-image/pkgs/container/verovio-humlib-docker-image

This image then can be used in another action to run inside a docker container:

https://github.com/WolfgangDrescher/lassus-tricinium-project/blob/master/.github/workflows/stage-verovio-humlib.yaml

But maybe this could even be simplified with a single GitHub workflow that runs directly inside a emscripten/emsdk:latest container then clones verovio and humlib and copied files, builds the wasm and commits it on the gh-pages branch.