RmlUi
RmlUi copied to clipboard
mikke89
Getting rid of the Stream class, simplify URL and FileInterface
I just don't really see the benefits of the Stream class. We are not loading files so big that they need to be streamed in parts. And we don't really need support for Write() and several other functions that are implemented.
The parsing in particular could be simplified a lot (and probably made faster) by just using a String or StringView in the necessary places.
Anybody have objections to this?
Similarly, the URL class is very big and overly generalized. Do we really need parameters, login/password fields? Perhaps it can all be replaced by a simple URL string, and a way to combine two URLs to resolve relative paths. This is lower priority though, as it doesn't really complicate the rest of the codebase. It's really big though, over 400 bytes here.
Finally, the FileInterface is really only used internally to read in whole files at a time. Thus, why not replace it with a single method to do exactly this?
I quite like the URL class and use it a lot in our web integration glue. Please keep it as it is!
Finally, the FileInterface is really only used internally to read in whole files at a time. Thus, why not replace it with a single method to do exactly this?
This would involve some allocation/memory management on the app side.. Overall, either way would work fine for me.
Sounds good, I'll keep the URL interface at least. I might try to improve the backend while keeping the API. We'll see se about the the Stream and File interface.
I rewrote my own version lua binding, and found a raw pointer for data block would be better than a Stream object.
btw, ElementDocument::ProcessHeader would try to open external script first, and pass the stream (if open success) into plugin.
https://github.com/mikke89/RmlUi/blob/master/Source/Core/ElementDocument.cpp#L133-L134
I think passing the filename into plugin is enough, so that the plugin can load the file by itself, and it can do something when the script can't open.
@cloudwu I think you are referring to this PR: https://github.com/mikke89/RmlUi/pull/144 which has now been merged :)