Shorten JSON sent to HTTP server#228
Conversation
|
Can we get some 👀 on this PR, please? |
Before this commit, "This is a test." is parsed into
[TEXT("This"), MARKUP(" "), TEXT("is"), MARKUP(" "), TEXT("test.")],
which wastes a lot of characters when converted to JSON.
After this commit, it's parsed into the more natural form
[TEXT("This is a test.")].
real-or-random
force-pushed
the
202302-optimize-annotated-text-builder
branch
from
471b6bd to
8ad7945
Compare
|
rebased on |
|
Thanks for the work @real-or-random, I hope we can get a review of this soon. It appears that @valentjn is not very active around here these days (not blaming them, life happens and open-source projects of this scale take a lot of energy). Although this should avoid the limits for most cases, there will still be a need for splitting requests for very long documents. |
As this concerns only requests sent to external servers, I'd suggest trying in src/main/kotlin/org/bsplines/ltexls/languagetool/LanguageToolHttpInterface.kt first. I think the complex part will be to ensure that we don't hit the rate limits at the external server... |
|
Thanks for the work! I am particularly interested by this PR and I regret that it is still under review. I wonder whether we should fork the project or provide an external release package. As I have become a regular LTeX user, I don't mind contributing actively to a potential fork but I wonder whether other devs would be interested in giving a hand? |
Forking is always easier said than done. I certainly cannot promise to be more active than @valentjn. If you think an external release will help people, that's less work, of course. @valentjn |
I agree it won't be easy, especially since I am still discovering the codebase. I am a rather experienced user, but I am totally new to LTeX dev. I don't promise to deliver new features soon. However, a few devs submitted interesting PRs recently so it would be nice to review, test, and merge them. With all the open PRs, we could already prepare a rather nice release. Indeed, @valentjn if you are still around, please know I am open to giving a hand maintaining your project. I have a lot of stuff to learn about the internals of LTeX, but I really like this software and want to keep it alive. |
|
Apparently, this has been integrated in the actively developed fork https://github.com/ltex-plus/ltex-ls-plus. I wasn't aware of this fork so far. @spitzerd Are you willing to open an issue here and raise attention for your fork? Perhaps some of the other open PRs here should also be merged into the fork? |
|
Yes, your PR was merged into ltex-ls-plus. I missed to notify you here, I am sorry. Merging all other PRs is a good idea. #268, #266 and #238 are not merged yet into ltex-ls-plus. I need to review them first. All other PRs are merged. I created #319 to raise more attention. |
5 participants
This is a first step towards solving #215.
The second commit merges consecutive parts of the same type (TEXT and MARKUP) together. This is what gives most savings in the JSON output. I believe merging TEXTs is fine, but I wonder if merging MARKUPs is too aggressive. When you have a match in a long MARKUP part, possibly stretching over multiple lines, then all the lines are "marked". I doubt that this is wrong, but it's certainly a change from the current behavior. @valentjn What do you think?