search

nlbdev / nordic-epub3-dtbook-migrator

Tools for converting between a strict subset of DTBook and EPUB3.
http://nlbdev.github.io/nordic-epub3-dtbook-migrator/
GNU Lesser General Public License v2.1
8 stars 7 forks source link

Add support for mailto and tel. #381

Closed kalaspuffar closed 4 years ago

kalaspuffar commented 5 years ago

I've only changed the epub_nordic_273b rule.

Sorry for the spacing changes. Hope these are ok.

josteinaj commented 5 years ago

Looks good, thanks. I'll merge it in the next release.

josteinaj commented 5 years ago

...although I think the regex should be '^([a-z]+:/+|mailto:|tel:)' and not '^([a-z]+:/+)|mailto:|tel:'? I'll have to check when I get back to a computer.

kalaspuffar commented 5 years ago

Hi @josteinaj

Well, it could be. It depends on the parser cohesion so we don't get a value that is or the character m and so on. I can even see the possibility of using none capturing groups ^(?:[a-z]+:/+)|(?:mailto:)|(?:tel:) in order to make it clear that we want these set of characters in a sequence and not a subset of them.

kalaspuffar commented 5 years ago

Hi @josteinaj

What is the status of this pull request? Still way from the computer?

Best regards Daniel

josteinaj commented 5 years ago

Hi @kalaspuffar.

I'm by the computer again :)

Making a complete new release of the migrator takes a little time. Not much, but some. And I'm swamped with other things right now. However, I merged it into the dev branch now, gave it a tag (in case there's more to come on the dev branch in the future), and set up Docker Hub to build such tags, so now it should very soon be available as the docker image nlbdev/pipeline:2019-06-12.

Are you using the docker image directly now?

josteinaj commented 4 years ago

I'm sorry this took forever. It's merged now in nlbdev/nordic-epub3-dtbook-migrator:1.4.2 on docker hub.

Due to changes with maven central I didn't manage to release it there, and the tests doesn't work anymore either as a consequence. But it's such a minor edit, and I've tested it manually locally, so I think this will be fine.