[vlc-devel] [PATCH 1/3] Changes to support lua 5.2

[Rafaël Carré]

Le 2011-12-31 22:13, Kelly Anderson a écrit :
> On 12/31/2011 05:20 PM, Rafaël Carré wrote:
>> Hello,
>>
>> Le 11-12-31 18:28, Kelly Anderson a écrit :
>>
>>> --- a/modules/lua/libs/playlist.c
>>> +++ b/modules/lua/libs/playlist.c
>>> @@ -37,9 +37,6 @@
>>>   #include<vlc_interface.h>
>>>   #include<vlc_playlist.h>
>>>
>>> -#include<lua.h>         /* Low level lua C API */
>>> -#include<lauxlib.h>     /* Higher level C API */
>>> -
>>>   #include "../vlc.h"
>>>   #include "../libs.h"
>>>   #include "playlist.h"
>>> @@ -381,6 +378,7 @@ static const luaL_Reg vlclua_playlist_reg[] = {
>>>       { "loop", vlclua_playlist_loop },
>>>       { "random", vlclua_playlist_random },
>>>       { "goto", vlclua_playlist_goto },
>>> +    { "goto_", vlclua_playlist_goto },
>> The first one should be removed, it's what you do in other files.
>>
>> Anyway I don't understand why you must rename it, it is user input so it
>> should not conflict with lua's goto, does it?
>>
>> Also goto_ is quite not user-friendly; if we really can't use "goto" we
>> must find another name for it.
> 
> Yeah, in 5.2 goto is a reserved keyword, and luac errors without 
> renaming it.
> Tell me what name you want to use and I'll make the appropriate change.

I just tried lua 5.2 and there's no problem with "goto", as I expected.

This is user input, it will be interpreted by lua as data, not as code.

What is the problem you had?

>>> diff --git a/share/lua/extensions/imdb.lua b/share/lua/extensions/imdb.lua
>>> index 829948d..194a219 100644
>>> --- a/share/lua/extensions/imdb.lua
>>> +++ b/share/lua/extensions/imdb.lua
>>> @@ -237,7 +237,7 @@ function parse_resultspage(data)
>>>                   if not link then break end -- this would not be normal behavior...
>>>                   _, pos, thistitle = string.find(table, "<a href=\"" .. link .. "\"[^>]*>([^<]+)</a>", pos)
>>>                   if not thistitle then break end -- this would not be normal behavior...
>>> -                local _, _, year = string.find(table, "\((%d+)\)", pos)
>>> +                local _, _, year = string.find(table, "%((%d+)%)", pos)
>>>                   -- Add this title to the list
>>>                   count = count + 1
>>>                   local _, _, imdbID = string.find(link, "/([^/]+)/$")
>> I think all \ ->  % should go in their own patch and separated from other
>> changes
> 
> A good idea,  I'll resubmit a new patchset when you tell me what you 
> want to rename goto.

Note that those \ -> % don't depend on other patches so you can submit
them now

>> Other remarks: the patch subjects are not good, you should say what you
>> do in each patch and perhaps more details in the commit log.
>>
>> And I think configure.ac explicitely checks for lua5.1; if lua5.2 works
>> we could check for it too.
>>
>> I can take care of this if you want once your patches are committed
> 
> OK, sounds good.