Page MenuHomePhabricator

Create a Scribunto interface
Open, LowPublic20 Story PointsFeature

Description

Variables are often used in Scribunto Modules to keep persistent data between invocations and to allow communication with templates and other wikicode. Currently the only way to do this is to use something like mw.getCurrentFrame():callParserFunction( '#vardefine', { 'foo', 'bar' } ) (or a separate extension like VariablesLua).

I'd like to suggest adding a native Scribunto support to the extension itself to improve usability and performance when using variables from Lua.

The best way to implement this would probably be using metamethods instead of just creating bindings to the already existing functions. Some examples for the suggested syntax can be found below:

  • vars.foo = 'bar' instead of vars.setVarValue( 'foo', 'bar' )
  • vars.foo instead of vars.getVarValue( 'foo' )
  • vars.foo = nil instead of vars.unbindVar( 'foo' )

Additionally it would be useful to have access to the raw variable table ($mVariables).

Event Timeline

Litzsch created this task.Aug 3 2018, 12:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 3 2018, 12:26 PM
Litzsch updated the task description. (Show Details)Aug 3 2018, 12:32 PM
Litzsch updated the task description. (Show Details)
Litzsch lowered the priority of this task from Normal to Low.Aug 3 2018, 12:39 PM

@Litzsch: Hi and welcome! Do you plan to work on this task, as you prioritized this task? If so, please set yourself as assignee. Thanks!

@Aklapper: Oh sry I didn't realize the priority was for the assignee, but I'd be happy to work on this if there's a chance of it being added to the Extension.

Litzsch claimed this task.Aug 5 2018, 5:02 PM

Thanks for clarifying! Cannot 'judge the chance' so let's better wait for a reply by a maintainer :)

Variables are often used in Scribunto Modules to keep persistent data between invocations and to allow communication with templates and other wikicode. Currently the only way to do this is to use something like mw.getCurrentFrame():callParserFunction( '#vardefine', { 'foo', 'bar' } ) (or a separate extension like VariablesLua).

It is? I'm kind of surprised to her that, since Wikimedia for example decided against activating further parser functions (StringFunctions, Variables, Loops) and decided to go with Lua instead.

I'd like to suggest adding a native Scribunto support to the extension itself to improve usability and performance when using variables from Lua.

This sounds like an interesting idea, if your use case isn't that unusual. (Are there people besided the Liquipedia wiki farm who are using a combination of Scribunto and Variables?) However, I have no knowledge of Scribunto, Lua and conditional Hook registration (ScribuntoExternalLibraries) myself, so I might be unqualified to review your patch.

The best way to implement this would probably be using metamethods instead of just creating bindings to the already existing functions. Some examples for the suggested syntax can be found below:

vars.foo = 'bar' instead of vars.setVarValue( 'foo', 'bar' )
vars.foo instead of vars.getVarValue( 'foo' )
vars.foo = nil instead of vars.unbindVar( 'foo' )

I think there should be something like local vars = mw.ext.Variables.vars needed before?

Additionally it would be useful to have access to the raw variable table ($mVariables).

I don't think the extension should allow to do something via Lua that it doesn't allow otherwise. This would be possible by implementing this proposal. (For example, unsetting variable keys.)

Yugipedia has both Variables and Scribunto installed, though we don't use them in combination currently, and I don't think we have any plans to do so either. I'd be curious to see one of the uses this issue was opened for, since to my knowledge, with careful module design you can completely eliminate the need for Variables. (I have kicked around the idea of porting Extension:Arrays to a module for a few years now, in which case Variables would be handy for saving arraystate between module invocations, but the "correct" way to do it would be to just reimplement the functionality that arrays are being used for, directly in a Lua module, meaning Variables still wouldn't be required.)

By the way, I just took a look at VariablesLua and there are two things that confuse me:

  1. I don't really understand what you're doint with $parser->getPreprocessor()->newFrame(). I don't have enough parser knowledge to definitely judge this, but Variables is using PPFrame to ensure the parser does nothing wrong if parsing a template multiple times. If you do something like this, weird bugs like the ones I fixed in T191574 might occur again. Shoudn't you use something related to mw.getCurrentFrame()?
  2. You use
if ( method_exists( 'ExtVariables', 'pf_varexists' ) ) {
  return [ ExtVariables::pf_varexists( $parser, ...$params ) ];
} else {
  return [ ExtVariables::pfObj_varexists( $parser, $parser->getPreprocessor()->newFrame(), $params ) ];
}

to make your extension work with the version of Variables before and after mentioned change. Given the nature of my change, you might get inconsistent data out of Variables, if you check it this way. You should check for pfObj_varexists first and use the old function as fallback.

Are there people besided the Liquipedia wiki farm who are using a combination of Scribunto and Variables?

It's also done regularly around the Gamepedia wiki farm (e.g. Leaguepedia, Minecraft Wiki, Dota 2 Wiki, …)

I think there should be something like local vars = mw.ext.Variables.vars needed before?

Yes, that would be needed at the top of Modules using the extension to actually load it. The examples I gave were just of a specific implementation (a table with metamethods similar to how frame.args works)

I don't think the extension should allow to do something via Lua that it doesn't allow otherwise. This would be possible by implementing this proposal. (For example, unsetting variable keys.)

  1. Unsetting variables is something I think makes sense in Lua, as there is an actual difference between nil and an empty string (so setting a variable to nil unsets it instead of setting an empty string as the value). This would be more in line with how Lua usually works. Setting something to nil and it storing an empty string just feels wrong imo.
  2. Access to the raw $mVariables table would allow implementing iteration over all variables, which imo would be useful to have but not a necessity. It would allow iteration over all variables and make debugging existing var values easier. It could also possibly be used to improve performance by loading the whole variables table only one time and working with the Lua table from there on when retrieving var values (this would create problems when used in combination with the callParserFunction method though so I'm not sure if the extra performance would be worth it).

with careful module design you can completely eliminate the need for Variables

I've looked around for quite a bit before creating this proposal and couldn't find a way to transfer data between invocations (and even templates), but should there be one it would obviously make this redundant.

I just took a look at VariablesLua and there are two things that confuse me:

  1. Passing a frame from mw.getCurrentFrame() doesn't work since it's not really a frame object. I'm aware of T191574 and am looking for a solution, but for now invokations through #invoke don't get cached anyways. The problem only exists when the #invoke is used in a template and that template gets cached.
  2. It's not my extension. I only fixed some bugs there (and implemented the var_table() function). I just referenced it to show there's a need for such an interface.

with careful module design you can completely eliminate the need for Variables

I've looked around for quite a bit before creating this proposal and couldn't find a way to transfer data between invocations (and even templates), but should there be one it would obviously make this redundant.

It was an intentional design choice on the part of Scribunto to disallow passing data between invocations, since that would have greatly increased the complexity of partial page parsing (e.g. editing a section and previewing; Parsoid also has the explicit goal of allowing for only part of a page to be reparsed on saving, again mainly for section editing). Any mechanism you find to do so which involves only vanilla MediaWiki and standard extensions (i.e. anything installed on WMF wikis) should be reported as a bug and not relied upon to continue working. For that matter, there is probably no guarantee that things such as Variables will continue to work into the indefinite future, but there at least, I am not aware of any plans to introduce breaking changes to MediaWiki.

The idea behind eliminating any need for Variables is to design your modules to work with a single invocation, instead of requiring multiple invocations per page. E.g. if you're currently using a module to build a table by outputting one table row per invocation, and passing data between rows/invocations with Variables, you can rewrite the module to instead generate the entire table in one invocation. Whether you actually follow through with this and to what degree depends on exactly what you're doing (e.g. if you're passing data between an infobox-like template at the top of a page and a navbox-like template at the bottom, you're probably not going to eliminate the need for Variables no matter what you do) and how closely you want to follow the philosophy set forth by the parser and Scribunto of no inter-invocation data passing.

I think there should be something like local vars = mw.ext.Variables.vars needed before?

Yes, that would be needed at the top of Modules using the extension to actually load it. The examples I gave were just of a specific implementation (a table with metamethods similar to how frame.args works)

I thought so, but I wanted to confirm my guess.

I don't think the extension should allow to do something via Lua that it doesn't allow otherwise. This would be possible by implementing this proposal. (For example, unsetting variable keys.)

  1. Unsetting variables is something I think makes sense in Lua, as there is an actual difference between nil and an empty string (so setting a variable to nil unsets it instead of setting an empty string as the value). This would be more in line with how Lua usually works. Setting something to nil and it storing an empty string just feels wrong imo.

Of course, but even though you can use wikitext to change a once defined variable to an empty string, #varexistswould still deliver 1. However, I'm not strictly opposed to allow unsetting Variables by Lua, given the upsides.

I just took a look at VariablesLua and there are two things that confuse me:

  1. Passing a frame from mw.getCurrentFrame() doesn't work since it's not really a frame object. I'm aware of T191574 and am looking for a solution, but for now invokations through #invoke don't get cached anyways. The problem only exists when the #invoke is used in a template and that template gets cached.
  2. It's not my extension. I only fixed some bugs there (and implemented the var_table() function). I just referenced it to show there's a need for such an interface.

I know it's just meant as a reference, but I think of it kind of as a blue print for what you are trying to do. Nevertheless, if you can in some way use similar hooks as if parsing Wikitext, this problem could be avoided. I don't want to introduce any new code that would create a loophole in which the problem handled in T191574 is happening again.

with careful module design you can completely eliminate the need for Variables

I've looked around for quite a bit before creating this proposal and couldn't find a way to transfer data between invocations (and even templates), but should there be one it would obviously make this redundant.

It was an intentional design choice on the part of Scribunto to disallow passing data between invocations, since that would have greatly increased the complexity of partial page parsing (e.g. editing a section and previewing; Parsoid also has the explicit goal of allowing for only part of a page to be reparsed on saving, again mainly for section editing). Any mechanism you find to do so which involves only vanilla MediaWiki and standard extensions (i.e. anything installed on WMF wikis) should be reported as a bug and not relied upon to continue working. For that matter, there is probably no guarantee that things such as Variables will continue to work into the indefinite future, but there at least, I am not aware of any plans to introduce breaking changes to MediaWiki.

Cite is an example of an extension that doesn't always work as it should if you parse only parts of a given page, as there can be named references defined in other sections. (Maybe it's even possible to abuse named refs as Variables.) To make this work without major problems, WMF introduced the setVolatile(); function in PPFrame, which I'm also using to prevent caching issues involving variables.

In general, I think this is a good idea and would like to see you work on it.

Litzsch added a comment.EditedAug 15 2018, 8:28 AM

I've just pushed a possible patch to Gerrit. There's a small issue with indentation since I had my editor set to use two spaces instead of tabs that I'm not sure how to fix without uploading a separate patch. Is there any way to ammend/change my patch without a separate one?

The Lua interface is very simplistic for now and just provides bindings to the already existing public functions as well as the $mVariables table, though that could certainly be expanded on to be more Lua like.

Additionally I have not found a good solution for T191574 so I would appreciate some help in that regard if someone has an idea how to fix this

I can take a look on the content of your patch next week. Which tool are you using to push patches to gerrit? I recommend git-review. If you wait for the CodeSniffer patch to be merged (and rebase), it will help you to identify code style issues.

Change 452899 had a related patch set uploaded (by MGChecker; owner: Litzsch):
[mediawiki/extensions/Variables@master] Added a Scribunto Lua interface

https://gerrit.wikimedia.org/r/452899

Anomie added a subscriber: Anomie.Aug 15 2018, 6:15 PM

Cite is an example of an extension that doesn't always work as it should if you parse only parts of a given page, as there can be named references defined in other sections.

Parsoid reimplements Cite in its own code to deal with that.

(Maybe it's even possible to abuse named refs as Variables.)

That was T63268: Abuse of Cite extension allows cross-invoke communication.

Change 452899 had a related patch set uploaded (by Litzsch; owner: Litzsch):
[mediawiki/extensions/Variables@master] Added a Scribunto Lua interface

https://gerrit.wikimedia.org/r/452899

I don't think the extension should allow to do something via Lua that it doesn't allow otherwise. This would be possible by implementing this proposal. (For example, unsetting variable keys.)

  1. Unsetting variables is something I think makes sense in Lua, as there is an actual difference between nil and an empty string (so setting a variable to nil unsets it instead of setting an empty string as the value). This would be more in line with how Lua usually works. Setting something to nil and it storing an empty string just feels wrong imo.
  2. Access to the raw $mVariables table would allow implementing iteration over all variables, which imo would be useful to have but not a necessity. It would allow iteration over all variables and make debugging existing var values easier. It could also possibly be used to improve performance by loading the whole variables table only one time and working with the Lua table from there on when retrieving var values (this would create problems when used in combination with the callParserFunction method though so I'm not sure if the extra performance would be worth it).

I put some additional thought into this and while I think unsetting should be no problem, I think you shouldn't be able to get a table of all Variables, (that this is possible in the first case is a bug resolved recently) as this could allow you to confuse extensions that are using variables internally to store their data during a parse. Mutiple unrelated uses of variables ideally shouldn't interfere with each other. However, you can add Lua functions like that get whole arrays of variables of an array of keys you put into the function.

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptAug 26 2018, 2:29 PM
MGChecker set the point value for this task to 20.Oct 25 2018, 12:10 AM
MGChecker changed the subtype of this task from "Task" to "Feature Request".Mar 1 2019, 11:35 PM