Omikhleia / ptable.sile

Paragraph boxes, framed boxes and table packages for the SILE typesetting system
MIT License
10 stars 2 forks source link

Remove dependency on silex.sile and other compatibility shims #13

Closed Omikhleia closed 9 months ago

Omikhleia commented 9 months ago

Things to address

1. The only reason why this package depends on silex.sile is for silex.compat.

It was for:

I don't think there was other reasons... We should probably stop trying to support these old SILE versions.

2. This module also provides a resilient-compat/hboxing.lua file which was providing a workaround (for other modules) for an issue fixed in SILE 0.14.9 (see SILE 1702)

We should check which module were concerned, and remove this old compatibility layer... Probably no longer applies anywhere.

3. In the parbox framing code, we still have

  if not SILE.typesetters or not SILE.typesetters.base then
    -- Compatibility shim for SILE 0.14.0..0.14.5
    parboxTypesetter = SILE.defaultTypesetter()
  else
    -- Breaking change for SILE 0.14.6 and upper
    parboxTypesetter = SILE.typesetters.base()
  end

Again, I don't think it's worth still trying to support those old versions, and I no longer test them.

How?

Maybe enforce using a recent enough version of SILE (ex. > 0.14.13) and warn if it's older

Not sure.

Moreover, it doesn't seem SILE exposes its new "semver" logic for use with 3rd party pages, though... So we can't do e.g.

if SU.semver and SU.semver(SILE.version) <= SU.semver(0.14.13) then
   SU.error("Your version of SILE is too old")
end

But we then have to require("semver")... which means doing it a a pcall for earlier versions. Doh, it defeats the point of checking anything, no?

alerque commented 9 months ago

Regarding semver: SILE can't retro-actively provide features in old versions, no. But then again neither can this library, so anything you add at this point people would have to upgrade to get. If you write new features/code and make new releases I would strongly support expecting them to be used against an equivalently new SILE version. As long as your minimum supported version of SILE is v0.14.11 or newer you can assume require("semver") will work.

If you want to make make a little more effort at warning people when they may be on too old a SILE version if they install a new version of your module, you can add semver to your packages's rockspec dependencies. Then you can safely load it and assume it is there even if not transitively supplied by SILE's dependencies.

Omikhleia commented 9 months ago

After a good night of sleep: I think I'll just remove all compat shims and update the README with a minimal requirement. On the long terms it doesn't seem I can properly handle the old compat shims without testing, and I can't afford the time.

alerque commented 9 months ago

I think that's fair. If there does become some reason to support old versions of SILE we do need to work out some way to test too. That probably means using Docker or Nix flakes and can be done, but personally I'd rather sink my time into helping people update to work with current SILE than in back porting stuff so they can stay behind.