r103035 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r103034‎ \| r103035 \| r103036 >
Date:	21:22, 14 November 2011
Author:	yuvipanda
Status:	ok (Comments)
Tags:
Comment:	Fixed (most) issues with Extension:ShortURL pointed out in CR Comprehensive CR URL: http://www.mediawiki.org/wiki/User:Catrope/Extension_review/ShortUrl
Modified paths:	/trunk/extensions/ShortUrl/ShortUrl.hooks.php (modified) (history) /trunk/extensions/ShortUrl/ShortUrl.i18n.php (modified) (history) /trunk/extensions/ShortUrl/ShortUrl.php (modified) (history) /trunk/extensions/ShortUrl/ShortUrl.utils.php (modified) (history) /trunk/extensions/ShortUrl/SpecialShortUrl.php (modified) (history) /trunk/extensions/ShortUrl/css/ext.shortUrl.css (modified) (history) /trunk/extensions/ShortUrl/js/ext.shortUrl.js (modified) (history) /trunk/extensions/ShortUrl/schemas/shorturls.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/ShortUrl/ShortUrl.hooks.php
—	—	@@ -21,8 +21,8 @@
22	22	public static function addToolboxLink( &$tpl ) {
23	23	global $wgOut, $wgShortUrlPrefix;
24	24
25		~~- if ( $wgShortUrlPrefix == null ) {~~
26		~~- $urlPrefix = SpecialPage::getTitleFor( 'ShortUrl' )->getFullURL() . '/';~~
	25	+ if ( $wgShortUrlPrefix === null ) {
	26	+ $urlPrefix = SpecialPage::getTitleFor( 'ShortUrl' )->getCanonicalUrl() . '/';
27	27	} else {
28	28	$urlPrefix = $wgShortUrlPrefix;
29	29	}
Index: trunk/extensions/ShortUrl/js/ext.shortUrl.js
—	—	@@ -1,6 +1,6 @@
2	2	jQuery( function( $ ) {
3	3	if( $( '#t-shorturl' ).length ) {
4	4	var url = $( '#t-shorturl a' ).attr( 'href' );
5		~~- $( '#firstHeading' ).append( '<a class="title-shortlink" href="' + url + '">' + url + '</a>' );~~
	5	+ $( '#firstHeading' ).append( $( '<div class="title-shortlink-container"></div>').append( $( '<a>' ).addClass( 'title-shortlink' ).attr( 'href', url ).text( url ) ) );
6	6	}
7	7	});
Index: trunk/extensions/ShortUrl/schemas/shorturls.sql
—	—	@@ -2,9 +2,9 @@
3	3	-- Replace /$wgDBTableOptions/ with the correct options
4	4
5	5	CREATE TABLE IF NOT EXISTS /_/shorturls (
6		~~- su_id integer NOT NULL AUTO_INCREMENT,~~
	6	+ su_id integer NOT NULL PRIMARY KEY AUTO_INCREMENT,
7	7	su_namespace integer NOT NULL,
8		~~- su_title varchar(255) NOT NULL,~~
9		~~- PRIMARY KEY (su_id),~~
10		~~- UNIQUE KEY (su_namespace, su_title)~~
	8	+ su_title varchar(255) NOT NULL
11	9	) /$wgDBTableOptions/;
	10	+
	11	+CREATE UNIQUE INDEX /i/shorturls_ns_title ON /_/shorturls (su_namespace, su_title);
Index: trunk/extensions/ShortUrl/ShortUrl.i18n.php
—	—	@@ -17,7 +17,8 @@
18	18	$messages['en'] = array(
19	19	'shorturl' => 'Short URL',
20	20	'shorturl-desc' => '[[Special:ShortUrl\|Short URL for redirects]]',
21		~~- 'shorturl-not-found' => 'Sorry, the URL you are looking for is not found (No Short URL with ID $1 exists)',~~
	21	+ 'shorturl-not-found-title' => 'Short URL not found',
	22	+ 'shorturl-not-found-message' => 'Sorry, the URL you are looking for is not found (No short URL with ID $1 exists)',
22	23	'shorturl-toolbox-title' => 'Copy this short link for sharing',
23	24	'shorturl-toolbox-text' => 'Short URL'
24	25	);
—	—	@@ -28,7 +29,7 @@
29	30	$messages['ast'] = array(
30	31	'shorturl' => 'URL curtia',
31	32	'shorturl-desc' => '[[Special:ShortUrl\|URL curtia pa redireiciones]]',
32		~~- 'shorturl-not-found' => "Lo sentimos, nun s'alcontró la URL que tas buscando (Nun esiste denguna URL curtia con ID $1)",~~
	33	+ 'shorturl-not-found-message' => "Lo sentimos, nun s'alcontró la URL que tas buscando (Nun esiste denguna URL curtia con ID $1)",
33	34	'shorturl-toolbox-title' => 'Copia esti enllaz curtiu pa compartir',
34	35	'shorturl-toolbox-text' => 'URL curtia',
35	36	);
—	—	@@ -40,7 +41,7 @@
41	42	$messages['be-tarask'] = array(
42	43	'shorturl' => 'Кароткія спасылкі',
43	44	'shorturl-desc' => '[[Special:ShortUrl\|Кароткія спасылкі для перанакіраваньняў]]',
44		~~- 'shorturl-not-found' => 'На жаль, пазначаны URL-адрас ня знойдзены (кароткая спасылка з ідэнтыфікатарам $1 не існуе)',~~
	45	+ 'shorturl-not-found-message' => 'На жаль, пазначаны URL-адрас ня знойдзены (кароткая спасылка з ідэнтыфікатарам $1 не існуе)',
45	46	'shorturl-toolbox-title' => 'Скапіюйце гэтую кароткую спасылку',
46	47	'shorturl-toolbox-text' => 'Кароткая спасылка',
47	48	);
—	—	@@ -51,7 +52,7 @@
52	53	$messages['br'] = array(
53	54	'shorturl' => 'URL berr',
54	55	'shorturl-desc' => '[[Special:ShortUrl\|URL berr evit adkasoù]]',
55		~~- 'shorturl-not-found' => "Digarezit, n'eo ket bet kavet an URL emaoc'h o klask (N'eus URL berr ebet gant an ID $1)",~~
	56	+ 'shorturl-not-found-message' => "Digarezit, n'eo ket bet kavet an URL emaoc'h o klask (N'eus URL berr ebet gant an ID $1)",
56	57	'shorturl-toolbox-title' => 'Kopiit al liamm berr-mañ evit e rannañ',
57	58	'shorturl-toolbox-text' => 'URL berr',
58	59	);
—	—	@@ -62,7 +63,7 @@
63	64	$messages['de'] = array(
64	65	'shorturl' => 'Kurz-URL',
65	66	'shorturl-desc' => 'Ergänzt eine [[Special:ShortUrl\|Spezialseite]] zum Erstellen von Kurz-URLs für Weiterleitungen',
66		~~- 'shorturl-not-found' => 'Die gesuchte Kurz-URL wurde leider nicht gefunden (Es ist keine Kurz-URL mit der Kennung $1 vorhanden)',~~
	67	+ 'shorturl-not-found-message' => 'Die gesuchte Kurz-URL wurde leider nicht gefunden (Es ist keine Kurz-URL mit der Kennung $1 vorhanden)',
67	68	'shorturl-toolbox-title' => 'Kurz-URL kopieren, um sie zu nutzen',
68	69	'shorturl-toolbox-text' => 'Kurz-URL',
69	70	);
—	—	@@ -74,7 +75,7 @@
75	76	$messages['fr'] = array(
76	77	'shorturl' => 'URL courte',
77	78	'shorturl-desc' => '[[Special:ShortUrl\|URL courte pour les redirections]]',
78		~~- 'shorturl-not-found' => 'Désolé, l’URL que vous recherchez est introuvable (aucune URL courte avec l’ID $1)',~~
	79	+ 'shorturl-not-found-message' => 'Désolé, l’URL que vous recherchez est introuvable (aucune URL courte avec l’ID $1)',
79	80	'shorturl-toolbox-title' => 'Copiez ce court lien pour le partager',
80	81	'shorturl-toolbox-text' => 'URL courte',
81	82	);
—	—	@@ -94,7 +95,7 @@
95	96	$messages['gl'] = array(
96	97	'shorturl' => 'Enderezo URL curto',
97	98	'shorturl-desc' => '[[Special:ShortUrl\|Enderezo URL curto para as redireccións]]',
98		~~- 'shorturl-not-found' => 'Sentímolo, non se atopou o enderezo URL que está a buscar (non existe ningún enderezo URL curto co ID $1)',~~
	99	+ 'shorturl-not-found-message' => 'Sentímolo, non se atopou o enderezo URL que está a buscar (non existe ningún enderezo URL curto co ID $1)',
99	100	'shorturl-toolbox-title' => 'Copie esta ligazón curta para compartir',
100	101	'shorturl-toolbox-text' => 'Enderezo URL curto',
101	102	);
—	—	@@ -105,7 +106,7 @@
106	107	$messages['he'] = array(
107	108	'shorturl' => 'כתובת קצרה',
108	109	'shorturl-desc' => '[[Special:ShortUrl\|כתובת קצרה להפניות]]',
109		~~- 'shorturl-not-found' => 'סליחה, הכתובת שאתם מחפשים אינה נמצאת (אין כתובת קצרה עם המזהה $1)',~~
	110	+ 'shorturl-not-found-message' => 'סליחה, הכתובת שאתם מחפשים אינה נמצאת (אין כתובת קצרה עם המזהה $1)',
110	111	'shorturl-toolbox-title' => 'העתיקו את הקישור הקצר הזה בשביל שיתוף',
111	112	'shorturl-toolbox-text' => 'כתובת קצרה',
112	113	);
—	—	@@ -123,7 +124,7 @@
124	125	$messages['ia'] = array(
125	126	'shorturl' => 'URL curte',
126	127	'shorturl-desc' => '[[Special:ShortUrl\|URL curte pro redirectiones]]',
127		~~- 'shorturl-not-found' => 'Le URL que tu cerca non ha essite trovate (nulle URL curte con le ID $1 existe).',~~
	128	+ 'shorturl-not-found-message' => 'Le URL que tu cerca non ha essite trovate (nulle URL curte con le ID $1 existe).',
128	129	'shorturl-toolbox-title' => 'Copiar iste ligamine curte pro divulgation',
129	130	'shorturl-toolbox-text' => 'URL curte',
130	131	);
—	—	@@ -134,7 +135,7 @@
135	136	$messages['lb'] = array(
136	137	'shorturl' => 'Kuerz URL',
137	138	'shorturl-desc' => '[[Special:ShortUrl\|Kuerz URL fir Viruleedungen]]',
138		~~- 'shorturl-not-found' => "Pardon, d'URL no där Dir sicht gouf net fonnt (Et gëtt keng Kuerz-URL mat der ID $1)",~~
	139	+ 'shorturl-not-found-message' => "Pardon, d'URL no där Dir sicht gouf net fonnt (Et gëtt keng Kuerz-URL mat der ID $1)",
139	140	'shorturl-toolbox-title' => 'Kopéiert dës Kuerz-URL fir se ze notzen',
140	141	'shorturl-toolbox-text' => 'Kuerz URL',
141	142	);
—	—	@@ -145,7 +146,7 @@
146	147	$messages['mk'] = array(
147	148	'shorturl' => 'Кратка URL-адреса',
148	149	'shorturl-desc' => '[[Special:ShortUrl\|Кратка URL-адреса за пренасочувања]]',
149		~~- 'shorturl-not-found' => 'Нажалост, не ја најдов URL-адресата што ја барате (не постои кратка URL-адреса со назнаката $1)',~~
	150	+ 'shorturl-not-found-message' => 'Нажалост, не ја најдов URL-адресата што ја барате (не постои кратка URL-адреса со назнаката $1)',
150	151	'shorturl-toolbox-title' => 'Ископирајте ја оваа кратка врска за да ја споделите',
151	152	'shorturl-toolbox-text' => 'Кратка URL-адреса',
152	153	);
—	—	@@ -156,7 +157,7 @@
157	158	$messages['ms'] = array(
158	159	'shorturl' => 'URL Ringkas',
159	160	'shorturl-desc' => '[[Special:ShortUrl\|URL Ringkas untuk lencongan]]',
160		~~- 'shorturl-not-found' => 'Maaf, URL yang anda cari itu tidak dijumpai (Tidak wujudnya URL yang ber-ID $1)',~~
	161	+ 'shorturl-not-found-message' => 'Maaf, URL yang anda cari itu tidak dijumpai (Tidak wujudnya URL yang ber-ID $1)',
161	162	'shorturl-toolbox-title' => 'Salin pautan ringkas ini untuk dikongsi',
162	163	'shorturl-toolbox-text' => 'URL Ringkas',
163	164	);
—	—	@@ -168,7 +169,7 @@
169	170	$messages['nl'] = array(
170	171	'shorturl' => 'Korte URL',
171	172	'shorturl-desc' => '[[Special:ShortUrl\|Korte URL voor doorverwijzingen]]',
172		~~- 'shorturl-not-found' => 'De URL waarnaar u zoekt is niet gevonden. Er bestaat geen korte URL met ID $1',~~
	173	+ 'shorturl-not-found-message' => 'De URL waarnaar u zoekt is niet gevonden. Er bestaat geen korte URL met ID $1',
173	174	'shorturl-toolbox-title' => 'Deze korte verwijzing kopiëren om te delen',
174	175	'shorturl-toolbox-text' => 'Korte URL',
175	176	);
—	—	@@ -179,7 +180,7 @@
180	181	$messages['or'] = array(
181	182	'shorturl' => 'ଛୋଟ URL',
182	183	'shorturl-desc' => '[[Special:ShortUrl\|ଆଉଥରେ ଫେରନ୍ତା ଲିଙ୍କପାଇଁ ଛୋଟ URL]]',
183		- 'shorturl-not-found' => 'କ୍ଷମା କରିବେ, ଆପଣ ଦେଇଥିବା URLଟି ମିଳିଲା ନାହିଁ ($1 ନାଆଁରେ କିଛି ଛୋଟ URL ମିଳିଲା ନାହିଁ)',
	184	+ 'shorturl-not-found-message' => 'କ୍ଷମା କରିବେ, ଆପଣ ଦେଇଥିବା URLଟି ମିଳିଲା ନାହିଁ ($1 ନାଆଁରେ କିଛି ଛୋଟ URL ମିଳିଲା ନାହିଁ)',
184	185	'shorturl-toolbox-title' => 'ଏହି ଛୋଟ ଲିଙ୍କଟି ବାଣ୍ଟିବା ପାଇଁ ନକଲ କରନ୍ତୁ',
185	186	'shorturl-toolbox-text' => 'ଛୋଟ URL',
186	187	);
—	—	@@ -190,7 +191,7 @@
191	192	$messages['pt'] = array(
192	193	'shorturl' => 'URL Curta',
193	194	'shorturl-desc' => '[[Special:ShortUrl\|URL Curta para redireccionamentos]]',
194		~~- 'shorturl-not-found' => 'A URL que pretende não foi encontrada (não existe nenhuma URL Curta com a identificação $1)',~~
	195	+ 'shorturl-not-found-message' => 'A URL que pretende não foi encontrada (não existe nenhuma URL Curta com a identificação $1)',
195	196	'shorturl-toolbox-title' => 'Copie este link curto para partilhar',
196	197	'shorturl-toolbox-text' => 'URL Curta',
197	198	);
—	—	@@ -201,7 +202,7 @@
202	203	$messages['ru'] = array(
203	204	'shorturl' => 'Короткая ссылка',
204	205	'shorturl-desc' => '[[Special:ShortUrl\|Короткая ссылка для перенаправлений]]',
205		~~- 'shorturl-not-found' => 'К сожалению, указанный URL-адрес не найден (нет короткой ссылки с идентификатором $1)',~~
	206	+ 'shorturl-not-found-message' => 'К сожалению, указанный URL-адрес не найден (нет короткой ссылки с идентификатором $1)',
206	207	'shorturl-toolbox-title' => 'Скопируйте эту короткую ссылку',
207	208	'shorturl-toolbox-text' => 'Короткая ссылка',
208	209	);
—	—	@@ -221,7 +222,7 @@
222	223	$messages['ta'] = array(
223	224	'shorturl' => 'குறுந்தொடுப்பு',
224	225	'shorturl-desc' => '[[Special:ShortUrl\|வழிமாற்றுகளுக்கான குறுந்தொடுப்பு]]',
225		- 'shorturl-not-found' => 'மன்னிக்கவும், நீங்கள் எதிர்பார்க்கும் உரலி கிடைக்கவில்லை ($1 என்ற அடையாளத்துடன் கூடிய குறுந்தொடுப்பு ஏதுமில்லை)',
	226	+ 'shorturl-not-found-message' => 'மன்னிக்கவும், நீங்கள் எதிர்பார்க்கும் உரலி கிடைக்கவில்லை ($1 என்ற அடையாளத்துடன் கூடிய குறுந்தொடுப்பு ஏதுமில்லை)',
226	227	'shorturl-toolbox-title' => 'பகிர்வதற்காக இக்குறுந்தொடுப்பை நகலெடுக்கவும்',
227	228	'shorturl-toolbox-text' => 'குறுந்தொடுப்பு',
228	229	);
Index: trunk/extensions/ShortUrl/ShortUrl.utils.php
—	—	@@ -25,11 +25,11 @@
26	26	public static function encodeTitle( $title ) {
27	27	global $wgMemc;
28	28
29		~~- $memcKey = wfMemcKey( 'shorturls', 'title', $title->getFullText() );~~
	29	+ $memcKey = wfMemcKey( 'shorturls', 'title', $title->getPrefixedText() );
30	30	$id = $wgMemc->get( $memcKey );
31	31	if ( !$id ) {
32	32	$dbr = wfGetDB( DB_SLAVE );
33		~~- $query = $dbr->select(~~
	33	+ $entry = $dbr->selectRow(
34	34	'shorturls',
35	35	array( 'su_id' ),
36	36	array(
—	—	@@ -38,8 +38,7 @@
39	39	),
40	40	__METHOD__
41	41	);
42		~~- if ( $dbr->numRows( $query ) > 0 ) {~~
43		~~- $entry = $dbr->fetchObject( $query );~~
	42	+ if ( $entry !== false ) {
44	43	$id = $entry->su_id;
45	44	} else {
46	45	$dbw = wfGetDB( DB_MASTER );
—	—	@@ -68,17 +67,19 @@
69	68	$entry = $wgMemc->get( $memcKey );
70	69	if ( !$entry ) {
71	70	$dbr = wfGetDB( DB_SLAVE );
72		~~- $query = $dbr->select(~~
	71	+ $entry = $dbr->selectRow(
73	72	'shorturls',
74	73	array( 'su_namespace', 'su_title' ),
75	74	array( 'su_id' => $id ),
76	75	__METHOD__
77	76	);
78	77
79		~~- $entry = $dbr->fetchRow( $query ); // Less overhead on memcaching~~
	78	+ if ( $entry === false ) {
	79	+ return false; // No such shorturl exists
	80	+ }
80	81	$wgMemc->set( $memcKey, $entry, 0 );
81	82	}
82		~~- return Title::makeTitle( $entry['su_namespace'], $entry['su_title'] );~~
	83	+ return Title::makeTitle( $entry->su_namespace, $entry->su_title );
83	84	}
84	85
85	86	/**
Index: trunk/extensions/ShortUrl/ShortUrl.php
—	—	@@ -15,6 +15,13 @@
16	16	die( 1 );
17	17	}
18	18
	19	+// Configuration variables
	20	+// Prefix to use for the shortened URL. mod_rewrite (or equivalent) needs to be setup
	21	+// to produce a shorter URL
	22	+// Default is 'null' which just uses the (not so short) URL that all Special Pages get
	23	+// Eg: http://en.wikipedia.org/wiki/Special:ShortUrl/5234
	24	+$wgShortUrlPrefix = null;
	25	+
19	26	// Extension credits that will show up on Special:Version
20	27	$wgExtensionCredits['specialpage'][] = array(
21	28	'path' => __FILE__,
—	—	@@ -42,10 +49,7 @@
43	50	$wgResourceModules['ext.shortUrl'] = array(
44	51	'scripts' => 'js/ext.shortUrl.js',
45	52	'styles' => 'css/ext.shortUrl.css',
46		~~- 'dependencies' => array( 'jquery' ),~~
47	53	'localBasePath' => dirname( __FILE__ ),
48	54	'remoteExtPath' => 'ShortUrl'
49	55	);
50	56
51		~~-// Configuration~~
52		~~-$wgShortUrlPrefix = null;~~
Index: trunk/extensions/ShortUrl/css/ext.shortUrl.css
—	—	@@ -1,5 +1,4 @@
2	2	.title-shortlink {
3	3	font-size: small;
4		~~- display: Block;~~
5	4	margin: 0px;
6	5	}
Index: trunk/extensions/ShortUrl/SpecialShortUrl.php
—	—	@@ -36,12 +36,10 @@
37	37	global $wgOut;
38	38
39	39	$title = ShortUrlUtils::decodeURL( $par );
40		~~- if ( $title ) {~~
	40	+ if ( $title !== false ) {
41	41	$wgOut->redirect( $title->getFullURL(), '301' );
42		~~- return;~~
	42	+ } else {
	43	+ $wgOut->showErrorPage( 'shorturl-not-found-title', 'shorturl-not-found-message', array( $par ) );
43	44	}
44		~~- // Wrong ID~~
45		~~- $notFound = Html::element( 'p', array(), wfMsg( 'shorturl-not-found', $par ) );~~
46		~~- $wgOut->addHTML( $notFound );~~
47	45	}
48	46	}

Comments

#Comment by Nikerabbit (talk | contribs) 11:03, 15 November 2011

Looks good. In ShortUrl.php I'd use this commenting style

/**
 * This does something
 */
 $wgBunnies = 4;

Status & tagging log

19:01, 23 November 2011 Catrope (talk | contribs) changed the status of r103035 [removed: new added: ok]