MediaWiki talk:Gadget-Fill Index.js

Latest comment: 1 month ago by Ignacio Rodríguez in topic Bug

Bogus regex edit

@Inductiveload: this regex is bogus. Not sure what it was intended to do, but you're starting a named capture group ((?<) that never gets closed. And if that ! is supposed to be a negation operator, that's not (iirc) something you can do in a named capture (were you aiming for a negative lookahead assertion?).

In any case it's throwing an error in my console so it's probably entirely borked just now (haven't tested, saw it on a mainspace page). --Xover (talk) 15:48, 29 November 2020 (UTC)Reply

@Xover: it's a negative lookbehind assertion. It works for me, but perhaps it's a JS feature not all browsers have, which I had totally forgotten about. CanIUse indicates it is not fully supported. Anyway, I've just removed the line for now and I'll think of a regex that works without NLB.
Thanks for the heads up! Inductiveloadtalk/contribs 18:55, 29 November 2020 (UTC)Reply
Meh. I keep forgetting that's the lookbehind syntax; I never use lookbehinds and the syntax just makes no sense to me, being conflated with named capture groups (why the heck do those two features have similar syntax?!?) as they are. In any case, I'm on desktop Safari which evidently do not support them (probably because lookbehind is hell to implement without killing regex engine performance and the WebKit project is operating under a zero performance regressions policy). --Xover (talk) 19:49, 29 November 2020 (UTC)Reply

Bug edit

The parse_template assumes that every = sign is surrounded by spaces. That causes a bug when Book templates at Commons are not done that way and prevents the parameter to be extracted

I fixed it crudely by adding //the function assumes every = char is surrounded by a single space each side. Let's fix it crudely text = text.replace(/\s*=\s*/g, ' = ') at the start of parse_template Ignacio Rodríguez (talk) 14:42, 22 February 2024 (UTC)Reply

@Xover: --Ignacio Rodríguez (talk) 15:33, 22 February 2024 (UTC)Reply
Another bug: the template parser always skips the final parameter. This is due to line 73. You break the loop without saving the last parameter to the "params" dictionary. I solved the bug at esWS by adding
param_name = param_name.trim();
				param_content = param_content.slice(0,-2).trim();
				if ( param_name.length === 0 ) {
					// positional parameter (pos=0 is the template name)
					params[ pos_param_idx ] = param_content;
					pos_param_idx += 1;
				} else {
					param_name = param_name[ 0 ].toUpperCase() + param_name.slice( 1 );
					params[ param_name ] = param_content;
				}
before the break. --Ignacio Rodríguez (talk) 03:56, 20 March 2024 (UTC)Reply
Also: Line 511 is preventing IP users to get the functionality. You can replace it with
if ( $( '.mw-newarticletext' ).length === 0 &&  $( '.mw-newarticletextanon' ).length === 0 ) {
--Ignacio Rodríguez (talk) 04:00, 20 March 2024 (UTC)Reply

Actually all the esoteric template parsing code can be replaced with something like this:

rexp = /{{ *Book\s*\|\s*([\w ]*=*)((?:[^{}\[\]\|]|{{[^}]+}}|\[\[[^\]]+\]\])*)(?=\||}})/i;
end = false
params = {}
while (!end) {
  a1 = text.match(rexp)
  if (a1){
    param_name = a1[1].replace("=",'').trim()
    param_content = a1[2].trim()
    params[param_name]=param_content;
    text = text.replace(rexp, '{{Book');
  }
  else{
    end=true;
  }
}

The very hideous regular expression does all the work. It doesn't capture positional parameters, but who cares, they won't be useful to this gadget anyways Ignacio Rodríguez (talk) 04:43, 20 March 2024 (UTC)Reply