Module talk:Color contrast
Appearance
![]() | Color Template‑class | ||||||
|
![]() | Text and/or other creative content from this version of Module:Color_contrast was copied or moved into incubator:Module:Wp/nod/Color_contrast with this edit. The former page's history now serves to provide attribution for that content in the latter page, and it must not be deleted as long as the latter page exists. |
Making the lum function accessible from modules
I've made the lum function accessible from modules by changing the code from:
function p.lum(frame) return color2lum(frame.args[1] or frame:getParent().args[1]) end
to:
function p.lum(frame) local args = frame.args[1] or frame:getParent().args[1] return p._lum(args) end function p._lum(args) return color2lum(args) end
The changes are on the sandbox and will implement them if there are no comments. --Gonnym (talk) 11:03, 6 January 2019 (UTC)
- @Gonnym: That looks like a very sensible and useful improvement. My only reservation is that passing the parameters as a table gives the person wanting to import the routine no idea of what values/types to supply to the routine without having to read through all of the code to determine them. In general I'd usually recommend either documenting a brief list of required values for the args table or passing the parameters as a list of obviously-named variables rather than a table. But that's just a minor point and shouldn't stop you from updating the main module. --RexxS (talk) 13:03, 6 January 2019 (UTC)
- @RexxS: I don't mind doing either if that helps. I just followed the current style used for the other three public functions. Do you want me to change _lum(args) to _lum(color)? --Gonnym (talk) 13:27, 6 January 2019 (UTC)
- @Gonnym: Sorry, my confusion: now I've looked harder at it, you're actually passing a string representing the colour, not a table (which is what args would most commonly indicate). I would suggest:I know we can put fuller information in the documentation, but I always suggest leaving small annotations in the code to help any re-users.
-- use {{#invoke:Color_contrast|somecolor}} directly or {{#invoke:Color_contrast}} from a wrapper template: function p.lum(frame) local color = frame.args[1] or frame:getParent().args[1] return p._lum(color) end -- This exports the function for use in other modules. -- The colour is passed as a string: function p._lum(color) return color2lum(color) end
- You could also write
p._lum = color2lum
instead of the second function definition, but setting it out explicitly, as you have done, helps whoever is importing the module to see what parameters to supply. Cheers --RexxS (talk) 15:41, 6 January 2019 (UTC)- Sure, looks good. I'll update the sandbox version. --Gonnym (talk) 15:43, 6 January 2019 (UTC)
- @Gonnym: Sorry, my confusion: now I've looked harder at it, you're actually passing a string representing the colour, not a table (which is what args would most commonly indicate). I would suggest:
- @RexxS: I don't mind doing either if that helps. I just followed the current style used for the other three public functions. Do you want me to change _lum(args) to _lum(color)? --Gonnym (talk) 13:27, 6 January 2019 (UTC)