Editing Module talk:Arguments (section)

== Wrapper templates ==

{{edit protected|Module:Arguments|answered=yes}}
<!-- Begin request -->
Please make the changes at [[Special:Diff/604718144/611675481]]. This adds support for the "wrappers" option. When set, it causes it to process parent arguments only if the parent is a wrapper, or frame arguments only otherwise.
<!-- End request -->
[[User:Jackmcbarn|Jackmcbarn]] ([[User talk:Jackmcbarn|talk]]) 00:25, 5 June 2014 (UTC)
:Perhaps [[User:Mr. Stradivarius]] could check your code and apply this. &mdash;&nbsp;Martin <small>([[User:MSGJ|MSGJ]]&nbsp;·&nbsp;[[User talk:MSGJ|talk]])</small> 09:12, 5 June 2014 (UTC)
::{{ping|Jackmcbarn}} I'm not quite understanding what it means to say "if the parent is a wrapper". What kind of wrapper are we talking about? I can see that it would make sense to not try and index frame:getParent() if it's going to return nil sometimes, but the only time I can see this happening is if you call frame:getParent() on the current frame and then pass the parent frame to getArgs. Then again, there is probably something I'm missing, and I imagine that getting my head round this wrapper business will clear things up. As for general code review, {{code|1=local title, found = parent:getTitle(), false|2=lua}} seems a little dangerous to me. That would break if for some reason frame:getTitle ever switches to outputting two values (unlikely, but possible), so I would put those statements on separate lines. Also, we should probably check that <code>options.wrappers</code> is a table, so that we can give people a more informative error message if they specify something like {{code|1={wrappers = true}|2=lua}}. — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr. Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪ talk ♪]]</sup> 09:43, 5 June 2014 (UTC)
:::{{ping|Mr. Stradivarius}} A wrapper is a template that just calls a module, like [[Template:Infobox]] is a wrapper for [[Module:Infobox]] and [[Template:Edit protected]] is a wrapper for [[Module:Protected edit request]]. That's unrelated to the nil issue; I just fixed that at the same time since I had to modify that part of the code anyway. You're right that the main time getParent() would be nil is if you'd already called getParent() once, but the other time is if you call a module with a real frame through the console. I fixed the locals on the same line. Instead of throwing an error on non-tables, I made it turn it into a table, to handle the (very) common case where a module only has one wrapper. New diff is [[Special:Diff/604718144/611678252]]. [[User:Jackmcbarn|Jackmcbarn]] ([[User talk:Jackmcbarn|talk]]) 13:31, 5 June 2014 (UTC)
::::Ah, I see what this is doing now. So if getArgs is called from a wrapper template, and that wrapper is listed in options.wrappers, it only loads the parent args, thereby saving a lookup in the frame args each time a new argument is requested from the client module. And if the parent frame isn't listed in options.wrappers it assumes that a user is calling the client module directly through #invoke. That sounds like a useful feature to add. One thing I was wondering - would it complicate the code too much to not call frame:getParent() if options.frameOnly is set? I'm not sure how expensive frame:getParent is to call, but I think it would make sense to not call it if we don't have to. (But then again, frameOnly isn't used that much as an option in my experience.) — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr. Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪ talk ♪]]</sup> 00:57, 6 June 2014 (UTC)
:::::{{ping|Mr. Stradivarius}} I've made it do that. New diff is [[Special:Diff/604718144/611759842]]. [[User:Jackmcbarn|Jackmcbarn]] ([[User talk:Jackmcbarn|talk]]) 01:11, 6 June 2014 (UTC)
::::::I've found one more optimization. [[Special:Diff/604718144/611760186]]. [[User:Jackmcbarn|Jackmcbarn]] ([[User talk:Jackmcbarn|talk]]) 01:13, 6 June 2014 (UTC)
:::::::I've added some comments: [[Special:Diff/604718144/611784069]]. The code and the test cases look good to me, so if you're happy with this then I think we're ready to update the main module. — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr. Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪ talk ♪]]</sup> 06:23, 6 June 2014 (UTC)
::::::::{{ping|Mr. Stradivarius}} I'm happy with it. [[User:Jackmcbarn|Jackmcbarn]] ([[User talk:Jackmcbarn|talk]]) 13:58, 6 June 2014 (UTC)
:::::::::[[File:Yes check.svg|20px|link=]] '''Done'''<!-- Template:EP --> Ok, it's updated. Let me know if you spot any issues with it. — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr. Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪ talk ♪]]</sup> 14:25, 6 June 2014 (UTC)