[Sputnik-list]luaenv.Sandbox (was : proposed versium modifications)

[pierre pracht]

Le 7 mars 08 à 09:08, Yuri Takhteyev a écrit :

>> In the same way, you can split do_lua in  2 version :
>>
>> -- will report error in err
>> values, err = luaenv.Sandbox:new{x = 1, y = 2}:do_lua_safe("z = x +  
>> y")
>>
>> -- will halt with proper error report
>> values = luaenv.Sandbox:new{x = 1, y = 2}:do_lua_fail("z = x + y")
>
> Maybe, but as long as do_lua == do_lua_fail, you can trivially
> simularte do_lua_safe with a pcall:
>
>    values, err = pcall(env.do_lua, env, "z = x + y")
>
> Considering this, I would put this off until we see that this is use
> so frequently that all the pcalls start getting on our nerves.

But actual version of didn't didn't fail (not throw an error).
It is reporting error in second return value.
And many place ignore this value.
I was trying to make caller code more clear on this point (discard  
error).

Your proposition is to make do_lua throw an error.
So existing code must catch it by a pcall if needed.
Yes ? Look a good solution for me.

> Of course, if some other project wants to use luaenv but needs the
> do_lua_safe(), they can submit a patch...

Well an other raison of the split was debugging, a global variable  
could change the comportment of do_lua depending the situation  
(development/production).

>> If each case get is own function it would make code self documented.
>
> "pcall" is more self-documenting that do_lua_safe / do_lua_fail, if  
> you ask me.

Yes, if I have well understood that do_lua would throw an error.
Better with your solution you can't invert use of safe or unsafe mode.

>> An other thing that can be clean by using an external module/Rocks :
>> Logging.
>> It is passed down to luaenv and it isn't clear how other module  
>> handle
>> it :
>>   - get one at instantiation ?
>>   - query one in sputnik global space ?
>
> That's a tricky issue that already came up with Versium.  Looking for
> Sputnik-specific globals is not an option, since the library might be
> called by some other client.  I suggest the following:
>
> local logger = self.logger or logger -- module level variable that the
> user would set with luaenv.logger = mylogger
> if logger then logger:log(...) end
>
> This way the client has two options: set a logger on the module:
>
> require"luaenv"
> luaenv.logger = logging.file(...)
>
> or set it on individual sandboxes:
>
> sandbox = luaenv.Sandbox:new(...)
> sandbox.logger = ...
>
> or not set it at all, in which case no logging occurs.

Sound good, can it use as a recurrent pattern on all module.
Code would be more clear as you will all-way see the same code pattern.
Even if unused, it would help development as you can use it for  
debugging purpose. Without need to make several change.

>> Actual scheme isn't very constant and it's a bit cumbersome to pass a
>> logger reference all the way down.
>
> Agreed.

I haven't totally understood how module play with global.
And I don't know the lua on their recommended use.
But if we allow use of _G.LOGGER :
local logger = self.logger or logger or _G.LOGGER
It would help to clean logger set-up. But it need to be a admitted use.

An other way :
local logger = self.logger or logger or pcall(require("system_logger"))
-- file: system_logger.lua
local Logger = .....
return Logger
Seen clean to me, Your take ?

- pierre
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.luaforge.net/pipermail/sputnik-list/attachments/20080307/936660d4/attachment.html