Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable desugaring. It's not needed anymore. #640

Closed
wants to merge 3 commits into from

Conversation

MohammedKHC0
Copy link
Contributor

Hello,

I believe that desugaring is not needed anymore.
It was enabled because tm4e was using toArray(T::new) (Introduced on API 33) but @dingyi222666 replaced it with toArray(new T[0]) which works even before API 33 so this PR disable desugaring.

Also please correct me if i am wrong.

Have a nice day!

@dingyi222666
Copy link
Contributor

There are actually a few places I didn't replace, such as:

cached = this.cached = new CompiledRule(regexps, items.stream().map(e -> e.ruleId).toArray(RuleId[]::new));

If we turn off desugaring, we need to go through the code thoroughly.

@MohammedKHC0
Copy link
Contributor Author

@dingyi222666 Hmm, you are right, but those places call toArray(IntFunction) on stream and it's supported from Android N API 24.
I think maybe we can increase min sdk to 24 on textmate module. And when it gets we implementing on kotlin we return minSdk to Android Lolipop.

I think desugring adds an extra overhead. And now we have Android 15, so maybe it's not worth supporting Android 5
Even termux's minSdk is 24.

image
Image from Android Studio linter.

Also there are some references to API 26 that can be easily fixed.
Like use new Duration(0, 0) instead of Duration.ZERO

Should i replace references to API 26 with compatible alternatives?

Or should we keep desugaring?

@dingyi222666
@Rosemoe

Of course it will be better if all of this gets rewitten again. but it would take time.

@MohammedKHC0
Copy link
Contributor Author

MohammedKHC0 commented Dec 31, 2024

Also there are some references to API 26 that can be easily fixed.
Like use new Duration(0, 0) instead of Duration.ZERO

Actullay i was wrong, the whole java.time package is not available before Android 26..

So we have 4 options.

  1. removing duration since we don't give timeLimit to tokenizeLine function.
  2. Replacing java.time with alternative Android apis.
  3. Set minSdk to 26 just like editor-lsp module.
  4. Keep every thing like it was.

@MohammedKHC0
Copy link
Contributor Author

I think this pr is pretty much useless, I think a better approach is to disable desugaring on the language-textmate module but keep it on Sora Editor Demo, And every app should implement desugaring if he would like to support old apis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants