Edge Rewrite
Jump to content

Template talk:MultiReplace

Page contents not supported in other languages.
Add topic
From Wikipedia, the free encyclopedia
(Redirected from Module talk:MultiReplace/sandbox)
Latest comment: just now by Chrisahn in topic Optimization: leave changeList loop early

Template-protected edit request on 12 April 2018

[edit]

Please add {{{|safesubst:}}} before the #invoke to make the template subst cleanly. {{3x|p}}ery (talk) 19:02, 12 April 2018 (UTC)Reply

 Done by Ahecht {{3x|p}}ery (talk) 20:59, 12 April 2018 (UTC)Reply

Superfluous input sanitization

[edit]

@Ahecht: I see that you have added the following lines:

			if args[1] == '' then
				args[1] = frame:getParent().args[1]
			end

I cannot understand the purpose of this check. Empty input is perfectly valid and nothing should be done about it. If this assignment takes place, then argument 1 is taken from the parent frame, but other arguments are taken from the current frame. This appears incorrect to me and I suggest that it should be removed. Petr Matas 13:10, 25 April 2018 (UTC)Reply

The intent was to allow for a template to invoke the module with the form {{#invoke:MultiReplace|main||findtext|replacetext}} without having to do {{#invoke:MultiReplace|main|{{{1|}}}|findtext|replacetext}}. If both the module invokation and the template call are blank, it will still be processed as a blank input. It's syntax I copied from other modules, where it's used because it allows the module to distinguish between blank (|1=) and missing input in the template call, but if there isn't a need to do that here, it can be removed. --Ahecht (TALK
PAGE
) 15:15, 25 April 2018 (UTC)Reply
Forgot to ping Petr Matas. --Ahecht (TALK
PAGE
) 15:16, 25 April 2018 (UTC)
Reply
Thank you for your explanation. Distinguishing between blank and missing input will not work here, because the assignment args[1] = nil does not work as expected (args is not an ordinary table). Therefore the better readable invocation {{#invoke:MultiReplace|main|{{{1|}}}|findtext|replacetext}} becomes equivalent to the shortened one.
What I dislike about your solution, is that the behavior is different when the input happens to be empty. Consider someone using in their template {{#invoke:MultiReplace|main|{{{2|}}}|findtext|replacetext}}. Now if the template's parameter 2 is empty or missing, MultiReplace will use the template's parameter 1 instead, which is unexpected and incorrect.
A cleaner way to distinguish between empty and missing template input: Enable a module to be invoked like this: {{#invoke:MultiReplace|main|inputparam=1|findtext|replacetext}}. Such invocation is self-explanatory and the module needs no hacks, which could cause unexpected behavior. Petr Matas 18:35, 25 April 2018 (UTC)Reply
@Petr Matas: That's fine. I'll go ahead and remove it. --Ahecht (TALK
PAGE
) 18:41, 25 April 2018 (UTC)Reply
I don't get why any of these changes were necessary, as opposed to keeping the system "everyone invokes the module through {{MultiReplace}}" {{3x|p}}ery (talk) 23:51, 28 April 2018 (UTC)Reply
An important improvement is the provision for a simple call require('Module:MultiReplace').main{input, findtext, replacetext} from lua, because calling templates from there is not so straightforward. The provision for a call using {{#invoke:MultiReplace|main|input|findtext|replacetext}} was added just for completeness, because it was simple to do so. The rest really serves no purpose, because the input always has to be provided and therefore there is no point in distinguishing an empty one from a missing one. Consider it to be just a "what if..." speculation with no direct consequences. Petr Matas 09:33, 29 April 2018 (UTC)Reply

Template-protected edit request on 14 November 2022

[edit]

There is a global variable that should be fixed, as I did here. Thanks in advance. Od1n (talk) 00:44, 14 November 2022 (UTC)Reply

Requesting an additional change. Currently, if the pattern contains some HTML code, and an error message is displayed, the HTML code is interpreted, whereas it shouldn't be.
For example, {{MultiReplace|1=haystack|2=<span style="color:green">foobar</span>}}
  • Currently outputs: MultiReplace: Unpaired argument: 2 = foobar
  • But it should be: MultiReplace: Unpaired argument: 2 = <span style="color:green">foobar</span>
Another example, wiki markup is interpreted as well : {{MultiReplace|1=haystack|2=''foobar''}}
  • Currently outputs: MultiReplace: Unpaired argument: 2 = foobar
  • But it should be: MultiReplace: Unpaired argument: 2 = ''foobar''
To fix this issue, a mw.text.nowiki() should be added, like I did here. Od1n (talk) 01:14, 14 November 2022 (UTC)Reply
 Done both * Pppery * it has begun... 20:58, 15 November 2022 (UTC)Reply

Edit request 20 August 2024

[edit]

Module:String, and subsequently all templates based on it, supports 'false' / 'no' / '0' and conversely 'true' / 'yes' / '1' values, case-insensitive (and unrecognized values are handled as "true"), for the "plain" parameter in particular. (note the module can be used only through template #require's, not from other modules)

However, this module Module:MultiReplace (used by {{MultiReplace}} and many other templates, see search) only supports 'yes', case-sensitive. For convenience and security, we should support more values, mainly 'true' (which is very often used in template #invoke's of Module:String), and case-insensitive.

Note the module function can be called from other modules, though currently it seems to be called only from template #invoke's.

Below are two implementations, that support 'yes', 'true' (string), '1' (string), true (boolean) and 1 (number):

local argPlain = args.plain
if type(argPlain) == 'string' then
	argPlain = argPlain:lower()
end
local plain = (argPlain == 'yes' or argPlain == 'true' or argPlain == true or tonumber(argPlain) == 1)
local argPlain = tostring(args.plain):lower()
local plain = (argPlain == 'yes' or argPlain == 'true' or argPlain == '1')

Lua's string.lower() is very fast, thus I would prefer the 2nd implementation.

Module:Yesno could have been used, but for performance reasons, I prefer to avoid require()'ing it.

Od1n (talk) 06:45, 20 August 2024 (UTC)Reply

Using "yes" seems just fine. I don't see a strong reason to support lots of aliases. People can read the documentation  Martin (MSGJ · talk) 07:35, 20 August 2024 (UTC)Reply
It's confusing because all templates based on Module:String can use true or yes (and true is the common one), but then in {{MultiReplace}} only yes works… (edit, case in point: 1241264158) Od1n (talk) 07:49, 20 August 2024 (UTC)Reply
At the very least, could you please add support for the 'true' value, case-sensitive?
local plain = args.plain == "yes" or args.plain == "true"
Od1n (talk) 01:45, 3 September 2024 (UTC)Reply
I would just use Module:Yesno. Accepting a standard parameter set and not reinventing the wheel beats performance here. * Pppery * it has begun... 20:30, 5 September 2024 (UTC)Reply
I consider that the string manipulation templates and modules may be used many times on a given page, and for sure, they are used many times on the wiki as a whole. And numbers add up. Therefore, I am bit hesitant about adding a "require", as I'd prefer to keep the modules really lightweight. However, most of the execution time is not spent in the Lua code, but in the overhead of the #invoke (i.e. bootstrapping the execution environment), see T357199.
I can think of two solutions:
  • Just add support of 'true' for the time being. This could always be expanded later.
  • Implement the Module:Yesno, but in that case, also implement it in Module:String for consistency.
Od1n (talk) 06:54, 7 September 2024 (UTC)Reply
@Od1n Or the third option, since Module:String is used 10 times as much as this module:
  • Use local plain = args.plain and require('Module:String')._getBoolean(args.plain) or false for consistency with Module:String
--Ahecht (TALK
PAGE
)
21:39, 16 September 2024 (UTC)Reply
As denoted by the leading underscore, the _getBoolean method is intended as private, and I think it should be respected. Case in point, in my local sandboxes for fixing this issue in Module:String, my best solution so far involves modifying the _getBoolean method (namely, adding it a second parameter to provide the default value).
By the way, your code should rather be -- NO, see next message -- local plain = args.plain ~= nil and require('Module:String')._getBoolean(args.plain) or true
So, provided we adopt Module:Yesno here, the code could be:
-- NO, see next message -- local plain = args.plain ~= nil and require('Module:Yesno')(args.plain, true) or true
Yes, the default value "true" is written twice. We have to support inputs "nil" (parameter absent), "empty string" (parameter present but empty), and for extra robustness, unrecognized input falls back to "true" instead of "nil".
Od1n (talk) 00:15, 17 September 2024 (UTC)Reply
No, just encountered (again) the usual pitfall with Lua's "pseudo-ternaries" (see "Standard solution: and/or" section in this page): if require('Module:Yesno')(args.plain, true) gives "false", then args.plain ~= nil and require('Module:Yesno')(args.plain, true) or true gives "false" instead of the expected "true"…
So… we would have to write no less than this:
local plain
if args.plain ~= nil then
    plain = require('Module:Yesno')(args.plain, true)
else
    plain = true
end
Od1n (talk) 00:21, 17 September 2024 (UTC)Reply
@Od1n You don't need args.plain ~= nil because there's no need to distinguish between it being missing ("nil") or false since we're emulating Module:String which passes new_args['plain'] or false to _getBoolean. And I'm not buying that the underscore indicates a private function -- if it were private it would use local function _getBoolean( boolean_str ). The underscore is commonly used on Wikipedia to indicate functions that should only be called from Lua and not invoked from wikitext (see mw:Manual:Coding conventions/Lua#Naming conventions). --Ahecht (TALK
PAGE
)
15:51, 17 September 2024 (UTC)Reply
  • Please, let's not mix up nil with false, just because "hey, it also works in this specific case". It won't do any good. Too much trouble already…
  • I rather guess the original module developer overlooked to use local functions. Notice there are also _getParameters(), _error(), _escapePattern(); at the very least, the first two are definitely not meant to be used anywhere else. Od1n (talk) 16:30, 17 September 2024 (UTC)Reply
Od1n (talk) 16:30, 17 September 2024 (UTC)Reply
For the record, I agree with od1n here on the second point - the functions with underscores in Module:String appear to be intended to be private, not to be called from Lua. But I agree with Ahecht on the second point - that including explicit nil checks here is just wastage.
We've gotten to the point where months in, there still isn't any consensus on what, if any, edit to make to a module with 200,000 uses. Which means this request will have to be closed as not done for lack of agreement on what to do. Which is a shame, since everyone agrees something should be done, but it's the state we've come to. * Pppery * it has begun... 22:30, 19 October 2024 (UTC)Reply
Deactivating per my previous comment. * Pppery * it has begun... 02:50, 4 November 2024 (UTC)Reply

Unexpected replacement behavior

[edit]

I believe this module has a design limitation. Each time part of the string undergoes a transformation, that portion is permanently flagged and prevented from undergoing further transformations. This effectively prevents replacement rules from being chained, which I believe undermines the module's main selling point.

I would expect the following code:

{{#invoke:MultiReplace|main|abcdefghijklmnopqrstuvwxyz!|[a-z]|%0%0|a|A}}

to return:

AAbbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz!

Instead, it returns:

aabbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz!

--Grufo (talk) 11:17, 26 July 2026 (UTC)Reply

Bumping this. Here is a clearer example:
{{#invoke:MultiReplace|main
	|1=Hello world!
	|2=world|3=foo
	|4=foo|5=bar
	|6=bar|7=wind
	|8=wind|9=surf
}}
Expected result:
Hello surf!
Actual result:
Hello foo!
--Grufo (talk) 13:52, 29 July 2026 (UTC)Reply
The current behavior is fine. The behavior you expect is also reasonable, but it's simply a different requirement / specification / algorithm.
There are several different ways to implement the "replace multiple patterns at once" idea. They have in common that they go through the string from start to end and look for matches. After a pattern has been found and replaced, there are (at least) three different ways to update the search position:
1. Move past the replaced pattern, i.e. continue at the first character after the replacement
2. Move back to the front of the replaced pattern, i.e. continue at the first character of the replacement
3. Move back to the start of the string
MultiReplace currently implements 1. If I understand correctly, you think 2 or 3 are preferable. That's not unreasonable, but it's simply a different requirement. Also, it would make the code a good deal more complex, because we'd have to guard against infinite loops, e.g. {{...MultiReplace...|A|B|B|A}}.
I think MultiReplace should not be changed for now. If the need arises to implement 2 or 3 (or some other approaches), we can discuss the best way to do it. We could give MultiReplace a new parameter that specifies the behavior. Or we might prefer to create a new module to keep the complexity of the infinite loop checking etc. out of MultiReplace. — Chrisahn (talk) 16:18, 29 July 2026 (UTC)Reply
As far as I can tell, few programming languages have a "replace multiple patterns at once" function built in.
Tcl is an exception, it has string map, which implements approach 1.
For Java, there's Apache Commons' StringUtils.replaceEach() (implements approach 1) and StringUtils.replaceEachRepeatedly() (implements approach 3).
tr (Unix) also implements approach 1, but it only replaces single characters.
Chrisahn (talk) 16:46, 29 July 2026 (UTC)Reply
sed's substitution scripts are probably the best-known example of sequential replacement, and they don't behave like this module. I don't know whether this behavior is intentional. What I do know is that I've always had a hard time seeing where it would be useful, and there have been so many occasions when I couldn't use MultiReplace because of it that I recently ended up adding this minimal stand-alone clone to the Translitteratio module on lawiki, without the unwanted behavior:
ifacies.multiplex = function(frame)
	local args, len = frame.args, 0
	for key, val in ipairs(args) do len = len + 1 end
	if len % 2 == 0 then error('Modulus:Translitteratio, ‘multiplex’: Numerus argumentorum compensandus est.', 0) end
	if len < 3 then error('Modulus:Translitteratio, ‘multiplex’: Numerus argumentorum est insufficiens.', 0) end
	local txt = args[1]
	for idx = 2, len, 2 do txt = txt:gsub(args[idx], args[idx + 1]) end
	return txt
end
Writing {{#invoke:translitteratio|multiplex|Hello world!|Hello|hELLO|hELLO|Hello|Hello|hELLO}} does not result in an infinite loop. (The code currently uses string.gsub rather than mw.ustring.gsub—I might add the latter as an option in the future if the need arises.) --Grufo (talk) 17:18, 29 July 2026 (UTC)Reply
Interesting. That's yet another approach. If I understand correctly, you want {{MultiReplace|ABC|A|B|B|C}} to mean the same thing as {{replace|{{replace|ABC|A|B}}|B|C}}. Not unreasobable, but a different requirement. And of course, your approach would disappoint people who expect approach 2 (e.g. DD for {{MultiReplace|ABC|C|D|AB|C}}) or approach 3 (e.g. an empty string for {{MultiReplace|AAABBB|AB|}}). :-) — Chrisahn (talk) 17:57, 29 July 2026 (UTC)Reply
Module:MultiReplace is used on over 2 million pages, Template:MultiReplace is used on 236,000 pages. We should not change MultiReplace's behavior. It's been using the same algorithm since it was created ten years ago. As far as I can tell, you're the first user who says MultiReplace's approach is deficient.
Of course, you're free to create a new module that implements your approach. (It wouldn't make sense to add the code to MultiReplace, there'd be little shared code.) If the new module finds users, great! If it doesn't, it will probably be deleted at some point. — Chrisahn (talk) 18:46, 29 July 2026 (UTC)Reply
would disappoint”: You seem overly certain that anyone ever noticed this odd behavior in the first place. This module is used in less than 60 templates (which, incidentally, is less than twice the number of templates that used Module:Params, so apparently we are well within the "nuke" order of magnitude), and in many of those it is inappropriately used as here. Simply pointing out that the algorithm has existed for ten years does not establish that anyone intentionally depends on this particular aspect of its behavior. Could you identify a template that actually relies on the current behavior and would be broken by changing it? Given how surprising the semantics are, it seems at least as plausible that editors either never noticed them or silently worked around them. I did exactly that for years, by the way. For instance, have you ever wondered why {{Numbered verses}} was implemented using Module:Params even though it essentially operates on a single parameter? The reason is that a construct like ...|mapping_by_replacing|pattern|replacement|mapping_by_replacing|pattern|replacement|...|mapping_by_replacing|pattern|replacement|..., when only one parameter is present, is effectively equivalent to the stand-alone function I just added to lawiki's Modulus:Translitteratio (the difference only becomes apparent when multiple parameters are involved, since Params can act on multiple strings in parallel). Before Module:Params, I used nested calls to Module:String. In between, I remember trying to implement {{Numbered verses}} using MultiReplace, and I kept running into this limitation. I didn't raise the issue then, nor the next time I encountered it, nor the time after that. Eventually, though, here we are. (Incidentally, the current implementation of {{Numbered verses}} is still not fully synchronized with the CSS.) --Grufo (talk) 12:14, 30 July 2026 (UTC)Reply
"Given how surprising the semantics are" They're not. You'd like different semantics. Your choice. You're free to create a new module. — Chrisahn (talk) 14:26, 30 July 2026 (UTC)Reply
You're focusing on your own subjective judgments instead of answering my question. You're implying that this specific behavior affects 2 million pages. Could you provide a few concrete examples, or at least a few hints? I do love subjective judgements too, but they are not what I focus on. --Grufo (talk) 14:35, 30 July 2026 (UTC)Reply
You misunderstand and/or misrepresent what I said. I'm done here. — Chrisahn (talk) 14:47, 30 July 2026 (UTC)Reply
"I do love subjective judgements too, but they are not what I focus on." This whole discussion exists because you made the subjective judgement that you'd like this module to behave differently. You seem to believe your judgement is objective, but you're mistaken. — Chrisahn (talk) 15:00, 30 July 2026 (UTC)Reply
"Could you identify ..." That's not how this works. The onus is on you to prove that your suggested change wouldn't break anything. But anyway: Yes, I can. Just look at the first template in your list of 60. — Chrisahn (talk) 14:54, 30 July 2026 (UTC)Reply
Finally! Indeed, {{Lowercase title}} is a very good example that relies on this specific behavior; thank you for pointing it out:
{{#invoke:MultiReplace|main|{{lcfirst:{{PAGENAME}}}}|^(.*)(%b())$|''%1''%2|.*|''%1''}}
What it does is: “if the title ends with parenthesized text (^(.*)(%b())$), italicize only the part outside the parentheses (%1%2); otherwise italicize the whole title.” This works because, once the first rule has matched, the second rule is prevented from acting on the replacement. So it does not aim for a multiple replacement, but for a conditional replacement instead (“try this rule, otherwise that rule”), working around the fact that Lua patterns do not support adding a question mark at the end (i.e. ^(.*)(%b()?)$). In other words, the rule pairs are mutually exclusive by design rather than intended to compose. I am more convinced than before that the current name of the module is confusing for people who want to perform multiple replacements on a string. The template you pointed out is, in fact, explicitly expressing the intent to “do one single replacement”. --Grufo (talk) 16:14, 30 July 2026 (UTC)Reply
"Finally" sounds like you were waiting for me to find an example. As I said, that's not how this works. The onus is on you to prove that your suggested change won't break anything. But it looks like you didn't even look at a single use of this module. More precisely, you didn't even look at the first use that came up in your search. That's not how this works... — Chrisahn (talk) 16:33, 30 July 2026 (UTC)Reply
"the current name of the module is confusing" That's your subjective judgement. The module (and template) have had the same name for ten years. As far as I can tell, you're the first user who complains about the module's name or semantics. — Chrisahn (talk) 16:37, 30 July 2026 (UTC)Reply
As I said, that's not how this works. The onus is on you”: I am afraid that is exactly how it works. This discussion is about whether that particular behavior is desirable or whether it instead imposes too many limitations or is confusing, and whether improvements are possible. As an argument in favor of maintaining the current status quo, you brought up the fact that too many templates rely on the current behavior; that, however, is not a particularly strong argument against improving the module or against keeping the current behavior unchanged without further examination, since there are issues. So yes, the onus is on you, and it could even turn out to be a total waste of time: for example, if we assess that the current behavior limits the possible use cases or that there is room for improvement, we can totally skip thinking about dependencies and decide that from now on this module has two functions: {{#invoke:multiReplace|chain}} (like the one I added to Modulus:Translitteratio, with opportune corrections) and {{#invoke:multiReplace|singlepass}} (identical to the current main function). Then we can make {{#invoke:multiReplace|main}} an alias of {{#invoke:multiReplace|singlepass}}, and eventually decide to remove the alias in the far future. End of story and no breaking changes. --Grufo (talk) 17:52, 30 July 2026 (UTC)Reply
Again, you misrepresent what I said regarding "onus" etc. But never mind. As I said before: As far as I can tell, you are the first user in ten years who complains about missing functionality or a "confusing" name. Just create a new module that has the semantics you want. If you want to name it "MultiReplace", that's not possible, but you can just call it "MultiReplace2". Just like the modules "String" and "String2", or the templates "Diff", "Diff2", "Diff3", etc. Not super elegant, but no big deal. There's no reason to add a new function to this module instead. The semantics are quite different, they can't share much code, and the documentation will be shorter and easier to understand if the two different approaches are in separate modules. — Chrisahn (talk) 21:48, 30 July 2026 (UTC)Reply
In the time you spent discussing this module's perceived shortcomings, you could have written your new and improved module, including test cases and documentation. :-) — Chrisahn (talk) 01:05, 31 July 2026 (UTC)Reply
By the way, I find the names "chain" and "singlepass" unclear. But I admit it's difficult to come up with names that are short but also fully and clearly describe the chosen algorithm. There are simply too many different algorithms: The three I described above, the one you mentioned, and probably several others. — Chrisahn (talk) 21:52, 30 July 2026 (UTC)Reply
I'll keep thinking about this and eventually come up with an edit request. My current idea is to propose a new function (as for the names singlepass and chain, they are just the first ones that came to mind; we could also consider other names, perhaps inspired by well-known utilities that implement similar semantics, like sed and tclmap, although that would be very nerdy). Anyway, I had been wondering for years what the utility of the current behavior was. Now I've seen a couple of examples that genuinely exploit it, but they do not necessarily establish that the current behavior is irreplaceable. For instance, if a chain function (or whatever we end up calling it) existed, the current behavior of {{Lowercase title}} could be rewritten as
{{#invoke:MultiReplace|chain|{{lcfirst:{{PAGENAME}}}}|^.+$|<i>%1</i>|^(.-)(%s*%b())</i>$|%1</i>%2}}
In other words, it may turn out that the current behavior is simply a special case of a more general chaining function. Even if I eventually reach that conclusion, however, I don't think it would imply that the current behavior should disappear. Given the non-negligible number of templates that use this module, preserving the existing interface while adding a new entry point would probably be the only realistic way to extend it. --Grufo (talk) 18:24, 31 July 2026 (UTC)Reply
Why don't you just create a new module? Why do you waste your time arguing? — Chrisahn (talk) 19:51, 31 July 2026 (UTC)Reply

@Petr Matas: I noticed you were the original author of this module. First of all, thank you for writing it—it has clearly proved useful over the years. I was wondering about one less-discussed aspect of the algorithm. The module deliberately prevents replaced text from undergoing subsequent replacements, and this limits how replacement rules can be chained, in some cases causing a chain of replacements to stop after the first one. What use case or design goal was this behavior intended to address? I'm asking because I've run into several cases where this behavior prevented me from using Module:MultiReplace, and I'm curious whether it was designed with a particular class of use cases in mind. --Grufo (talk) 15:19, 30 July 2026 (UTC)Reply

Optimization: leave changeList loop early

[edit]

We can leave the following loop

for _, change in ipairs(changeList) do
	local start, stop = mw.ustring.find(input, change.pattern, pos, plain)
	if start and (start < bestStart) then
		bestStart = start
		bestStop = stop
		bestChange = change
	end
end

once we've found a pattern that starts at pos. No pattern can start before pos, so start < bestStart will never be true, so we don't need to call find() (which might be expensive for long strings or complex patterns) for the remaining patterns.

I think the following should work, but I haven't tested it:

for _, change in ipairs(changeList) do
	local start, stop = mw.ustring.find(input, change.pattern, pos, plain)
	if start and (start < bestStart) then
		bestStart = start
		bestStop = stop
		bestChange = change
		if start == pos then break end
	end
end

Chrisahn (talk) 21:54, 31 July 2026 (UTC)Reply