pkulchenko / ZeroBraneStudio

Lightweight Lua-based IDE for Lua with code completion, syntax highlighting, live coding, remote debugger, and code analyzer; supports Lua 5.1, 5.2, 5.3, 5.4, LuaJIT and other Lua interpreters on Windows, macOS, and Linux
http://studio.zerobrane.com/
Other
2.61k stars 519 forks source link

Conflict between package projectsettings and syntax highlighting (master) #1044

Closed jjvbsag closed 4 years ago

jjvbsag commented 4 years ago

I observed this first on my production system, but then could reproduce this on a clean installation

Steps to reproduce:

mkdir -p ~/.zbstudio/packages
wget https://raw.githubusercontent.com/pkulchenko/ZeroBranePackage/master/projectsettings.lua -O ~/.zbstudio/packages/projectsettings.lua 
zbstudio ~/Lua/proj1 ~/Lua/proj1/proj1.lua
# and exit zbstudio
zbstudio ~/Lua/proj2 ~/Lua/proj2/proj2.lua
# and exit zbstudio

Any time, the project is changed, the project is loaded and displayed without syntag highlighting.

If you start zbstudio without explizit argument, then syntax highlighting is functional. But even if you change the project with Menu "File" / "Recent projects", syntax highlighting gets lost.

This can clearly be tracked to the package projectsetting

If I remove the package with rm ~/.zbstudio/packages/projectsettings.lua and start zbstudio, after one project change the effect is permanently gone. If I reinstall the package with above wget command, after one project change the effect is back again.

pkulchenko commented 4 years ago

@jjvbsag, thank you for the report! I suspect it's related to the recent changes in the config handling for styles, which were introduced for better dark mode support. It looks like AddConfig and RemoveConfig methods are not fully compatible with the new handling and will need to be updated.

jjvbsag commented 4 years ago

@pkulchenko Another Issue (in the same constellation) When I

I get the following crash zbs-crash

No crash appears, if I

pkulchenko commented 4 years ago

@jjvbsag, I can add a check for this condition, but this means that you don't have text field set in your color style (which you should always have). I can't reproduce this, but it's probably because you have some style-setting changes in your config file.

jjvbsag commented 4 years ago

@pkulchenko I just verified this again on a clean mint vm. (VmWare, Create new machine, install mint) Just follow the setup as described in the initial post an you'll get the crash. There is no local config file with any settings, just what I get from the installation of ZBS. IF there is something missing in any styles, then it is missing in the ZBS installation.

pkulchenko commented 4 years ago

@jjvbsag, yes, I see the issue. I expect to have a possible fix out shortly...

pkulchenko commented 4 years ago

@jjvbsag, can you try the following patch:

diff --git a/src/editor/package.lua b/src/editor/package.lua
index 2636d544..b65498d5 100644
--- a/src/editor/package.lua
+++ b/src/editor/package.lua
@@ -1272,7 +1272,11 @@ function ide:AddConfig(name, files)
   if type(files) ~= "table" then files = {files} end -- allow to pass one value
   configcache[name] = {
     config = require('mobdebug').dump(self.config, {nocode = true}),
-    configmeta = getmetatable(self.config),
+    configmeta = {
+      [0] = getmetatable(self.config),
+      styles = getmetatable(self.config.styles),
+      stylesoutshell = getmetatable(self.config.stylesoutshell),
+    },
     packages = {},
     overrides = {},
   }
@@ -1314,7 +1318,10 @@ function ide:RemoveConfig(name)
     self.config = res
     -- restore overrides
     for key, value in pairs(configcache[name].overrides) do setLongKey(self.config, key, value) end
-    if configcache[name].configmeta then setmetatable(self.config, configcache[name].configmeta) end
+    -- restore metatables on a set of (predefined) config elements
+    for metaname, metaval in pairs(configcache[name].configmeta) do
+       setmetatable(metaname == 0 and self.config or self.config[metaname], metaval)
+    end
   else
     ide:Print(("Error while restoring configuration: '%s'."):format(res))
   end

It works for me, but I want to confirm it also covers your use cases.

It maintains the list of metatables where appropriate (and I can probably extend it to make it more future proof), which should be sufficient for restore them when project settings are switched.

There may still be conflicts between styles (for example, you change the background dark/light mode and then switch projects), but I'm not going to worry about those case for now.

jjvbsag commented 4 years ago

@pkulchenko Verified: It Works :+1: Both issues with syntax highlighting and crash on global find seems to be fixed. I will monitor for some days, if there are further issues and then come back to you.

jjvbsag commented 4 years ago

@pkulchenko configmeta = getmetatable(self.config) :question: Are you aware of metatables can be protected by settings __metatable to a string ¹). :question:

In this case you'll only get the string back from getmetatable and setmetatable will throw an error. Example:

local proctected_metatable={__metatable="DontAskMe!"}
local table_with_protected_metatable=setmetatable({},proctected_metatable)
local what_youll_get=getmetatable(table_with_protected_metatable)
print("what_youll_get=",what_youll_get)
setmetatable({},what_youll_get)

Output

what_youll_get= DontAskMe!
test-metatable.lua:5: bad argument #2 to 'setmetatable' (nil or table expected)

¹) or anything else

pkulchenko commented 4 years ago

Both issues with syntax highlighting and crash on global find seems to be fixed.

Ok, great, I'll get the fix in.

Are you aware of metatables can be protected by settings __metatable to a string

Yes, but it's not going to help in this case, as the entire element that has a metatable is removed (as it needs to be), so the metatable needs to be stored and then restored on the same element (which is what the patch does).

jjvbsag commented 4 years ago

Yes, but it's not going to help in this case,

I wasn't assuming it would help. In contrast, it'll make it worse. If somebody decides to protect his metatable, then the whole save/restore mechanism will break down. I have not seen this yet in any ZBS packages, but I just wanted to make you aware of this.

Regards