phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[LukeWCS]

Bei meiner Sidebar EXT bemängelt VA in der ext.php Zeile 28 jenes:

Unused variable $lang.

soweit ich dies noch in Erinnerung habe, benötigt man dies doch unter 3.1 $lang = $user->lang;
Die komplette ext.php kann man sich in Pastebin ansehen.

Du weisst der Variable $lang die Funktion lang aus der Klasse user zu, verwendest diese Variable $lang aber nirgends. Von daher hat VA Recht: die Zeile kann raus.

Was 3.1 betrifft, diese Methode die lang Funktion anzusprechen, ist seit 3.2 als DEPRECATED eingestuft und wird, wie ich eben im Source von phpBB gesehen habe, bei 4.0 endgültig entfernt. Bis dahin fliesst noch viel Wasser den Bach runter. Du kannst also den Rest so lassen wie es ist, oder die neue Methode ab 3.2 nutzen, da deine Ext eh für 3.2+ vorgesehen ist.

Da war noch was mit JS?

edit: also so langsam wirds beängstigend Doc.

[Kirk]

Ich hätte schwören konnen das man dies $lang = $user->lang; unter 3.1 benötigt, habe extra die letzte 3.1er Version inst. um zu sehen wie es sich ohne verhält und siehe da man brauch es wirklich nicht, so kann man sich irren.
@Patrick
In der Extension selbst werden ja schon die neuen Methoden benutzt, es ging nur um die ext.php das niemand unter 3.1 diese EXT inst. kann.

Was die js betrifft, gibt es allerdings noch ein Problem. Der PHP_CodeSniffer bemängelt in der ./ext/kirk/sidebar/styles/prosilver/template/sidebar-responsive.js in Zeile 48 jenes:

Inline control structures are not allowed

die komplette js Datei kann man sich in Pastebin ansehen.

[LukeWCS]

Ich hätte schwören konnen das man dies $lang = $user->lang; unter 3.1 benötigt, habe extra die letzte 3.1er Version inst. um zu sehen wie es sich ohne verhält und siehe da man brauch es wirklich nicht, so kann man sich irren.

Man muss hier differenzieren: das Konstrukt $user->lang wird unter 3.1 sehr wohl benötigt, da irrst du dich nicht. Aber wenn du das Konstrukt selber dann anschliessend nicht verwendest, dann ist das komplett überflüssig, egal ob 3.1 oder 3.2.

@Patrick
In der Extension selbst werden ja schon die neuen Methoden benutzt, es ging nur um die ext.php das niemand unter 3.1 diese EXT inst. kann.

Ah stimmt ja, sorry, mein Fehler.

Was die js betrifft, gibt es allerdings noch ein Problem. Der PHP_CodeSniffer bemängelt in der ./ext/kirk/sidebar/styles/prosilver/template/sidebar-responsive.js in Zeile 48 jenes:

Inline control structures are not allowed

Inline Kontrollstrukturen sind im phpBB Umfeld generell nicht erlaubt, weder bei PHP noch bei JS. "Inline control structures" sind Strukturen, bei denen der auszuführende Code der zur einer Abfrage gehört, in derselben Zeile wie die Abfrage notiert wurde. Und das ist wie gesagt nicht erlaubt.

Aus dieser merkwürdigen Konstruktion bin ich ehrlich gesagt nicht sofort schlau geworden, weshalb ich mir das mal direkt an der Quelle angeschaut habe und dann habe ich erst verstanden, was eigentlich tatsächlich in der betreffenden Funktion get_cookie(Name) beabsichtigt war. ^^ Dazu eine Frage: hast du diesen JS Code ebenfalls direkt von der Quelle, oder von einer anderen phpBB Ext/Mod?

[Kirk]

Ich habe diesen JS Code direkt von dynamicdrive.com, allerdings hatte der EC an mehreren Stellen diese Inline control structures are not allowed bemängelt, bis auf die oben genannte Stelle konnte ich alle Fehler in dieser js Datei beseitigen.

Edit:
Danke für deine Erklärung Patrick, ich denke ich hab die Lösung gefunden, ich habe dies:

Code: Alles auswählen

function get_cookie(Name) {
	var search = Name + "="
	var returnvalue = "";
	if (document.cookie.length > 0)
	{
		offset = document.cookie.indexOf(search)
		if (offset != -1)
		{
			offset += search.length
			end = document.cookie.indexOf(";", offset);
			if (end == -1) (end = document.cookie.length);
			{
				returnvalue=unescape(document.cookie.substring(offset, end))
			}
		}
	}
	return returnvalue;
}

so geändert:

Code: Alles auswählen

function get_cookie(Name) {
	var search = Name + "="
	var returnvalue = "";
	if (document.cookie.length > 0)
	{
		offset = document.cookie.indexOf(search)
		if (offset != -1)
		{
			offset += search.length
			end = document.cookie.indexOf(";", offset);
			if (end == -1)
			{
				(end = document.cookie.length);
				returnvalue=unescape(document.cookie.substring(offset, end))
			}
		}
	}
	return returnvalue;
}

Funktionieren tut alles wie es soll und der PHP_CodeSniffer hat auch nichts mehr zu bemängeln.

[LukeWCS]

Ich habe diesen JS Code direkt von dynamicdrive.com, allerdings hatte der EC an mehreren Stellen diese Inline control structures are not allowed bemängelt, bis auf die oben genannte Stelle konnte ich alle Fehler in dieser js Datei beseitigen.

Ja CS meldet das, weil das gegen die phpBB Vorgaben verstösst. Okay, dann war das im Prinzip dein Versuch den von CS gemeldeten Fehler zu beheben. Dann können wir auch das Original nehmen und das an die Vorgaben anpassen. Das Original sieht auch schon etwas merkwürdig aus, so ganz ohne Einrückungen und es fehlt auch an mehreren Stellen ein Semikolon. Aber okay, das Ding ist auch mindestens 16 Jahre alt.

Code: Alles auswählen

function get_cookie(Name) {
	var search = Name + "=";
	var returnvalue = "";
	if (document.cookie.length > 0) {
		offset = document.cookie.indexOf(search);
		if (offset != -1) {
			offset += search.length;
			end = document.cookie.indexOf(";", offset);
			if (end == -1) {
				end = document.cookie.length;
			}
			returnvalue = unescape(document.cookie.substring(offset, end));
		}
	}
	return returnvalue;
}

Was ich ebenfalls geändert habe, sind die Block-Starts, diese müssen bei JS (nicht bei PHP!) immer in der gleichen Zeile stehen, wie die Deklarationen von Bedingungen, Schleifen und Funktionen. CS meldet diesen Fehler bei JS leider nicht, das steht aber in den Richtlinien. Das musst du also dann noch in den anderen Funktionen der Datei bei den Blöcken (function, if, for) anpassen. Ausserdem auch die fehlenden Semikolons setzen und bei Zuweisungen sollten auch Leerzeichen eingesetzt werden, z.B:

Schlecht:
for (var i=0; i<ar.length; i++){

Besser:
for (var i = 0; i < ar.length; i++) {

edit: grad erst dein Edit gesehen, stimmt noch nicht ganz, weil bei dir returnvalue in der falschen Bedingungsebene ist.

[Kirk]

Danke, das mit den Einrückungen war ich so aus den php Dateien gewohnt.

[LukeWCS]

Danke, das mit den Einrückungen war ich so aus den php Dateien gewohnt.

Ich ebenfalls, auch von anderen Sprachen. Aber okay, so sind die Richtlinien.

[LukeWCS]

Da gerade keine Berichte auf dem Server gespeichert sind, war der Zeitpunkt ideal: neue Version mit "Meine Berichte" installiert.

[LukeWCS]

In den letzten 2 Monaten gab es Updates für CS und VA:

CS 3.5.6:
Primär Anpassungen für PHP 8 sowie einige Fixes. PHP 8 ist für uns "noch" ohne Bedeutung. Die Fixes betreffen uns auch nicht, wenn ich das richtig sehe.

VA 2.8.3:
Die PHP Mindestversion wurde von 5.6 auf 5.4 reduziert. Für uns ebenfalls ohne Bedeutung, da EC mit PHP 7.2 läuft und ohnehin mindestens 7.0 benötigt.

EC 1.3.3:
Ausserdem ein Update von EC selbst. Weiterhin Startbeitrag aktualisiert.

[Crizzo]

Danke für deine Arbeit.

Finde das übersichtlicher als der Travis CI Output, nutze es daher doch recht oft.