On Tue, May 27, 2008 at 10:42 AM, catrope@svn.wikimedia.org wrote:
- Refactor Title::isValidMoveOperation() and Title::moveTo() to return an array of arrays like Title::getUserPermissionsErrors() does; other functions used by the write API have undergone similar refactoring earlier
There is absolutely no point in using all this ugly array nesting if you never use arrays with more than one element. The entire idea of using arrays (ugly as all this nesting gets) is that you can return all errors that occur, instead of just returning the first error you notice. You only ever handle arrays of one element here, and in fact you explicitly ignore all others:
call_user_func_array(array($this, 'showForm'), $error[0]);
That really sort of defeats the point.
Simetrical schreef:
There is absolutely no point in using all this ugly array nesting if you never use arrays with more than one element. The entire idea of using arrays (ugly as all this nesting gets) is that you can return all errors that occur, instead of just returning the first error you notice. You only ever handle arrays of one element here, and in fact you explicitly ignore all others:
call_user_func_array(array($this, 'showForm'), $error[0]);That really sort of defeats the point.
At least one level of arrays was necessary to be able to return parameters to messages (only 'hookaborted' uses this right now, but there could be others in the future). I agree that the second level isn't really *necessary*, but I added it for uniformity with getUserPermissionsErrors() and all those other functions that return these nested arrays. Stuff kind of gets confusing when one function returns one level while another returns two levels.
Roan Kattouw (Catrope)
On Tue, May 27, 2008 at 11:02 AM, Roan Kattouw roan.kattouw@home.nl wrote:
At least one level of arrays was necessary to be able to return parameters to messages (only 'hookaborted' uses this right now, but there could be others in the future). I agree that the second level isn't really *necessary*, but I added it for uniformity with getUserPermissionsErrors() and all those other functions that return these nested arrays. Stuff kind of gets confusing when one function returns one level while another returns two levels.
This should return one level but return all applicable errors, then.
Simetrical schreef:
On Tue, May 27, 2008 at 11:02 AM, Roan Kattouw roan.kattouw@home.nl wrote:
At least one level of arrays was necessary to be able to return parameters to messages (only 'hookaborted' uses this right now, but there could be others in the future). I agree that the second level isn't really *necessary*, but I added it for uniformity with getUserPermissionsErrors() and all those other functions that return these nested arrays. Stuff kind of gets confusing when one function returns one level while another returns two levels.
This should return one level but return all applicable errors, then.
You're not getting the point: you're forgetting about message parameters.
An error is formatted as follows: array('hookaborted', $error) for a message with parameters, or array('articleexists') for errors without parameters. An array of errors is therefore two levels deep: array(array('articleexists'), array('hookaborted', $err))
Roan Kattouw (Catrope)
On Tue, May 27, 2008 at 11:10 AM, Roan Kattouw roan.kattouw@home.nl wrote:
You're not getting the point: you're forgetting about message parameters.
An error is formatted as follows: array('hookaborted', $error) for a message with parameters, or array('articleexists') for errors without parameters. An array of errors is therefore two levels deep: array(array('articleexists'), array('hookaborted', $err))
I know that. I think I was the one who suggested the format, for exactly that reason:
http://lists.wikimedia.org/pipermail/wikitech-l/2007-July/032172.html ((2) at the bottom)
My objection is that you should actually return all the errors. Rather than
- return 'badtitletext'; + return array(array('badtitletext'));
You should have
+ $errors []= array( 'badtitletext' );
and then once the whole function is through, return $errors (which would be empty, on no errors). There's no reason to rewrite the whole function to change its return type and then not using the new format properly.
If you don't want to fix it, maybe I will at some point. I'm just remarking that it would have been easier if you had done this while you were changing the function to begin with.
Simetrical schreef:
I know that. I think I was the one who suggested the format, for exactly that reason:
http://lists.wikimedia.org/pipermail/wikitech-l/2007-July/032172.html ((2) at the bottom)
My objection is that you should actually return all the errors. Rather than
return 'badtitletext';
return array(array('badtitletext'));You should have
$errors []= array( 'badtitletext' );and then once the whole function is through, return $errors (which would be empty, on no errors). There's no reason to rewrite the whole function to change its return type and then not using the new format properly.
If you don't want to fix it, maybe I will at some point. I'm just remarking that it would have been easier if you had done this while you were changing the function to begin with.
It's not that hard to fix, I think. I'm on it.
Roan Kattouw (Catrope)
wikitech-l@lists.wikimedia.org