Opened 2 weeks ago

Closed 6 days ago

Last modified 14 hours ago

#2011 closed Frage (closed)

Einheitliches auto-include für Modul-Typen

Reported by: oneQ Owned by:
Priority: normal Milestone:
Component: Module Version: 2.0.6.0

Description

Mit 2.0.6.0 sind jetzt Einträge wie

foreach(auto_include($module_directory, substr($file_extension, 1)) as $file)
foreach(auto_include($module_directory, $file_extension) as $file)
foreach(auto_include($module_dir.$module_type.'/','php') as $file)

vom bisherigen auto_include Syntax abweichen und meist mit switch/case auch unterschiedlich initialisiert sind.

Wurde hier bewusst mal module_dir und mal mudule_directory verwendet?
Bei $module_dir wird $module_type als Variable mitgegeben, bei $module_directory das Unterverzeichnis/Type jeweils "hardgecoded" dran gehangen. Ist der Effekt nicht der gleiche?
Wenn ja, sollte das dann nicht einheitlich umgesetzt werden (Stichwort: "System")?

Attachments (0)

Change History (12)

comment:1 Changed 6 days ago by GTB

  • Resolution set to closed
  • Status changed from new to closed

Replying to oneQ:

[...]
Wurde hier bewusst mal module_dir und mal mudule_directory verwendet?
[...]

Ja, da es unterschiedliche Einsätze dazu gibt (frontend / backend)

Last edited 6 days ago by Tomcraft (previous) (diff)

comment:2 Changed 6 days ago by Tomcraft

  • Milestone modified-shop-2.0.6.1 deleted

comment:3 Changed 5 days ago by oneQ

Ok. Danke für die Antwort. Dann schaue ich mir das bei Gelegenheit noch einmal an und überlege mir wie ich das im Wiki ggf. anpasse.

comment:4 follow-up: Changed 4 days ago by oneQ

Sorry. So richtig verstanden habe ich es doch noch nicht.

In update_module_configuration.inc.php ist $module_dir je nach $module_type entweder

        $module_dir = DIR_FS_CATALOG.DIR_ADMIN.'includes/modules/';

oder

        $module_dir = DIR_FS_CATALOG.'includes/modules/';

wobei $module_type "der switch ist" (z.B. 'system') und

    foreach(auto_include($module_dir.$module_type.'/','php') as $file) {

verwendet wird.

In module_export.php und modules.php wird z.B.

        $module_directory = DIR_FS_ADMIN.DIR_WS_MODULES . 'system/';

verwendet, wobei $module_type im selben case mit 'system' gesetzt wird und später dann

              foreach(auto_include($module_directory, [...]

verwendet.

Für die letzten beiden Dateien könnte es doch genauso wie in der ersten Datei mit

        $module_dir = DIR_FS_CATALOG.DIR_ADMIN.'includes/modules/';

und

        foreach(auto_include($module_dir.$module_type. [...]

umgesetzt werden ($module_type ist ja definiert). Der unterschiedliche Weg wie man in das Modulverzeichnis vom Admin kommt, irritiert mich auch noch etwas.

Ist DIR_FS_CATALOG.DIR_ADMIN.'includes/modules/ nicht das gleiche wie DIR_FS_ADMIN.DIR_WS_MODULES?

Ich liege vielleicht auch komplett daneben, aber die "schlanke" Begründung mit dem Einsatz frontend/backend ist für mich nicht nachvollziebar, wieso mal die Pfadangaben mit $module_type und mal ohne im auto_include verwendet wird, wobei in allen fällen $module_type gesetzt ist und im Beispiel 'system' der Pfad im auto_include in allen Fällen auf ~/admin/includes/modules/system/ zeigt!?! Wo ist da mein Denkfehler?

comment:5 in reply to: ↑ 4 Changed 4 days ago by Tomcraft

Replying to oneQ:

[...]
Ist DIR_FS_CATALOG.DIR_ADMIN.'includes/modules/ nicht das gleiche wie DIR_FS_ADMIN.DIR_WS_MODULES?
[...]

Nein! DIR_FS_ADMIN steht nur im Adminbereich zur Verfügung. Aus dem Frontend kommend muss DIR_FS_CATALOG.DIR_ADMIN benutzt werden.

comment:6 Changed 33 hours ago by oneQ

Check! Wegen des Speicherorts der path.php und weil ~/inc/update_module_configuration.inc.php außerhalb vom Adminverzeichnis ist.

Wieso einmal der $type separat zu $module_dir und einmal schon Bestandteil $module_directory ist, ist vermutlich einfach so. Sind diese auto_includes überhaupt für das Auto include Modul System gedacht? Oder ist das für die "interne" Verwendung vom Team? Im letzten Fall würde ich es dann aus dem Wiki werfen. Ich habe einfach stumpf nach auto_includes gesucht und alles verarbeitet was im Ergebnis stand.

comment:7 Changed 17 hours ago by Tomcraft

Ich würde meinen, dass du folgende auto_includes raus schmeissen kannst:
-/admin/includes/classes/categories.php -> /templates/'.CURRENT_TEMPLATE.$path, 'html'
-/admin/modules.php -> /admin/includes/extra/submenu/modules/$module_directory, $file_extension
-/admin/module_export.php -> $module_directory
-/inc/update_module_configuration.inc.php -> $module_dir.$module_type

Ich habe in der Anleitung gerade mal ein paar Rechtschreibfehler korrigiert: https://www.modified-shop.org/mediawiki/index.php?title=Auto_include_Modul_System&type=revision&diff=4033&oldid=4030

comment:8 Changed 17 hours ago by GTB

Check! Wegen des Speicherorts der path.php und weil ~/inc/update_module_configuration.inc.php außerhalb vom Adminverzeichnis ist.

weil die Funktion auch aus dem Frontend aus aufgerufen werden kann

-/admin/includes/classes/categories.php -> /templates/'.CURRENT_TEMPLATE.$path, 'html'
-/admin/modules.php -> /admin/includes/extra/submenu/modules/$module_directory, $file_extension
-/admin/module_export.php -> $module_directory
-/inc/update_module_configuration.inc.php -> $module_dir.$module_type

sehe ich auch so

Last edited 17 hours ago by GTB (previous) (diff)

comment:9 Changed 16 hours ago by oneQ

Ich habe in der Anleitung gerade mal ein paar Rechtschreibfehler korrigiert

Danke. Mit den Fehlern in der Tabelle warst Du etwas zu schnell ;) . Stand schon auf der ToDo? Liste. Da hat der Substring in einigen Fällen falsch zugeschlagen und die letzten 3 Zeichen abgeschnitten. Script ist schon entsprechend korrigiert.

Ich würde meinen, dass du folgende auto_includes raus schmeissen kannst:

Da lag ich mit meiner Vermutung ja mal richtig. 3 von 4 Treffer :) . Werde mein Script für die vier Fälle anpassen und dann die Tabelle heute oder morgen aktualisieren.

comment:10 Changed 15 hours ago by Tomcraft

Danke dir!

comment:11 Changed 14 hours ago by oneQ

Gerne. Teepause schnell genutzt. Sollte erledigt sein. Eine Kleinigkeit ist mir noch aufgefallen. Ab 2.0.5.0 hat sich ein zusätzliches Leerzeichen vor dem 'php' eingeschlichen. Vermutlich nicht systemrelevant. Hatte nur bei mir zu einem zweiten Eintrag geführt.

~\admin\includes\filenames.php
	Zeile 14: foreach(auto_include(DIR_FS_ADMIN.'includes/extra/filenames/', 'php') as $file) require ($file);

Ich hoffe jetzt passt alles.

comment:12 Changed 14 hours ago by Tomcraft

Danke dir! Passt alles soweit ich sehe. ;-)
Das zusätzliche Leerzeichen ist gewollt zwecks Code-Konventionen.

Add Comment

Modify Ticket

Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.