search

zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.93k stars 1.17k forks source link

Revert "Don't expose Go timers directly to lua" #3211

Closed dmaluka closed 6 months ago

dmaluka commented 6 months ago

This reverts commit 4ffc2206eeea4a7d58bdf283e35cdf28b29b0b42.

Reason for revert: some plugins happen to use raw Go timers via time.AfterFunc(), in an unsafe way (without synchronizing their async code with micro). Let them keep doing that for now, in an unsafe way but at least without immediate crashes.

Fixes #3209

JoeKar commented 6 months ago

Shall we wrap it and add deprecation warning? In case no, then it can immediately being merged.

dmaluka commented 6 months ago

Shall we wrap it and add deprecation warning?

I can't see how. Lua is an interpreted language, all errors/warnings are runtime. Do we want users of those plugins to see a "this API is deprecated" warning every now and then?

JoeKar commented 6 months ago

Lua is an interpreted language

Well, right. So It would only be possible by exposing an Lua function including --- @deprecated. But to be honest, I don't know if it will cause what I expect it to do, since I don't use Lua that often.

In case it isn't worth the effort then forget my idea. Hopefully we remember then, that the support shall be dropped.

dmaluka commented 6 months ago

I don't use Lua often either, I don't know what is @deprecated (can't see anything about it in https://www.lua.org/manual/5.4/manual.html ?), so I don't quite understand what is your idea about.

taconi commented 6 months ago

Maybe something like this:

  L.SetField(pkg, "AfterFunc", luar.New(L, func(d time.Duration, f func()) *time.Timer {
    log.Println("The 'AfterFunc' function will be removed in version 2.0.15")
    return time.AfterFunc(d, f)
  }))

But I believe this log will only be seen run with -debug and examine the log.txt

dmaluka commented 6 months ago

But I believe this log will only be seen run with -debug and examine the log.txt

Yep.

Hopefully we remember then, that the support shall be dropped.

Pushed https://github.com/zyedidia/micro/pull/3211/commits/4718c98fc969459a938838f00770e9687601e649 with a TODO, so that we are a bit less likely to forget.

Actually, I'd be even fine if we keep these raw APIs around forever (undocumented though). Let people shoot themselves in the foot if they prefer to.