^demon, Happy-melon, ialex, ashley and I have been preparing a new class. It is intended to be replacement for the old wfMsg* functions that many seem to dislike. The most important reasons why we want to replace them are below. There is some more at [1]. * There is too many functions with too many parameters * It is easy to do the wrong thing * It is difficult to do the right thing
The new class is in my opinion now ready enough for comments and criticism. The full source code is at [2] and formatted documentation at [3]. It should be possible form the documentation to see how it is meant to be used.
Few examples are below. More examples and how they compare to old wfMsg* functions can be found at [2]. # $button = Xml::submitButton( Message::key( 'submit' )->text() ); # Message::key( 'welcome-to' )->params( $wgSitename )->parse(); # Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped(); # Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
It should be noted that it is not our intention to replace OutputPage::addWikiMsg() and ::wrapWikiMsg().
Things I'd like to have comments for: (1) Is it easy to use this class? Have we solved the three main issues given above? (2) The primary entry point is Message::key(). The syntax is little more verbose than wfMsg*'s, but much more readable imho. Do we want to use even short wrapper for the entry point? If yes, how should it be called? For example _() (often used in Gettext projects) and Msg::key() have been suggested. (3) Anything else with regards to the documentation, the code or other issues.
In the current state the class should be able to cover almost all use cases wfMsg-functions had, with some exceptions. I'd like to have some tests for this class, can somebody help with that? Obviously this quite a small change itself, but it will have a big impact when we start converting code to use these new methods. For that reason I want to get it right. I think we should proceed slowly during 1.17 cycle, for example using these only in new code and iron out all problems. Like ^demon pointed out at bug 16026 [4], we should perhaps review HTML-safety of any message we convert to use these new methods.
[1] http://www.mediawiki.org/wiki/New_messages_api [2] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Message.php [3] http://svn.wikimedia.org/doc/classMessage.html [4] https://bugzilla.wikimedia.org/show_bug.cgi?id=16026#c8
On 03/28/2010 05:06 PM, Niklas Laxström wrote:
(2) The primary entry point is Message::key(). The syntax is little more verbose than wfMsg*'s, but much more readable imho. Do we want to use even short wrapper for the entry point? If yes, how should it be called? For example _() (often used in Gettext projects) and Msg::key() have been suggested.
The "::key" function seems to just be noise, could the current "third" parameter be moved to the front.
Message::parse( 'welcome-to' )->params( $wgSitename );
I suppose it's possible that someone might want to render the same message with the same parameters in several ways, but I can't think of many use-cases.
With any code like this I worry that someone might try:
$msg = Message::key( 'example' ); $nicemsg = $msg->params( 'nice' ); $nastymsg = $msg->params( 'nasty' ); echo $nicemsg->text(); echo $nastymsg->text();
Which will break unexpectedly, but perhaps that is livable with.
(3) Anything else with regards to the documentation, the code or other issues.
The names of the methods are somewhat confusing and it'd be nice if they were consistent. (language/inContentLanguage), (parse/text/plain/escaped/parseAsBlock)
perhaps (inLanguage/inContentLanguage) and (html/text/wikitext/htmlentities/htmlblock)
The current documentation uses {{-transformation a lot, which is hard to pronounce - is it just parsing, or something else?
* As a side effect interface message status is unconditionally * turned off. What does that mean?
Conrad
2010/3/28 Conrad Irwin conrad.irwin@googlemail.com:
The "::key" function seems to just be noise, could the current "third" parameter be moved to the front.
Message::parse( 'welcome-to' )->params( $wgSitename );
Not really. The call to parse() actually triggers the parsing. With a call chain like this, the object has to either magically know when the call chain ends, or continually reparse stuff.
I suppose it's possible that someone might want to render the same message with the same parameters in several ways, but I can't think of many use-cases.
With any code like this I worry that someone might try:
$msg = Message::key( 'example' ); $nicemsg = $msg->params( 'nice' ); $nastymsg = $msg->params( 'nasty' ); echo $nicemsg->text(); echo $nastymsg->text();
Which will break unexpectedly, but perhaps that is livable with.
This won't do what you expect, true. We should probably warn against this in the docs and recommend that chains be used whenever feasible. Coders who know why this fails and want similar functionality anyway can use clone.
The current documentation uses {{-transformation a lot, which is hard to pronounce - is it just parsing, or something else?
"Brace-transformation"? It's really just preprocessing, i.e. the expansion of magic words and other stuff in {{braces}}.
Roan Kattouw (Catrope)
On 28 March 2010 23:57, Conrad Irwin conrad.irwin@googlemail.com wrote:
With any code like this I worry that someone might try:
$msg = Message::key( 'example' ); $nicemsg = $msg->params( 'nice' ); $nastymsg = $msg->params( 'nasty' ); echo $nicemsg->text(); echo $nastymsg->text();
Which will break unexpectedly, but perhaps that is livable with.
That's true, but on the other hand there documentation about the indented usage. I don't think that there is often need to call the same message with different parameters and reuse the object.
(3) Anything else with regards to the documentation, the code or other issues.
The names of the methods are somewhat confusing and it'd be nice if they were consistent. (language/inContentLanguage), (parse/text/plain/escaped/parseAsBlock)
perhaps (inLanguage/inContentLanguage) and (html/text/wikitext/htmlentities/htmlblock)
Perhaps language could be inLanguage. I don't really like (html/text/wikitext/htmlentities/htmlblock) because it emphasizes the output format, not what has been done to the string. I reiterate the meanings here so that is easier to give suggestions.
* parse: parsed wikitext * text: plain text with for working plural and grammar * escaped: same as previous but already escaped for html output * plain: the raw message text for special uses, like Special:Allmessages * parseAsBlock: normal parsing, not going to be used a lot since there is OutputPage::addWikiMsg
- As a side effect interface message status is unconditionally
- turned off.
What does that mean?
Currently the only thing that depends on ParserOptions' "InterfaceMessage" property is the gender magic word, which only works (without user parameter) in interface messages.
On 03/29/2010 10:36 AM, Niklas Laxström wrote:
On 28 March 2010 23:57, Conrad Irwin conrad.irwin@googlemail.com wrote:
The names of the methods are somewhat confusing and it'd be nice if they were consistent. (language/inContentLanguage), (parse/text/plain/escaped/parseAsBlock)
perhaps (inLanguage/inContentLanguage) and (html/text/wikitext/htmlentities/htmlblock)
Perhaps language could be inLanguage. I don't really like (html/text/wikitext/htmlentities/htmlblock) because it emphasizes the output format, not what has been done to the string. I reiterate the meanings here so that is easier to give suggestions.
- parse: parsed wikitext
- text: plain text with for working plural and grammar
- escaped: same as previous but already escaped for html output
- plain: the raw message text for special uses, like Special:Allmessages
- parseAsBlock: normal parsing, not going to be used a lot since there
is OutputPage::addWikiMsg
The problem I have with these is that you must internally be "parsing" to get text() and escaped(), both text() and plain() output plain text. And escaped() doesn't make it clear why it is being escaped. The coding conventions mention using verb phrases for functions, so perhaps all my suggestions should be parseToX or getX instead of just X.
That said, providing there's a summary somewhere, the naming of functions doesn't really matter.
Conrad
I'm definitely a fan of trying to make our message system better and more consistent.
However, I'm not really convinced by this proposal. A few points:
* I don't like that the trivial case is Message::key() – you can't beat the brevity of a global function, and I'd like to see something short and to the point. * I don't like the idea of passing parameters through a call chain – it's just not how things are really done in PHP, and it leads to all sorts of bizarre behaviour (like the bug mentioned above). It's not really in conformance with standard MediaWiki syntax. I don't see that it confers any advantages over named parameters.
I'd prefer to see a Message class (instantiated with a shorthand global function like wfMessage()) which represented an original message. Then, methods like withParameters() or withRawParameters() would create a MessageInstance class, which is like a Message class but with some parameters included. You could include the language in either the Message constructor, or you could set it with a method, perhaps setLanguage(). You could then call $msg->html(), $msg->escapedHtml(), $msg->wikitext(), $msg->escapedWikitext() or $msg->text() on EITHER the Message or MessageInstance class to get the appropriate output.
This is just off the top of my head, does anybody have any thoughts?
* Andrew Garrett agarrett@wikimedia.org [Mon, 29 Mar 2010 09:21:45 +1100]:
I'm definitely a fan of trying to make our message system better and more consistent.
However, I'm not really convinced by this proposal. A few points:
- I don't like that the trivial case is Message::key() – you can't
beat the brevity of a global function, and I'd like to see something short and to the point.
- I don't like the idea of passing parameters through a call chain –
it's just not how things are really done in PHP, and it leads to all sorts of bizarre behaviour (like the bug mentioned above). It's not really in conformance with standard MediaWiki syntax. I don't see that it confers any advantages over named parameters.
I personally like how Linker::Link() uses arrays as parameters, instead of bunch of early outdated methods. Flexible and quite powerful. Also, it's nice when tags and attributes are rendered and properly closed by output method itself, so instead of Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped(); tagname 'script' would be a call parameter.
Tags autonesting (autoclosing) is especially nice for tables and lists.
I'd prefer to see a Message class (instantiated with a shorthand global function like wfMessage()) which represented an original message. Then, methods like withParameters() or withRawParameters() would create a MessageInstance class, which is like a Message class but with some parameters included. You could include the language in either the Message constructor, or you could set it with a method, perhaps setLanguage(). You could then call $msg->html(), $msg->escapedHtml(), $msg->wikitext(), $msg->escapedWikitext() or $msg->text() on EITHER the Message or MessageInstance class to get the appropriate output.
This is just off the top of my head, does anybody have any thoughts?
Calls has to be short so the code won't grew too much. Dmitriy
On Mon, Mar 29, 2010 at 7:54 AM, Dmitriy Sintsov questpc@rambler.ru wrote:
Calls has to be short so the code won't grew too much.
I disagree. Readability counts and you only have to write a certain code once, but you will read it many times. Therefore a clear, but longer function call is preferable if it is more readable than a short one.
Bryan
* Bryan Tong Minh bryan.tongminh@gmail.com [Mon, 29 Mar 2010 10:57:49 +0200]:
On Mon, Mar 29, 2010 at 7:54 AM, Dmitriy Sintsov questpc@rambler.ru wrote:
Calls has to be short so the code won't grew too much.
I disagree. Readability counts and you only have to write a certain code once, but you will read it many times. Therefore a clear, but longer function call is preferable if it is more readable than a short one.
Bryan, sorry for going a bit offtopic (although my question is also related to generation of output). Why XML class does not allow to close _arbitrary_set_ of opened tags automatically (not a rendering of DOM-like trees, but at least a mere list of opened XML tags in a stack). There is a method SpecialAllpages::namespaceForm() with uses multiple Xml::openElement() then multiple Xml::closeElement() in "reverse order" before to return. Eg. right now I am making customization of Special:Allpages. I've added some more tag nesting to that method. Why one should not implement XML::startStack(), XML::OpenElement('table'), XML::OpenElement('div') and so on and at the end simply one XML::flushStack() call instead of set of XML::closeElement() (which are simply the manual "stack pop outs") ? Such way ::closeElement() would be much less often required to call manually in reverse order, and XML::flushStack() is called once and fixed - no matter how many XML::OpenElement() calls you made. Dmitriy
On 29 March 2010 01:21, Andrew Garrett agarrett@wikimedia.org wrote:
- I don't like that the trivial case is Message::key() – you can't
beat the brevity of a global function, and I'd like to see something short and to the point.
That's why I asked whether there needs to be a wrapper (global function) which calls Message::key() and what should it be.
- I don't like the idea of passing parameters through a call chain –
it's just not how things are really done in PHP, and it leads to all sorts of bizarre behaviour (like the bug mentioned above). It's not really in conformance with standard MediaWiki syntax. I don't see that it confers any advantages over named parameters.
If you can't do everything in one call (or chain of calls), you need to create temporary variables. I bet people are not going to like that either. Call chains are very flexible and readable (in my opinion at least). It could be possible to add parameters already in Message::key(). It actually already allows that, but as an array. I'd actually prefer if it too took varargs instead, since I dislike PHP's array syntax.
-Niklas
On Mon, Mar 29, 2010 at 5:44 PM, Niklas Laxström niklas.laxstrom@gmail.com wrote:
- I don't like the idea of passing parameters through a call chain –
it's just not how things are really done in PHP, and it leads to all sorts of bizarre behaviour (like the bug mentioned above). It's not really in conformance with standard MediaWiki syntax. I don't see that it confers any advantages over named parameters.
If you can't do everything in one call (or chain of calls), you need to create temporary variables. I bet people are not going to like that either. Call chains are very flexible and readable (in my opinion at least). It could be possible to add parameters already in Message::key(). It actually already allows that, but as an array. I'd actually prefer if it too took varargs instead, since I dislike PHP's array syntax.
I guess I don't take issue with the syntax so much as the implementation, it doesn't behave as you would expect it to.
My proposal allows the use of chained syntax without the awkward behaviour illustrated by Roan.
* Niklas Laxström niklas.laxstrom@gmail.com [Mon, 29 Mar 2010 09:44:19 +0300]:
If you can't do everything in one call (or chain of calls), you need to create temporary variables. I bet people are not going to like that either. Call chains are very flexible and readable (in my opinion at least). It could be possible to add parameters already in Message::key(). It actually already allows that, but as an array. I'd actually prefer if it too took varargs instead, since I dislike PHP's array syntax.
Tim Starling once said that arrays are good thing in PHP (I probably can find that quote - it was few months ago). Also arrays can be casted to stdobject and back, thus making the access "prettier". One can even use is_object or is_array or instanceof stdclass to detect any of these passed as parameters. Dmitriy
"Andrew Garrett" agarrett@wikimedia.org wrote in message news:2916cbf61003281521r5f0f785bje90965f3d4ec88f3@mail.gmail.com...
I'm definitely a fan of trying to make our message system better and more consistent.
However, I'm not really convinced by this proposal. A few points:
- I don't like that the trivial case is Message::key() – you can't
beat the brevity of a global function, and I'd like to see something short and to the point.
- I don't like the idea of passing parameters through a call chain –
it's just not how things are really done in PHP, and it leads to all sorts of bizarre behaviour (like the bug mentioned above). It's not really in conformance with standard MediaWiki syntax. I don't see that it confers any advantages over named parameters.
I'd prefer to see a Message class (instantiated with a shorthand global function like wfMessage()) which represented an original message. Then, methods like withParameters() or withRawParameters() would create a MessageInstance class, which is like a Message class but with some parameters included. You could include the language in either the Message constructor, or you could set it with a method, perhaps setLanguage(). You could then call $msg->html(), $msg->escapedHtml(), $msg->wikitext(), $msg->escapedWikitext() or $msg->text() on EITHER the Message or MessageInstance class to get the appropriate output.
This is just off the top of my head, does anybody have any thoughts?
I'd say that, while being academically clean, isn't really how we use messages in the codebase. Messages are basically glorified strings, some of which have placeholders and some of which don't. It's pretty much unheard of to use the same message with and then without parameters, let alone to do so in the same scope, so there's not much point in having a Message factory which spits out instance subclasses, because 99% of the time you just want one message in a particular place, which may or may not need one set of parameters. It's pretty rare to even use the same message more than once within a scope.
--HM
On Sun, Mar 28, 2010 at 12:06 PM, Niklas Laxström niklas.laxstrom@gmail.com wrote:
^demon, Happy-melon, ialex, ashley and I have been preparing a new class. It is intended to be replacement for the old wfMsg* functions that many seem to dislike. The most important reasons why we want to replace them are below. There is some more at [1].
- There is too many functions with too many parameters
- It is easy to do the wrong thing
- It is difficult to do the right thing
The new class is in my opinion now ready enough for comments and criticism. The full source code is at [2] and formatted documentation at [3]. It should be possible form the documentation to see how it is meant to be used.
Few examples are below. More examples and how they compare to old wfMsg* functions can be found at [2]. # $button = Xml::submitButton( Message::key( 'submit' )->text() ); # Message::key( 'welcome-to' )->params( $wgSitename )->parse(); # Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped(); # Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
It should be noted that it is not our intention to replace OutputPage::addWikiMsg() and ::wrapWikiMsg().
Things I'd like to have comments for: (1) Is it easy to use this class? Have we solved the three main issues given above? (2) The primary entry point is Message::key(). The syntax is little more verbose than wfMsg*'s, but much more readable imho. Do we want to use even short wrapper for the entry point? If yes, how should it be called? For example _() (often used in Gettext projects) and Msg::key() have been suggested. (3) Anything else with regards to the documentation, the code or other issues.
In the current state the class should be able to cover almost all use cases wfMsg-functions had, with some exceptions. I'd like to have some tests for this class, can somebody help with that? Obviously this quite a small change itself, but it will have a big impact when we start converting code to use these new methods. For that reason I want to get it right. I think we should proceed slowly during 1.17 cycle, for example using these only in new code and iron out all problems. Like ^demon pointed out at bug 16026 [4], we should perhaps review HTML-safety of any message we convert to use these new methods.
[1] http://www.mediawiki.org/wiki/New_messages_api [2] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Message.php [3] http://svn.wikimedia.org/doc/classMessage.html [4] https://bugzilla.wikimedia.org/show_bug.cgi?id=16026#c8
-- Niklas Laxström
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Different train of thought from the factory/naming discussions. I don't really see the need to call ->params() once you've gotten the object, it seems like a needless step. Our current format of wfMsg( $key, $param, $param2, ... ) has worked well and is easy to remember. Might I suggest:
Msg::get( $key, $param, $param2 )->parse();
I'm not commenting on the factory function, naming, or desired calling method. Just wanted to drop the (seemingly needless) call to params().
-Chad
On 3/28/10 9:06 AM, Niklas Laxström wrote:
^demon, Happy-melon, ialex, ashley and I have been preparing a new class. It is intended to be replacement for the old wfMsg* functions that many seem to dislike. The most important reasons why we want to replace them are below. There is some more at [1].
- There is too many functions with too many parameters
- It is easy to do the wrong thing
- It is difficult to do the right thing
The new class is in my opinion now ready enough for comments and criticism. The full source code is at [2] and formatted documentation at [3]. It should be possible form the documentation to see how it is meant to be used.
Few examples are below. More examples and how they compare to old wfMsg* functions can be found at [2]. # $button = Xml::submitButton( Message::key( 'submit' )->text() ); # Message::key( 'welcome-to' )->params( $wgSitename )->parse(); # Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped(); # Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
It should be noted that it is not our intention to replace OutputPage::addWikiMsg() and ::wrapWikiMsg().
Things I'd like to have comments for: (1) Is it easy to use this class? Have we solved the three main issues given above? (2) The primary entry point is Message::key(). The syntax is little more verbose than wfMsg*'s, but much more readable imho. Do we want to use even short wrapper for the entry point? If yes, how should it be called? For example _() (often used in Gettext projects) and Msg::key() have been suggested. (3) Anything else with regards to the documentation, the code or other issues.
In the current state the class should be able to cover almost all use cases wfMsg-functions had, with some exceptions. I'd like to have some tests for this class, can somebody help with that? Obviously this quite a small change itself, but it will have a big impact when we start converting code to use these new methods. For that reason I want to get it right. I think we should proceed slowly during 1.17 cycle, for example using these only in new code and iron out all problems. Like ^demon pointed out at bug 16026 [4], we should perhaps review HTML-safety of any message we convert to use these new methods.
[1] http://www.mediawiki.org/wiki/New_messages_api [2] http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Message.php [3] http://svn.wikimedia.org/doc/classMessage.html [4] https://bugzilla.wikimedia.org/show_bug.cgi?id=16026#c8
I have been experimenting with a very different approach to this problem which essentially goes like this...
// Provide a source for a message set at start-up time, this opens up other message sources in the future MessageCache::bind( 'myMessages', new MessageFile( 'i18n/myMessages.php' ) );
// Get a message set object for that set $msgSet = new MessageSet( 'myMessages' );
// Use messages as *magic* properties of the object echo "$msgSet->name says $msgSet->test";
// Use a message, passing in parameters echo $msgSet->parse( 'test-number', 4 );
I have some basic code for this, but I'm mostly just suggesting it as an alternative to using a static class like this. I think the other thing this implies is something that would be beneficial to MediaWiki - putting messages is sets (similar to gettext's domains). Anyways, just my 2 cents.
- Trevor
I like it, especially the "messages in sets" thing. Though there should be a smart way to avoid loading the same set more than once. Perhaps a static factory class which manages one instance per set name.
The magic properties also arn't bad, though I kind of like the chaining thing... $msgSet->parse( 'test-number', 4 ) isn't so nice, it would be much nicer to have $msgSet->test_number->parameters( 4 )->escape() etc. That would however mean that the magic properties wouldn't be strings, but objects themselves. not sure if that's good for performance...
just some thoughts
-- daniel
Trevor Parscal schrieb:
I have been experimenting with a very different approach to this problem which essentially goes like this...
// Provide a source for a message set at start-up time, this opens up other message sources in the future MessageCache::bind( 'myMessages', new MessageFile( 'i18n/myMessages.php' ) );
// Get a message set object for that set $msgSet = new MessageSet( 'myMessages' );
// Use messages as *magic* properties of the object echo "$msgSet->name says $msgSet->test";
// Use a message, passing in parameters echo $msgSet->parse( 'test-number', 4 );
I have some basic code for this, but I'm mostly just suggesting it as an alternative to using a static class like this. I think the other thing this implies is something that would be beneficial to MediaWiki - putting messages is sets (similar to gettext's domains). Anyways, just my 2 cents.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Great points, another idea would be that $msgSet->some_message_name returns a very simple object which has a toString method defined, as well as other things like parse() and such. Then $msgSet->test or $msgSet->test->parse( 3 ) could both return strings and work just fine. To Roan's point, using $msgSet->{test-message} isn't too bad - but switching to _ from - might start being popular anyways so a switch over might be something we could consider.
On 3/30/10 12:58 AM, Daniel Kinzler wrote:
I like it, especially the "messages in sets" thing. Though there should be a smart way to avoid loading the same set more than once. Perhaps a static factory class which manages one instance per set name.
The magic properties also arn't bad, though I kind of like the chaining thing... $msgSet->parse( 'test-number', 4 ) isn't so nice, it would be much nicer to have $msgSet->test_number->parameters( 4 )->escape() etc. That would however mean that the magic properties wouldn't be strings, but objects themselves. not sure if that's good for performance...
just some thoughts
-- daniel
Trevor Parscal schrieb:
I have been experimenting with a very different approach to this problem which essentially goes like this...
// Provide a source for a message set at start-up time, this opens up other message sources in the future MessageCache::bind( 'myMessages', new MessageFile( 'i18n/myMessages.php' ) );
// Get a message set object for that set $msgSet = new MessageSet( 'myMessages' );
// Use messages as *magic* properties of the object echo "$msgSet->name says $msgSet->test";
// Use a message, passing in parameters echo $msgSet->parse( 'test-number', 4 );
I have some basic code for this, but I'm mostly just suggesting it as an alternative to using a static class like this. I think the other thing this implies is something that would be beneficial to MediaWiki - putting messages is sets (similar to gettext's domains). Anyways, just my 2 cents.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
From: Trevor Parscal Sent: Tuesday, March 30, 2010 5:47 PM
[..] but switching to _ from - might start being popular anyways so a switch over might be something we could consider.
[sm] personally I very much prefer "-" over "_" in message keys, as the underscore is ambiguous in a MediaWiki: pagename context. Furthermore having a huge rename operation of keys just because we want to change the i18n implementation does not sound like a useful activity...
Siebrand
On 3/30/10 12:26 PM, Siebrand Mazeland wrote:
From: Trevor Parscal Sent: Tuesday, March 30, 2010 5:47 PM
[..] but switching to _ from - might start being popular anyways so a switch over might be something we could consider.
[sm] personally I very much prefer "-" over "_" in message keys, as the underscore is ambiguous in a MediaWiki: pagename context. Furthermore having a huge rename operation of keys just because we want to change the i18n implementation does not sound like a useful activity...
Siebrand
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Fair enough with the point on MediaWiki: page name context. Do you have any other thoughts about the idea?
- Trevor
On Tue, Mar 30, 2010 at 7:01 PM, Trevor Parscal tparscal@wikimedia.org wrote:
On 3/30/10 12:26 PM, Siebrand Mazeland wrote:
From: Trevor Parscal Sent: Tuesday, March 30, 2010 5:47 PM
[..] but switching to _ from - might start being popular anyways so a switch over might be something we could consider.
[sm] personally I very much prefer "-" over "_" in message keys, as the underscore is ambiguous in a MediaWiki: pagename context. Furthermore having a huge rename operation of keys just because we want to change the i18n implementation does not sound like a useful activity...
Siebrand
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Fair enough with the point on MediaWiki: page name context. Do you have any other thoughts about the idea?
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I'd like to chime in here saying I agree with using - over _. We try to stick to - consistently, and the coding guide suggests using it over _. Going the opposite direction will just confuse people :)
I'd actually like to see us rename all of the message keys (in core at least) from having _ to -. I agree that it will be disruptive, but if we do it in the span of one release (and provide a maintenance script to update page titles), it would be far less disruptive than trying to do it over time.
-Chad
On 3/30/10 4:08 PM, Chad wrote:
On Tue, Mar 30, 2010 at 7:01 PM, Trevor Parscaltparscal@wikimedia.org wrote:
On 3/30/10 12:26 PM, Siebrand Mazeland wrote:
From: Trevor Parscal Sent: Tuesday, March 30, 2010 5:47 PM
[..] but switching to _ from - might start being popular anyways so a switch over might be something we could consider.
[sm] personally I very much prefer "-" over "_" in message keys, as the underscore is ambiguous in a MediaWiki: pagename context. Furthermore having a huge rename operation of keys just because we want to change the i18n implementation does not sound like a useful activity...
Siebrand
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Fair enough with the point on MediaWiki: page name context. Do you have any other thoughts about the idea?
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I'd like to chime in here saying I agree with using - over _. We try to stick to - consistently, and the coding guide suggests using it over _. Going the opposite direction will just confuse people :)
I'd actually like to see us rename all of the message keys (in core at least) from having _ to -. I agree that it will be disruptive, but if we do it in the span of one release (and provide a maintenance script to update page titles), it would be far less disruptive than trying to do it over time.
-Chad
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
I think in either case, making things more consistent would be a good idea.
- Trevor
2010/3/29 Trevor Parscal tparscal@wikimedia.org:
// Use messages as *magic* properties of the object echo "$msgSet->name says $msgSet->test";
While I like the idea of using magic properties for message keys, there's one problem with it: the - character is extremely common in message keys, but in $msgSet->foo-bar it's interpreted as the subtraction operator. I'm not sure whether $msgSet->{foo-bar} or $key='foo-bar'; $msgSet->{$key} works, but either way we probably want to ban dashes from message keys (and replace them with e.g. _) if we do end up using this syntax. This, in turn, requires renaming thousands of messages; this is not necessarily bad software-wise, but it's annoying and a lot of work.
Roan Kattouw (Catrope)
The new Message system should allow passing a Language class representing the language that is to be used. This way, passing $wgContLang instead of a default $wgLang you could get the message in content language.
On 31.03.2010, 3:22 Platonides wrote:
The new Message system should allow passing a Language class representing the language that is to be used. This way, passing $wgContLang instead of a default $wgLang you could get the message in content language.
Already supported:
Message::key( 'foobar' )->inLanguage( $wgContLang ) Message::key( 'foobar' )->inLanguage( 'sq' )
On Sun, Mar 28, 2010 at 12:06 PM, Niklas Laxström niklas.laxstrom@gmail.com wrote:
# $button = Xml::submitButton( Message::key( 'submit' )->text() ); # Message::key( 'welcome-to' )->params( $wgSitename )->parse(); # Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped(); # Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
It's great that we're trying to think of new ways to handle messages. I like some of the aspects of the proposal, especially that it uses a clear way of specifying the output format. However, I agree with some of the others that there are issues with this that I'd like to see worked out before we switch to it.
In particular, IMO, it's much more verbose than necessary. Verbosity is okay if it adds clarity, but Message::key( 'welcome-to' )->params( $wgSitename ) doesn't seem any clearer to me than Message::key( 'welcome-to', $wgSitename ), for instance. Also, Msg would be about as intelligible as Message, but significantly easier to type (and it's what we've used for ages until now anyway).
Actually, I think the current system is pretty good, except that the variants are confusing for historical reasons. I'd stick fairly close to the general syntax of what we have now. Off the top of my head, I would suggest syntax like the following for the examples given:
# $button = Xml::submitButton( Message::key( 'submit' )->text() );
Msg::text( 'submit' );
# Message::key( 'welcome-to' )->params( $wgSitename )->parse();
Msg::parse( 'welcome-to', $wgSitename );
# Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped();
Msg::escaped( 'bad-message', array( 'raw-params' ), '<script>...</script>' );
# Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
Msg::text( 'file-log', array( 'content' ), $user, $filename );
This is where named formal parameters would be awesome, but if we're not going to use arrays for the options, overloading would work -- an array is options, a string is a parameter. (This means the usual shortcut of allowing 'content' instead of array( 'content' ) would be prohibited.)
This is kind of ugly, but options are only used in a fairly small minority of cases (I think?), while parameters are used very often, so I'd really like concise syntax for typical calls that just involve message name, parameters, and output format. With __toString() we could do
Msg::text( 'file-log', $user, $filename )->inContentLanguage()
or something, *without* adding extra syntax to the common case, but I don't see a nice way to do this without __toString(). (I agree with Werdna anyway that this feels weird and doesn't match how the rest of MediaWiki works.)
An alternative to overloading would be having a separate version of each method, like Msg::*Opts(), that accepted an extra options parameter before the parameter parameters, but that seems uglier to me.
On 04/01/2010 07:33 PM, Aryeh Gregor wrote:
Actually, I think the current system is pretty good, except that the variants are confusing for historical reasons. I'd stick fairly close to the general syntax of what we have now. Off the top of my head, I would suggest syntax like the following for the examples given:
# $button = Xml::submitButton( Message::key( 'submit' )->text() );
Msg::text( 'submit' );
# Message::key( 'welcome-to' )->params( $wgSitename )->parse();
Msg::parse( 'welcome-to', $wgSitename );
# Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped();
Msg::escaped( 'bad-message', array( 'raw-params' ), '<script>...</script>' );
# Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
Msg::text( 'file-log', array( 'content' ), $user, $filename );
This is where named formal parameters would be awesome, but if we're not going to use arrays for the options, overloading would work -- an array is options, a string is a parameter. (This means the usual shortcut of allowing 'content' instead of array( 'content' ) would be prohibited.)
If you're going to do overloading, why use an Array:
Msg::text( 'file-log', Msg::language( $wgContLang ), $user, $filename ); Msg::escaped( 'bad-message', Msg::rawParams( '<script>...</script>' ) );
The use-case for being able to add extra paramters as you're going along (one of the reasons given to me for the chaining thing) could be handled by a Msg::params() object that you could construct.
Conrad
On Thu, Apr 1, 2010 at 2:46 PM, Conrad Irwin conrad.irwin@googlemail.com wrote:
If you're going to do overloading, why use an Array:
Msg::text( 'file-log', Msg::language( $wgContLang ), $user, $filename ); Msg::escaped( 'bad-message', Msg::rawParams( '<script>...</script>' ) );
Because it's more succinct, and no less clear.
The use-case for being able to add extra paramters as you're going along (one of the reasons given to me for the chaining thing) could be handled by a Msg::params() object that you could construct.
What's a real case where anyone actually wants to do this? It's not a big deal to compute the params before the call, and it's easier to understand IMO.
On 1 April 2010 21:33, Aryeh Gregor Simetrical+wikilist@gmail.com wrote:
In particular, IMO, it's much more verbose than necessary. Verbosity is okay if it adds clarity, but Message::key( 'welcome-to' )->params( $wgSitename ) doesn't seem any clearer to me than Message::key( 'welcome-to', $wgSitename ), for instance. Also, Msg would be about as intelligible as Message, but significantly easier to type (and it's what we've used for ages until now anyway).
Message::key( 'welcome-to', $wgSitename ) works already. Renaming the class is easy and I don't mind if it is Message or Msg.
Actually, I think the current system is pretty good, except that the variants are confusing for historical reasons. I'd stick fairly close to the general syntax of what we have now. Off the top of my head, I would suggest syntax like the following for the examples given:
I don't think the current system is good. Details below:
# $button = Xml::submitButton( Message::key( 'submit' )->text() );
Msg::text( 'submit' );
If Xml::submitButton() would be intelligent, it could just be Xml::submitButton( Msg::key( 'submit' ) ); This is related to the pain that is Xml::element( 'foo', array() wfMsgExt( 'bar', 'escapenoentities' ) );
# Message::key( 'welcome-to' )->params( $wgSitename )->parse();
Msg::parse( 'welcome-to', $wgSitename );
All is fine until...
# Message::key( 'bad-message' )->rawParams( '<script>...</script>' )->escaped();
Msg::escaped( 'bad-message', array( 'raw-params' ), '<script>...</script>' );
Bang! That would break one thing we was trying to fix. "rawness" should be per param, not per message. There is Message::rawParam() to wrap those, but I don't like it as much as chaining.
# Message::key( 'file-log' )->params( $user, $filename )->inContentLanguage()->text();
Msg::text( 'file-log', array( 'content' ), $user, $filename );
This is where named formal parameters would be awesome, but if we're not going to use arrays for the options, overloading would work -- an array is options, a string is a parameter. (This means the usual shortcut of allowing 'content' instead of array( 'content' ) would be prohibited.)
The other things I don't like flat array of options that do different unrelated things. I also don't like they perhaps worst ever array syntax for lists in PHP. I don't really like that we just stuff all the stuff in one function call and do with some magic with them either.
I'm not really sure *what* are the issues in current proposal that people want fixed. I've just seen dozens of suggestions to do something very different. I guess they could be: * Syntax is too verbose -- Suggested: Msg instead of Message -- Implemented: allow ::key( 'key', $param1, $param2... ) * Chaining is confusing/different from everything else -- Class has good documentation * Chaining can be used in a way that may cause unexpected behaviour -- Class has good documentation -- IMO, the risk is very low that someone needs to do something special and get it wrong too * Naming of the functions is not consistent -- Open to suggestions here
Things that seem to be ok: * Thy syntax is readable * It has all the features that were initially specified
If we just replace ::key() with ::format() and tack options somewhere in the params, we just have new set of wfMsg* functions, just with better names. Is that what we want? That also means we can't do any cool stuff because there wont by any Message objects. With objects functions can see that they are getting a message instead of just any random string. There is also many places in the code where messages are passed to functions. We could pass Message objects there instead of array( 'key', 'param', 'param' ), which is quite inflexible.
What do others think of the above?
-Niklas
"Niklas Laxström" niklas.laxstrom@gmail.com wrote in message news:k2l501f5b5d1004011239ja616a33cjb60ac8eafbd5f442@mail.gmail.com...
If we just replace ::key() with ::format() and tack options somewhere in the params, we just have new set of wfMsg* functions, just with better names. Is that what we want? That also means we can't do any cool stuff because there wont by any Message objects. With objects functions can see that they are getting a message instead of just any random string. There is also many places in the code where messages are passed to functions. We could pass Message objects there instead of array( 'key', 'param', 'param' ), which is quite inflexible.
I think the existence of a Message object that can be instantiated and passed around, is a very important benefit. As you say, if we go down the other route, we just end up with the wfMsg functions in a different namespace.
--HM
On Thu, Apr 1, 2010 at 3:39 PM, Niklas Laxström niklas.laxstrom@gmail.com wrote:
If Xml::submitButton() would be intelligent, it could just be Xml::submitButton( Msg::key( 'submit' ) );
That would automatically HTML-escape it? That would be nice, but you'd have to modify an awful lot of functions for that to work reliably enough to not seem like voodoo magic. What would be much awesomer is if we could use __toString() and make that default to HTML-escaping.
Bang! That would break one thing we was trying to fix. "rawness" should be per param, not per message. There is Message::rawParam() to wrap those, but I don't like it as much as chaining.
I like it more than chaining. :) Chaining looks weird, IMO. And that *is* a valid criticism as long as we're debating aesthetics, although I understand that it's probably frustrating.
The other things I don't like flat array of options that do different unrelated things.
How is that different from a flat chain of method calls that do different unrelated things?
I also don't like they perhaps worst ever array syntax for lists in PHP.
Seems simpler to me than chaining, nonetheless.
I don't really like that we just stuff all the stuff in one function call and do with some magic with them either.
I prefer that style. It's certainly what we tend to use elsewhere in MediaWiki, and PHP encourages it.
- Chaining is confusing/different from everything else
-- Class has good documentation
- Chaining can be used in a way that may cause unexpected behaviour
-- Class has good documentation
Good documentation is not a substitute for behaving as expected. It should behave consistently with other functions *and* also have good documentation.
If we just replace ::key() with ::format() and tack options somewhere in the params, we just have new set of wfMsg* functions, just with better names. Is that what we want?
I have no problem with that.
That also means we can't do any cool stuff because there wont by any Message objects. With objects functions can see that they are getting a message instead of just any random string.
What are some good uses of this that won't magically work for some methods but fail in 95% of the others, so that you have to memorize which exact methods support Message parameters and which don't? And that also won't require adding boilerplate Message-handling code to hundreds of methods?
There is also many places in the code where messages are passed to functions. We could pass Message objects there instead of array( 'key', 'param', 'param' ), which is quite inflexible.
Where are some examples of this, and some sample benefits?
On Thu, Apr 1, 2010 at 3:59 PM, Happy-melon happy-melon@live.com wrote:
I think the existence of a Message object that can be instantiated and passed around, is a very important benefit. As you say, if we go down the other route, we just end up with the wfMsg functions in a different namespace.
We'd end up with *cleaned-up* wfMsg() functions in a different namespace. That's the only benefit I see as compelling, that they be clean and easy to use. I have no problem with the general approach taken by wfMsg*(), they've just become rather crufty and confusing over the years. Compare Linker::link() to all the methods it replaced. It's not fundamentally different, it's just cleaner and easier to understand.
I don't see a lot we can do with objects without adding excessive clutter, given PHP's deficiencies. In almost all cases you really do just want plain old strings, and requiring extra method calls everywhere for the sake of uncommon cases is uneconomical.
Of course, once we require a version of PHP that supports __toString(), we could transparently alter all existing message-related functions (including wfMsg() and friends) to return some Message object with a __toString() method. That would give us the best of all worlds. Until then, if you really wanted objects you could create them for whatever uses you have for them, but the vast majority of callers don't need any kind of object and would really just like a string.
"Aryeh Gregor" Simetrical+wikilist@gmail.com wrote in message news:p2n7c2a12e21004011306k9308cbfasb8aa0b688705ec93@mail.gmail.com...
That also means we can't do any cool stuff because there wont by any Message objects. With objects functions can see that they are getting a message instead of just any random string.
What are some good uses of this that won't magically work for some methods but fail in 95% of the others, so that you have to memorize which exact methods support Message parameters and which don't? And that also won't require adding boilerplate Message-handling code to hundreds of methods?
There is also many places in the code where messages are passed to functions. We could pass Message objects there instead of array( 'key', 'param', 'param' ), which is quite inflexible.
Where are some examples of this, and some sample benefits?
On Thu, Apr 1, 2010 at 3:59 PM, Happy-melon happy-melon@live.com wrote:
I think the existence of a Message object that can be instantiated and passed around, is a very important benefit. As you say, if we go down the other route, we just end up with the wfMsg functions in a different namespace.
We'd end up with *cleaned-up* wfMsg() functions in a different namespace. That's the only benefit I see as compelling, that they be clean and easy to use. I have no problem with the general approach taken by wfMsg*(), they've just become rather crufty and confusing over the years. Compare Linker::link() to all the methods it replaced. It's not fundamentally different, it's just cleaner and easier to understand.
I don't see a lot we can do with objects without adding excessive clutter, given PHP's deficiencies. In almost all cases you really do just want plain old strings, and requiring extra method calls everywhere for the sake of uncommon cases is uneconomical.
Of course, once we require a version of PHP that supports __toString(), we could transparently alter all existing message-related functions (including wfMsg() and friends) to return some Message object with a __toString() method. That would give us the best of all worlds. Until then, if you really wanted objects you could create them for whatever uses you have for them, but the vast majority of callers don't need any kind of object and would really just like a string.
"Roan Kattouw" roan.kattouw@gmail.com wrote in message news:s2kf154f3a81004011326n7938f5b3i8d484f3d50f28fae@mail.gmail.com...
This'd be very nice, and would kind of supersede the Status class currently used to shove a message key and some params in so the callee can either get it automatically processed by wfMsg() (UI functions) or grab the raw message key + params and process that in their own way (API). This would require the Message class have getters for both of these though (does it currently?).
Roan Kattouw (Catrope)
As Roan says, a Message object essentially deprecates the (IMO pig-ugly) Status class, which is used erratically throughout the codebase as essentially a way to bundle up a message key, some parameters, and a success flag without converting to String too soon. Status has always struck me as an unpleasant implementation: processes should throw exceptions on fatal errors, return messages on non-catastrophic problems, and bool true on success.
Even more erratic is the way we have functions returning message keys, or maybe array('key','param','param'), or maybe array('key',array('param','param')), or maybe something even more exotic. Cf r63678; somewhere in that stack a String is appearing when it's "supposed" to be an array, but when nested arrays anywhere between 0 and 2 layers deep are valid input, it's not altogether surprising. Having an array of Message objects on which you can call array_udiff() and array_uintersect() with a static method Message::equals() (even PHP's native implementation of Object == Object is ok), will make that process reasonably sane. And "that process" is what you get every time User::getPermissionsErrors() is invoked.
--HM
On Sun, Apr 4, 2010 at 1:43 PM, Happy-melon happy-melon@live.com wrote:
As Roan says, a Message object essentially deprecates the (IMO pig-ugly) Status class, which is used erratically throughout the codebase as essentially a way to bundle up a message key, some parameters, and a success flag without converting to String too soon. Status has always struck me as an unpleasant implementation: processes should throw exceptions on fatal errors, return messages on non-catastrophic problems, and bool true on success.
As opposed to returning a single class which can handle errors, warnings and OK statuses just fine? I really don't know why people don't like the Status class, I'm trying to use it *more* in the new-installer, rather than returning weird mixtures of booleans, strings and exceptions.
-Chad
"Chad" innocentkiller@gmail.com wrote in message news:j2h5924f50a1004041133v522f88e9vf1349e6447667253@mail.gmail.com...
On Sun, Apr 4, 2010 at 1:43 PM, Happy-melon happy-melon@live.com wrote:
As Roan says, a Message object essentially deprecates the (IMO pig-ugly) Status class, which is used erratically throughout the codebase as essentially a way to bundle up a message key, some parameters, and a success flag without converting to String too soon. Status has always struck me as an unpleasant implementation: processes should throw exceptions on fatal errors, return messages on non-catastrophic problems, and bool true on success.
As opposed to returning a single class which can handle errors, warnings and OK statuses just fine? I really don't know why people don't like the Status class, I'm trying to use it *more* in the new-installer, rather than returning weird mixtures of booleans, strings and exceptions.
-Chad
It's pretty rare for 'error', 'warning', and 'ok' to all have independent meanings in MW. I can see the installer being an exception; but in general, we don't implement an "I did your action but this problem meant it's not entirely as you would expect" result; we fail and give the user an option to try again; that's not really any different to an error.
The real distinction is between errors that *preclude* a repeat attempt, and those that just indicate a failure this time around or with that particular set of data. Errors like read-only-wiki or not-authorised should be handled with exceptions which are caught high up in the call stack, probably at the level of Wiki.php; the message you get if you try to edit a page on a read-only wiki needn't be much different to trying to change userrights on a read-only wiki. Ditto for permissions errors, and anything else that leads to a 'dead end' error screen. All we're achieving with our if(wfReadOnly()){global $wgOut; $wgOut->readOnlyPage(); return false; } is to terminate call stacks early; that's exactly what exceptions should be used for. We just need a wfAssertWritable() function which throws a MWReadOnlyException on a locked wiki.
Any error which should be handled by redisplaying the form unquestionably needs to be accompanied by an informative error message, and conversely the "Your foo has been barred" success message is either going to be state-independent ("we successfully made the requested changes to your Foo") or will have done object instantiation ("based on your input, we have created a shiny new Foo"), which you'd keep in a member variable anyway. As such, every success needs no data returned from it, every transient error needs a message associated with it, and every permanent error should be handled by exceptions. Why complicate things with a wrapper class to convey information you can work out from context anyway?
--HM
2010/4/1 Niklas Laxström niklas.laxstrom@gmail.com:
With objects functions can see that they are getting a message instead of just any random string. There is also many places in the code where messages are passed to functions. We could pass Message objects there instead of array( 'key', 'param', 'param' ), which is quite inflexible.
What do others think of the above?
This'd be very nice, and would kind of supersede the Status class currently used to shove a message key and some params in so the callee can either get it automatically processed by wfMsg() (UI functions) or grab the raw message key + params and process that in their own way (API). This would require the Message class have getters for both of these though (does it currently?).
Roan Kattouw (Catrope)
wikitech-l@lists.wikimedia.org