Subset API

[pgundlach]

I think subsetting is valuable for the Lua library, so I started implementing subsetting methods. Perhaps it is useful for the main repository?

I am sure that the code is not optimal and I might be something missing. I'd be happy to get feedback if a) it is of any interest to the luaharfbuzz repo and b) where I can enhance the code.

[deepakjois]

I am a bit too far removed from this to give a comprehensive review, so will defer to the other maintainers.

It would be good to have some tests along with this functionality if applicable. I also noticed the current tests are failing, but not sure whether it's your change that's causing it.

[khaledhosny]

The subset API should be in a separate Lua module, since not all users (especially LuaTeX) will need the subset API.

[pgundlach]

The subset API should be in a separate Lua module, since not all users (especially LuaTeX) will need the subset API.

you mean a different like luaharfbuzz-subset? Same luarock repo but different sub directory?

This module is made especially for LuaTeX, since currently there is no way to use variable fonts with LuaHBTeX.

[khaledhosny]

The subset API should be in a separate Lua module, since not all users (especially LuaTeX) will need the subset API.

you mean a different like luaharfbuzz-subset? Same luarock repo but different sub directory?

Yes.

This module is made especially for LuaTeX, since currently there is no way to use variable fonts with LuaHBTeX.

I think you will need to check with LuaTeX maintainers then if they are OK with the new dependency on he subset library, and probably TeXLive as well as I don’t think they currently build it.

[pgundlach]

This module is made especially for LuaTeX, since currently there is no way to use variable fonts with LuaHBTeX.

I think you will need to check with LuaTeX maintainers then if they are OK with the new dependency on he subset library, and probably TeXLive as well as I don’t think they currently build it.

I have done that, and the maintainers encouraged me to do this.

[khaledhosny]

Excellent.

[pgundlach]

I will address the issue with the second library in a later commit, I'd like to resolve the other issues first.

Thank you very much for your time reviewing this.

[khaledhosny]

Looks good now. Other than the two standing minor issues, this needs some tests and documentation needs to be updated.

[pgundlach]

I am not sure if the lua file in the src-subset makes sense or if it should be placed in a different directory. Currently it is a stub only, but might be extended in the future.

[khaledhosny]

So we now have two separate libraries? I don’t mind either way, though I though we decided earlier to keep it as one library.

[pgundlach]

I understand the comment at #65 (comment) "The subset API should be in a separate Lua module, since not all users (especially LuaTeX) will need the subset API." in a way that two libraries should be created.

One would be easier to deploy of course. I wouldn't mind to merge both into one library.

[khaledhosny]

I’m OK with it this way. We can change it later if, say, integration in TeX Live turned out to be harder. I took the opportunity to fix a few notpicks that I commented about. I have one more nitpick to fix and this should be ready.

[khaledhosny]

I think I’m happy with the current state. Please take a look and let me know if it is OK with you as well and I’ll merge it.

[pgundlach]

I think I’m happy with the current state. Please take a look and let me know if it is OK with you as well and I’ll merge it.

This is really great. Thank you so much for the excellent work!