r66057 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r66056‎ | r66057 | r66058 >
Date:23:57, 7 May 2010
Author:svip
Status:resolved (Comments)
Tags:
Comment:
Far more render logic added. Still not added so much to parse input data.
Modified paths:
  • /trunk/extensions/TemplateAdventures/TemplateAdventures.i18n.php (modified) (history)
  • /trunk/extensions/TemplateAdventures/Templates/Citation.php (modified) (history)

Diff [purge]

Index: trunk/extensions/TemplateAdventures/TemplateAdventures.i18n.php
——@@ -12,5 +12,18 @@
1313 'ta-pubmed-url' => 'http://www.pubmedcentral.nih.gov/articlerender.fcgi?tool=pmcentrez&artid=$1',
1414 'ta-transtitle-render' => '&#91;$1&#93;',
1515 'ta-writtenat' => 'written at $1',
 16+ 'ta-inlanguage' => ' (in $1)',
 17+ 'ta-formatrender' => ' ($1)',
 18+ 'ta-publicationplaceandpublisher' => ' ($1: $2)',
 19+ 'ta-publicationplace' => ' ($1)',
 20+ 'ta-volumerender' => " '''$1'''",
 21+ 'ta-issuerender' => ' ($1)',
 22+ 'ta-atrender' => ': $1',
 23+ 'ta-titletyperender' => ' ($1)',
 24+ 'ta-publisherrender' => ' $1',
 25+ 'ta-published' => ' (published $1)',
 26+
 27+ 'ta-printonlyspan' => '<span class="printonly">$1</span>',
 28+ 'ta-accessdatespan' => '<span class="reference-accessdate">$1</span>',
1629 );
1730
Index: trunk/extensions/TemplateAdventures/Templates/Citation.php
——@@ -31,13 +31,14 @@
3232 'transitalic' => null, # translated title in italic
3333 'includedwork' => null,
3434 'type' => null, # the title type
 35+ 'note' => null,
3536 );
3637 private $dWorkLink = array( # data related to the link
3738 'url' => null,
3839 'originalurl' => null,
3940 'includedwork' => null,
40 - 'at' => null, # wherein the source
4141 );
 42+ private $dAt = null; # wherein the source
4243 private $dArchived = array( # information about its archiving if archived
4344 'url' => null,
4445 'date' => null,
——@@ -117,7 +118,7 @@
118119 $this->mOutput .= $editorArea;
119120 }
120121 # included work title
121 - if ( $this->notNull( $this->dWorkTitle['includedwork'] ) && ( $this->notNull( $this->dPeriodical ) || $this->notNull( $this->dWorkTitle['transitalic'] ) || $this->notNull( $this->dWorkTitle['transtitle'] ) ) ) {
 122+ if ( $this->notNull( $this->dWorkTitle['includedwork'] ) && ( $this->notNull( $this->dPeriodical['name'] ) || $this->notNull( $this->dWorkTitle['transitalic'] ) || $this->notNull( $this->dWorkTitle['transtitle'] ) ) ) {
122123 # I am no way certain this is a correct copy of the following logic:
123124 # {{#if: {{{IncludedWorkTitle|}}}{{#if:{{{Periodical|}}}||{{#if:{{{TransItalic|}}}||{{{TransTitle|}}}}}}} | ... | ... }}
124125 if ( $authorArea != '' || $editorArea != '' ) {
——@@ -139,7 +140,7 @@
140141 }
141142 }
142143 # and now the title
143 - if ( $this->dPeriodical != null ) {
 144+ if ( $this->notNull ( $this->dPeriodical['name'] ) ) {
144145 $tmp = str_replace( "'", '&#39;', $this->dWorkTitle['includedwork']);
145146 $title = "''$tmp''";
146147 } else {
——@@ -170,10 +171,316 @@
171172 # TODO: we'll do this later...
172173
173174 # periodicals
174 - # TODO: I'll get on it!
 175+ if ( $this->notNull ( $this->dPeriodical['name'] ) ) {
 176+ $perArea = '';
 177+ if ( $this->notNull ( $this->dOther ) )
 178+ $perArea .= $this->getSeparator ( 'section' ) . ' ' . $this->dOther;
 179+ if ( $authorArea != '' || $editorArea != '' || $this->notNull ( $this->dWorkTitle['includedwork'] ) )
 180+ $perArea .= $this->getSeparator ( 'section' );
 181+ # make the link!
 182+ if ( $this->notNull ( $this->dWorkTitle['title'] ) || $this->notNull ( $this->dWorkTitle['transtitle'] ) ) {
 183+ # let's get the url
 184+ if ( $this->notNull ( $this->dWorkTitle['includedwork'] ) ) {
 185+ if ( $this->notNull ( $this->dWorkLink['includedwork'] ) ) {
 186+ if ( $this->notNull ( $this->dWorkLink['url'] ) ) {
 187+ $url = $this->dWorkLink['url'];
 188+ } else {
 189+ # some explain to me what exactly the following is supposed to mean:
 190+ # <!-- Only link URL if to a free full text - as at PubMedCentral (PMC)-->
 191+ # |{{#ifexpr:{{#time: U}} > {{#time: U | {{{Embargo|2001-10-10}}} }}
 192+ if ( $this->dPubMed['pmc'] != null ) {
 193+ $url = wfMsg ( 'ta-pubmed-url', $this->dPubMed['pmc'] );
 194+ }
 195+ }
 196+ }
 197+ } else {
 198+ if ( $this->notNull ( $this->dWorkLink['url'] ) ) {
 199+ $url = $this->dWorkLink['url'];
 200+ } else {
 201+ # some explain to me what exactly the following is supposed to mean:
 202+ # <!-- Only link URL if to a free full text - as at PubMedCentral (PMC)-->
 203+ # |{{#ifexpr:{{#time: U}} > {{#time: U | {{{Embargo|2001-10-10}}} }}
 204+ if ( $this->dPubMed['pmc'] != null ) {
 205+ $url = wfMsg ( 'ta-pubmed-url', $this->dPubMed['pmc'] );
 206+ }
 207+ }
 208+ }
 209+ # and now the title
 210+ $tmp = $this->notNull ( $this->dWorkTitle['title'] )
 211+ ? $this->dWorkTitle['title']
 212+ : '';
 213+ if ( $this->notNull ( $this->dWorkTitle['transtitle'] ) ) {
 214+ if ( $tmp != '' )
 215+ $tmp .= ' ';
 216+ $tmp .= wfMsg ( 'ta-transtitle-render', $this->dWorkTitle['transtitle'] );
 217+ }
 218+ $title = "\"$tmp\"";
 219+ $perArea .= $this->makeLink ( $url, $title );
 220+ if ( $this->notNull ( $this->dWorkTitle['note'] ) ) {
 221+ $perArea .= $this->getSeparator ( 'section' ) . ' ' . $this->dWorkTitle['note'];
 222+ }
 223+ }
 224+ $this->mOutput .= $perArea;
 225+ }
 226+ # language
 227+ if ( $this->notNull ( $this->dLanguage ) ) {
 228+ $this->mOutput .= wfMsg ( 'ta-inlanguage', $this->dLanguage );
 229+ }
 230+ # format
 231+ if ( $this->notNull ( $this->dFormat ) ) {
 232+ $this->mOutput .= wfMsg ( 'ta-formatrender', $this->dFormat );
 233+ }
 234+ # more periodical!
 235+ if ( $this->notNull ( $this->dPeriodical['name'] ) ) {
 236+ $newPerArea = '';
 237+ if ( $this->notNull ( $this->dWorkTitle['includedwork'] )
 238+ || $this->notNull ( $this->dWorkTitle['title'] )
 239+ || $this->notNull ( $this->dWorkTitle['transtitle'] )
 240+ ) {
 241+ $newPerArea .= $this->getSeparator ( 'section' ) . ' ';
 242+ }
 243+ $newPerArea .= "''" . $this->clean ( $this->dPeriodical['name'] ) . "''";
 244+ if ( $this->notNull ( $this->dSeries ) ) {
 245+ $newPerArea .= $this->getSeparator ( 'section' ) . ' ' . $this->dSeries;
 246+ }
 247+ if ( $this->notNull ( $this->dPublication['place'] ) ) {
 248+ if ( $this->notNull ( $this->dPublisher ) ) {
 249+ $newPerArea .= wfMsg ( 'ta-publicationplaceandpublisher', $this->dPublication['place'], $this->dPublisher );
 250+ } else {
 251+ $newPerArea .= wfMsg ( 'ta-publicationplace', $this->dPublication['place'] );
 252+ }
 253+ }
 254+ if ( $this->notNull ( $this->dVolume ) ) {
 255+ $newPerArea .= wfMsg ( 'ta-volumerender', $this->dVolume );
 256+ if ( $this->notNUll ( $this->dIssue ) ) {
 257+ $newPerArea .= wfMsg ( 'ta-issuerender', $this->dIssue );
 258+ }
 259+ } else {
 260+ if ( $this->notNUll ( $this->dIssue ) ) {
 261+ $newPerArea .= wfMsg ( 'ta-issuerender', $this->dIssue );
 262+ }
 263+ }
 264+ if ( $this->notNull ( $this->dAt ) ) {
 265+ $newPerArea .= wfMsg ( 'ta-atrender', $this->dAt );
 266+ }
 267+ # now we get to the title! Exciting stuff!
 268+ if ( $this->notNull ( $this->dWorkTitle['title'] )
 269+ || $this->notNull ( $this->dWorkTitle['transitalic'] ) ) {
 270+ if ( $authorArea != ''
 271+ || $editorArea != ''
 272+ || $this->notNull ( $this->dWorkTitle['includedwork'] )
 273+ || $this->notNull ( $this->dPeriodical['name'] )
 274+ ) {
 275+ $newPerArea .= $this->getSeparator ( 'section' );
 276+ }
 277+ # let's get the url
 278+ if ( $this->notNull ( $this->dWorkTitle['includedwork'] ) ) {
 279+ if ( $this->notNull ( $this->dWorkLink['includedwork'] ) ) {
 280+ if ( $this->notNull ( $this->dWorkLink['url'] ) ) {
 281+ $url = $this->dWorkLink['url'];
 282+ } else {
 283+ # some explain to me what exactly the following is supposed to mean:
 284+ # <!-- Only link URL if to a free full text - as at PubMedCentral (PMC)-->
 285+ # |{{#ifexpr:{{#time: U}} > {{#time: U | {{{Embargo|2001-10-10}}} }}
 286+ if ( $this->dPubMed['pmc'] != null ) {
 287+ $url = wfMsg ( 'ta-pubmed-url', $this->dPubMed['pmc'] );
 288+ }
 289+ }
 290+ }
 291+ } else {
 292+ if ( $this->notNull ( $this->dWorkLink['url'] ) ) {
 293+ $url = $this->dWorkLink['url'];
 294+ } else {
 295+ # some explain to me what exactly the following is supposed to mean:
 296+ # <!-- Only link URL if to a free full text - as at PubMedCentral (PMC)-->
 297+ # |{{#ifexpr:{{#time: U}} > {{#time: U | {{{Embargo|2001-10-10}}} }}
 298+ if ( $this->dPubMed['pmc'] != null ) {
 299+ $url = wfMsg ( 'ta-pubmed-url', $this->dPubMed['pmc'] );
 300+ }
 301+ }
 302+ }
 303+ # and now the title
 304+ $tmp = $this->notNull ( $this->dWorkTitle['title'] )
 305+ ? $this->dWorkTitle['title']
 306+ : '';
 307+ if ( $this->notNull ( $this->dWorkTitle['transitalic'] ) ) {
 308+ if ( $tmp != '' )
 309+ $tmp .= ' ';
 310+ $tmp .= wfMsg ( 'ta-transtitle-render', $this->dWorkTitle['transitalic'] );
 311+ }
 312+ $tmp = $this->clean ( $tmp );
 313+ $title = "''$tmp''";
 314+ $newPerArea .= $this->makeLink ( $url, $title );
 315+ }
 316+ # may change this into some if () statements though,
 317+ # it is easier to write this, but it also means that all of the
 318+ # second input is actually evaluated, even if it contains nothing.
 319+ $newPerArea .= $this->addNotNull ( $this->dWorkTitle['type'],
 320+ wfMsg ( 'ta-titletyperender', $this->dWorkTitle['type'] ) );
 321+ $newPerArea .= $this->addNotNull ( $this->dSeries,
 322+ $this->getSeparator ( 'section' ) . ' ' . $this->dSeries );
 323+ $newPerArea .= $this->addNotNull ( $this->dVolume,
 324+ $this->getSeparator ( 'section' ) . ' ' . wfMsg ( 'ta-volumerender', $this->dVolume ) );
 325+ $newPerArea .= $this->addNotNull ( $this->dOther,
 326+ $this->getSeparator ( 'section' ) . ' ' . $this->dOther );
 327+ $newPerArea .= $this->addNotNull ( $this->dEdition,
 328+ wfMsg ( 'ta-editionrender', $this->dEdition ) );
 329+ $newPerArea .= $this->addNotNull ( $this->dPublication['place'],
 330+ $this->getSeparator ( 'section' ) . ' ' . $this->dPublication['place'] );
 331+ if ( $this->notNull ( $this->dPublisher ) ) {
 332+ if ( $this->dPublication['place'] ) {
 333+ $newPerArea .= ':';
 334+ } else {
 335+ $newPerArea .= $this->getSeparator ( 'section' );
 336+ }
 337+ $newPerArea .= wfMsg ( 'ta-publisherrender', $this->dPublisher );
 338+ }
 339+ $this->mOutput .= $newPerArea;
 340+ }
 341+ # date if no author/editor
 342+ if ( $authorArea == '' && $editorArea == '' ) {
 343+ if ( $this->notNull ( $this->dDate ) ) {
 344+ $this->mOutput .= $this->getSeparator ( 'section' ) . wfMsg ( 'ta-citeauthordate', $this->dDate );
 345+ if ( $this->notNull ( $this->dYearNote ) ) {
 346+ $this->mOutput .= wfMsg ( 'ta-citeauthoryearnote', $this->dYearNote );
 347+ }
 348+ }
 349+ }
 350+ # publication date
 351+ if ( $this->notNull ( $this->dPublication['date'] ) && $this->dPublication['date'] != $this->dDate ) {
 352+ if ( isset ( $this->dEditors[1] ) ) {
 353+ if ( isset ( $this->dAuthors[1] ) ) {
 354+ $this->mOutput .= $this->getSeparator ( 'section' ) . ' ' . $this->dPublication['date'];
 355+ } else {
 356+ $this->mOutput .= wfMsg ( 'ta-published', $this->dPublication['date'] );
 357+ }
 358+ } else {
 359+ if ( $this->notNull ( $this->dPeriodical['name'] ) ) {
 360+ $this->mOutput .= $this->getSeparator ( 'section' ) . ' ' . $this->dPublication['date'];
 361+ } else {
 362+ $this->mOutput .= wfMsg ( 'ta-published', $this->dPublication['date'] );
 363+ }
 364+ }
 365+ }
 366+ # page within included work
 367+ if ( !$this->notNull ( $this->dPeriodical['name'] ) && $this->notNull ( $this->dAt ) ) {
 368+ $this->mOutput .= $this->getSeparator ( 'section' ) . ' ' . $this->dAt;
 369+ }
 370+ # doi
 371+ # TODO: I'll do this code later:
 372+ # {{{Sep|,}}}&#32;{{citation/identifier |identifier=doi |input1={{{DOI|}}} |input2={{{DoiBroken|}}} }}
 373+
 374+ # misc identifier
 375+ # TODO: Awh shit.
 376+ # #if: {{{ID|}}}
 377+ # |{{
 378+ # #if: {{{Surname1|}}}{{{EditorSurname1|}}}{{{IncludedWorkTitle|}}}{{{Periodical|}}}{{{Title|}}}{{{TransItalic|}}}
 379+ # |{{{Sep|,}}}&#32;{{{ID}}}
 380+ # |{{{ID}}}
 381+ # }}
 382+
 383+ # more identifiers:
 384+ # TODO: Do all this crap.
 385+ /*
 386+ {{
 387+<!--============ ISBN ============-->
 388+ #if: {{{ISBN|}}}
 389+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=isbn |input1={{{ISBN|}}} }}
 390+}}{{
 391+<!--============ ISSN ============-->
 392+ #if: {{{ISSN|}}}
 393+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=issn |input1={{{ISSN|}}} }}
 394+}}{{
 395+<!--============ OCLC ============-->
 396+ #if: {{{OCLC|}}}
 397+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=oclc |input1={{{OCLC|}}} }}
 398+}}{{
 399+<!--============ PMID ============-->
 400+ #if: {{{PMID|}}}
 401+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=pmid |input1={{{PMID|}}} }}
 402+}}{{
 403+<!--============ PMC ============-->
 404+ #if: {{{PMC|}}}
 405+ |{{
 406+ #if: {{{URL|}}}
 407+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=pmc |input1={{{PMC|}}} }}
 408+ |{{only in print|{{{Sep|,}}}&#32;{{citation/identifier |identifier=pmc |input1={{{PMC|}}} }} }}<!--Should only display by default in print-->
 409+ }}
 410+}}{{
 411+<!--============ BIBCODE ============-->
 412+ #if: {{{Bibcode|}}}
 413+ |{{{Sep|,}}}&#32;{{citation/identifier |identifier=bibcode |input1={{{Bibcode|}}} }}
 414+}}
 415+ */
 416+
 417+ # archive data, etc.
 418+ # TODO: Yeah, O_O
 419+
 420+ # URL and accessdate
 421+ if ( $this->notNull ( $this->dWorkLink['url'] )
 422+ || $this->notNull ( $this->dWorkLink['includedwork'] ) ) {
 423+ if ( $this->notNull ( $this->dWorkTitle['title'] )
 424+ || $this->notNull ( $this->dWorkTitle['includedwork'] )
 425+ || $this->notNull ( $this->dWorkTitle['transtitle'] ) ) {
 426+ $this->mOutput .= $this->printOnly (
 427+ $this->getSeparator ( 'section' ) . ' ' .
 428+ ( $this->notNull ( $this->dWorkLink['includedwork'] )
 429+ ? $this->dWorkLink['includedwork']
 430+ : $this->dWorkLink['url'] ) );
 431+ } else {
 432+ $this->mOutput .= $this->getSeparator ( 'section' ) . ' ' .
 433+ ( $this->notNull ( $this->dWorkLink['includedwork'] )
 434+ ? $this->dWorkLink['includedwork']
 435+ : $this->dWorkLink['url'] );
 436+ }
 437+ if ( $this->notNull ( $this->dAccessDate ) ) {
 438+ if ( $this->getSeparator ( 'section' ) == '.' )
 439+ $tmp = ". Retrieved {$this->dAccessDate}";
 440+ else
 441+ $tmp = ", retrieved {$this->dAccessDate}";
 442+ $this->mOutput .= wfMsg ( 'ta-accessdatespan', $tmp );
 443+ }
 444+ }
 445+
 446+ # layman stuff
 447+ # TODO
 448+
 449+ # quote
 450+ # TODO
 451+
 452+ # some other shit nobody cares about.
 453+ # COinS? waaaaat
 454+ # TODO
175455 }
176456
177457 /**
 458+ * Surround the string in a span tag that tells the css not to render it
 459+ * unless it for print.
 460+ *
 461+ * @param $string
 462+ * @return $string
 463+ */
 464+ private function printOnly ( $string ) {
 465+ return wfMsg ( 'ta-printonlyspan', $string );
 466+ }
 467+
 468+ private function addNotNull ( $check, $add ) {
 469+ if ( $this->notNull ( $check ) )
 470+ return $add;
 471+ return '';
 472+ }
 473+
 474+ /**
 475+ * Clean a string for characters that might confuse the parser.
 476+ *
 477+ * @param $value The string to be cleaned.
 478+ * @return The cleaned string.
 479+ */
 480+ private function clean ( $value ) {
 481+ return str_replace ( "'", '&#39;', $value );
 482+ }
 483+
 484+ /**
178485 * Create the section for authors, editors, etc. in a neat similar function.
179486 *
180487 * @param $writers Authors, editors, etc. array
——@@ -334,7 +641,10 @@
335642 break;
336643 case 'includedworktitle':
337644 $this->dWorkTitle['includedwork'] = $value;
338 - break;
 645+ break;
 646+ case 'periodical':
 647+ $this->dPeriodical['name'] = $value;
 648+ break;
339649 }
340650 }
341651

Follow-up revisions

RevisionCommit summaryAuthorDate
r66106Follow up/fix to r66057, hopefully this suffice greatly regarding the spaces ...svip13:52, 9 May 2010
r66491Following up on r66057, r66106 & r66107. I really, really need to wrap my he...svip19:13, 15 May 2010

Comments

#Comment by Siebrand (talk | contribs)   08:32, 8 May 2010

Please do no hardcode leading (or trailing) spaces in your messages, or glue multiple messages together into something that makes sense in English. This is a bad internationalisation (i18n) practice, that makes localisation (L10n) in many languages impossible. Please do use duplication of similar messages, instead of making it a Lego project.

So do not "Text" + " ($1)" and "Text" + " ($1 $2)", but make 2 messages as "Message ($1)" and "Message ($1 $2)", or whatever is needed to put parts that have to be used together together. Thanks.

If you haven't done so, please read Localisation.

#Comment by Svippong (talk | contribs)   10:21, 8 May 2010

Don't you mean "$1 ($2)" rather than " ($1)"?

#Comment by Siebrand (talk | contribs)   10:30, 8 May 2010

Not really. I was providing an example not specific for the above.

#Comment by Svippong (talk | contribs)   10:35, 8 May 2010

Well, in all honesty, I did originally not anticipate it to be like this (purely a temporary fashion until I found a better solution). The point is, sometimes 'Message' is a separator (e.g. ',') or sometimes it is nothing, and the space is just there to distance itself from the previous element.

We sort of got away with it in Extension:NaturalLanguageList, where we had a 'lastseparator' string consisting of ' and ' in English. But to make up for that, we changed it to ' and ', to make it completely obvious that we were talking about spaces.

While I won't contest that it should be changed, I am still curious as to what language would prefer a different method than ' ($1)'.

#Comment by Siebrand (talk | contribs)   13:10, 8 May 2010

Aside from ' ($1)' not being clear, and running the risk of using it in different contexts, making it work only in a few Western European languages, if at all, the languages that are know to have a lot of word order/i18n issues with split up text parts are Slavic languages, Chinese, Japanese, languages that do not use spaces (Chinese), languages that use spaces between a work and a colon (French)...

#Comment by Svippong (talk | contribs)   13:14, 8 May 2010

Well, I suppose. But since the separator is going to be a lone message (I admit, I have not added this yet; I intend to!), I am actually honestly looking for suggestions!

Any ideas?

#Comment by Svippong (talk | contribs)   13:53, 9 May 2010

I hope my fix in r66106 suffices. :) Otherwise, please comment further!

Status & tagging log