Module talk:parameters

From Wiktionary, the free dictionary
Jump to navigation Jump to search

Lua error in Module:parameters at line 130: attempt to call global 'remove_holes' (a nil value)[edit]

I think you need to change 'remove_holes' to 'export.remove_holes'. —Игорь Тълкачь 22:11, 16 May 2015 (UTC)[reply]

+1 For example fad is currently completely out of order. JackPotte (talk) 22:42, 16 May 2015 (UTC)[reply]
Fixed. Sorry for not noticing this sooner. —CodeCat 22:43, 16 May 2015 (UTC)[reply]

Two proposals[edit]

  1. Add description to parameter tags and include it in the error text generated if it is absent. It will be much more descriptive especially for unnamed parameters.
  2. Get rid of the initial words of the error message (Lua error in Module:parameters at line) by not using builtin error function. --Dixtosa (talk) 17:59, 23 April 2016 (UTC)[reply]
@CodeCat what are your thoughts? --Dixtosa (talk) 19:03, 10 May 2016 (UTC)[reply]
What alternatives are there for error? —CodeCat 19:11, 10 May 2016 (UTC)[reply]
Simply not using it at all and having process function return the error data and letting the caller decide what to do with it. This module could provide some default behaviour on the returned data (again not exiting the script with the builtin error function but returning a string). The caller code would look like this if the error code is not desirable:
local paramErrorData = mod_params.getErrors(frame);
if paramErrorData is not nil then
 return mod_params.defaultBehaviour(paramErrorData);

--Dixtosa (talk) 20:07, 10 May 2016 (UTC)[reply]

We can do that with the error function, too. It's possible to "catch" an error and repackage it. —CodeCat 20:15, 10 May 2016 (UTC)[reply]

Quoting of %d[edit]

There is a problem with how the module performs the substitution of parameters. The %d needs quoting, as explained in the string.gsub documentation:

The character % works as an escape character: any sequence in repl of the form %n, with n between 1 and 9, stands for the value of the n-th captured substring. The sequence %0 stands for the whole match, and the sequence %% stands for a single %.

It just happens to work because the Scribunto extension does not exactly behave like the underlying lua implementation. Should this get changed our code will break.

Line 47:

patterns["^" .. mw.ustring.gsub(plain(param.list), "=", "(%d+)") .. "$"] = name

should read:

patterns["^" .. mw.ustring.gsub(plain(param.list), "=", "(%%d+)") .. "$"] = name

likewise, line 57:

patterns["^" .. mw.ustring.gsub(plain(name), "=", "(%d+)") .. "$"] = mw.ustring.gsub(name, "=", "")

should read:

patterns["^" .. mw.ustring.gsub(plain(name), "=", "(%%d+)") .. "$"] = mw.ustring.gsub(name, "=", "")

This can be verified in the lua console:

Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print(string.gsub("=input", "=", "%d"))
dinput 	1
> print(string.gsub("=input", "=", "%%d"))
%dinput	1

Lua 5.2 actually reports an error in the first case (invalid use of '%' in replacement string).

Jberkel (talk) 18:38, 24 August 2016 (UTC)[reply]

This is now fixed. – Jberkel (talk) 21:41, 29 August 2016 (UTC)[reply]

Parameters corresponding to other parameters[edit]

It seems to me that there should be a way to indicate that, for instance, the args["alt"], args["tr"], and args["t"] in {{affix}} and its sisters correspond to the args[2], and to make Module:parameters return an error when there are more args["alt"], args["tr"], or args["t"] parameters than args[2] parameters. Thus, if you had {{affix|jump|-ing|alt2=jump|alt3=-ing}}, the module would return an error, because there are three "alt" parameters, but only two 2 parameters for them to correspond to.

This could be invoked by a corresponds_to = 2 tag in the table for the args["alt"], args["tr"], and args["t"] parameter, similar to the current tag alias_of =. Module:parameters would return an error if args["alt"].maxindex, args["tr"].maxindex, or args["t"].maxindex is greater than args[2].maxindex.

I'll see if I can implement this myself; but not sure if I understand the module well enough, and if someone else would do it, I would appreciate that. — Eru·tuon 19:51, 14 February 2017 (UTC)[reply]

I don't think it's necessary. It's perfectly valid for one of them to be empty, {{m}} and its module handle this case just fine. —CodeCat 19:52, 14 February 2017 (UTC)[reply]
It's not about parameters being empty (that's handled by allow_holes or allow_empty), it's about there being n term parameters, but n + x transliteration, translation, or link text parameters, meaning that x of the transliteration, translation, or link text parameters will not be displayed, but there will be no error telling someone that they have added undisplayed and unnecessary parameters. — Eru·tuon 19:56, 14 February 2017 (UTC)[reply]
There shouldn't be an error. There's just be a request for a missing term, provided by Module:links. —CodeCat 19:57, 14 February 2017 (UTC)[reply]
That may be true in Module:nyms, but not in {{alter}} if you have entered 2 terms but 3 alt parameters. — Eru·tuon 19:59, 14 February 2017 (UTC)[reply]
Then that's an unexpected behaviour of {{alter}}. The known behaviour of {{m}} is to display "[Term?]" when the term is missing but some other parameter is provided, like say {{m|xx|tr=yy}}. {{m|xx||zz}} also has well-defined behaviour. Other templates should replicate this, or better yet, just let Module:links do its job. —CodeCat 20:02, 14 February 2017 (UTC)[reply]
It's also the behavior of {{affix}}, isn't it? If you enter {{affix|en|jump|-ing|alt2=x|alt3=y}}, the last alt is undisplayed: jump +‎ x +‎ y. Okay, I guess not. Indicating an element without an entry is allowed.
Alternatively: corresponds_to = 2 could automatically make args[2].maxindex be equal to the highest index of the parameters that correspond to it. So, if there is an alt3, or tr3, or t3, then args[2].maxindex would be 3 rather than 2 in the example above, and calling math.max(args[2].maxindex, args.alt.maxindex, args.tr.maxindex, args.t.maxindex) would be unnecessary. — Eru·tuon 20:22, 14 February 2017 (UTC)[reply]
Or what would make more sense is requiring args[2] to have allow_holes = true set, or else return an error if args[2].maxindex is less than the maxindex of the args that correspond to it, since such a situation implies that args[2] is missing at least one value. — Eru·tuon 20:28, 14 February 2017 (UTC)[reply]
While the idea makes sense, it also breaks the invariant that maxindex shows the maximum index of the current list. Really, what might be desirable instead is a "record"-type parameter. This would be a parameter that consists not of just a single value, but a set of named values. If such a parameter is also a list, it would be a list of such records. It would be more involved to implement it though, and I'm not sure how the parameters should be named. Consider also that an item within a record could be a list itself, an example is the fNg=, fNg2=, fNg3= etc parameters of {{head}}, which specify the various genders of an individual form. —CodeCat 20:36, 14 February 2017 (UTC)[reply]
Hm, you're right, so perhaps the variable could be given another name: args[2].allmaxindex. Could you show an example of what you mean with your record idea? It would be an interesting challenge to figure out how to make the module able to interpret it. — Eru·tuon 00:12, 15 February 2017 (UTC)[reply]

Module errors[edit]

@Erutuon Lots of new module errors- I think it's related to your recent changes here but I'm not sure. DTLHS (talk) 15:14, 11 March 2017 (UTC)[reply]

@DTLHS: My first edit caused errors, but I think my second edit should have solved the problem. I suspect the errors were caused by my faulty edit to {{quote-meta/source}}, which @Suzukaze-c fixed. — Eru·tuon 16:42, 11 March 2017 (UTC)[reply]

New table keys set during iteration[edit]

It seems to me that the code params[mw.ustring.gsub(name, "=", "")] = params[name] at line 54 is setting a new key in a table while it is being iterated over, which may lead to problematic behaviour (unless things have changed). --Njardarlogar (talk) 12:41, 10 June 2017 (UTC)[reply]

Aliases of required parameters not working[edit]

Having aliases of required parameters doesn't seem to work (see e.g. this module code and use 1 and use 2). --Njardarlogar (talk) 08:12, 19 July 2017 (UTC)[reply]

@Njardarlogar: Okay, I think I fixed it. Now, the required parameter can have aliases (and they will satisfy the requirement of the required parameter), but the aliases can't be required. — Eru·tuon 17:13, 19 July 2017 (UTC)[reply]
@Erutuon There's an error on immortalis. —CodeCat 18:38, 19 July 2017 (UTC)[reply]
@CodeCat: Thanks. Module:la-headword is fixed to comply with the changes now. — Eru·tuon 18:42, 19 July 2017 (UTC)[reply]
Seems to be working now, yes. Good stuff. --Njardarlogar (talk) 19:33, 19 July 2017 (UTC)[reply]

@DTLHS asked for my help in getting the |ts= parameter to work in {{head}}. When he added ts to the table of parameters that is passed to Module:parameters, the |fNalt= parameter would vanish from the output. I did some experimentation and the parameter is present in the parameters table when Module:parameters receives it, but it is not seen by the iterator function returned by pairs in the for loop that interprets the parameters table. As far as I can tell, that indicates a bug in the function returned by pairs. But I couldn't replicate this in Module:sandbox, so maybe there's something I'm missing. — Eru·tuon 05:48, 12 March 2018 (UTC)[reply]

I fixed this. The problem was that when processing params with = signs in them, the `params` table being iterated over was modified. Modifying a table while iterating over it leads to unpredictable results, including entries never being iterated over or being iterated over twice. Benwing2 (talk) 01:40, 1 March 2019 (UTC)[reply]

Avoiding mutating params[edit]

@Benwing2: There were a bunch of errors after your recent change, so I reverted it. The error I noticed first was the failure of mw.clone to clone a table loaded with mw.loadData in the declension tables in θάλασσα, but there were errors in Module:headword/templates that I didn't understand. Probably there is a way to repair your edit so that it doesn't cause errors. — Eru·tuon 01:37, 1 March 2019 (UTC)[reply]

I put back the change without the call to mw.clone(). The reason for the errors in Module:headword/templates is because you reverted the bug fix that allowed my change to Module:headword/templates to work. (See the previous entry on missing "falt".) Benwing2 (talk) 01:41, 1 March 2019 (UTC)[reply]
@Erutuon Benwing2 (talk) 01:42, 1 March 2019 (UTC)[reply]
@Benwing2: Ouch, sorry about that. Seems I misread what was going on in CAT:E. — Eru·tuon 01:46, 1 March 2019 (UTC)[reply]
@Erutuon No prob, should all be fixed now. Benwing2 (talk) 01:51, 1 March 2019 (UTC)[reply]

Acceptance of non-ASCII numbers in parameter names[edit]

In خرج, an Arabic-Indic digit ١ in {{alter|ota|خرجین|خرجینه|خورج|tr2=hurcine|tr3=hurç|tr١=hurcin}} was accepted by the Lua pattern used to handle list parameters.

The parameter happened to be treated correctly. It was put at index 1 in the list as expected. Calling tonumber on it returns nil, and the module replaces nil with 1 (basically, assumes the parameter is |tr=). But the module shouldn't accept non-ASCII numbers.

Handling parameter names with the string patterns would fix this. I think it should be safe. Most templates don't use non-ASCII parameter names, and anyway I don't think handling parameter names ever requires features of mw.ustring patterns: non-ASCII character sets or quantifiers acting on non-ASCII characters. But I'll have to think about this more.

Another fix would be replacing %d with [0-9] in all the patterns in the module. But using string functions could also improve performance. — Eru·tuon 20:54, 29 August 2019 (UTC)[reply]