greyblake / whatlang-rs

Natural language detection library for Rust. Try demo online: https://whatlang.org/
https://whatlang.org/
MIT License
969 stars 109 forks source link

Move latin table generation to compile time #113

Closed Dr-Emann closed 2 years ago

Dr-Emann commented 2 years ago

Rather than a once_cell::Lazy, build the known latin chars, and corresponding list of languages per character at compile time, with a build script. This removes the once_cell dependency.

Additionally, mark some functions on Lang/Script as const where possible. This allows e.g. building an array of [T; Lang::all().len()].

greyblake commented 2 years ago

@Dr-Emann Thanks again for your initiative and you work.

While generally I like the idea of moving things towards compile time, I'd rather stay away from build.rs.

The reason for that is that build.rs significantly decreases understandability and maintanability. I think you would I agree if I say, that it's not easy to understand what actually code generated by build.rs does just looking at build.rs. In fact, there were times when build.rs was used already in whatlang:

So I'd rather generate the Rust code with ERB and apply cargo fmt. It's the way lang.rs is generated at the moment.

Since it's already at least 3rd PR that introduces build.rs I take my blame for not documenting "the decision" about in README.

I leave the PR open for now, to give you a room and time for a contra-arguments, but it's probably unlikely that the PR will be merged in the current state.

greyblake commented 2 years ago

I am closing this for now for the reasons I have mentioned above. Hope for your understanding.