Don't take a HighlightingAssets detour to build assets

[Enselic]

In the current code base, it is necessary to create a temporary HighlightingAssets instance to be able to build assets.

This PR moves the code to build assets to its own place, independent of HighlightingAssets. This allows us to significantly simplify HighlightingAssets since we can make SerializedSyntaxSet mandatory.

This also makes sense on a conceptual level. When building assets, it will never be necessary to also highlight source code. So it makes sense to stop using HighlightingAssets for this.

This means that we no longer allow clients of our API to build assets during startup. But that is very slow, and I can't imagine why anyone would want to do that. And this simplification is worth a lot in terms of flexibility. So IMHO it is worth having to deprecate API.

New public API

I currently put the new API at

• bat::assets::build

to not introduce a new module. Another option I was considering was bat::build_assets. I am open to further ideas.

Deprecated and no longer functional API

• bat::assets::HighlightingAssets::from_files
• bat::assets::HighlightingAssets::save_to_cache

Background information

As I was experimenting with zero-copy deserialization (see #1787) I bumped into the problem of having to use HighlightingAssets to build assets. To support zero-copy deserialization, it will be natural to serialize temporarily owned data. Having to take a HighlightingAssets detour really complicates things.

But IMO this PR has merits of its own, regardless of the work related to #1787.

Status of PR

I'm happy with the code, and consider it ready for CR. But I have not yet done broad verification. I would prefer to get your thoughts on this before I put much time on verification. In case you think this is way too wild :)

[sharkdp]

I like this change - thank you very much!

[Enselic]

I've done some additional sanity-checking, and everything seems to work. Do you want to do a detailed review before I merge, @sharkdp ?

[sharkdp]

I have a (nitpicky) question concerning the deprecate+always-return-error strategy. I'm not even sure if anyone is using this API, so this might be a completely hypothetical scenario.

As a user of this library, out of the three ways to handle this…

1. deprecate the existing methods and keep the existing behavior somehow
2. simply remove the existing methods
3. deprecate them and return a runtime error

… I think I would prefer 1 or 2. What I would like to avoid is to have my own software still compile*, but change its runtime behavior (option 3).

* with a warning

what do you think @Enselic?

[Enselic]

@sharkdp Good point, it might very well be better to remove the methods completely in this case.

I don't think 1 is an option for us here, because with 1, we could not make the nice simplification with SerializedSynaxSet, and getting zero-copy deserialization to work will be much more messy.

So I think we should go ahead with 2 then instead. Sounds OK?

[sharkdp]

So I think we should go ahead with 2 then instead. Sounds OK?

Yes - thank you.