Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years 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
Keywords: Cc:
Blocked By: Blocking:

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 by Gerhard Waldemair, 5 years ago

Resolution: closed
Status: newclosed

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 5 years ago by Torsten Riemer (previous) (diff)

comment:2 by Torsten Riemer, 5 years ago

Milestone: modified-shop-2.0.6.1

comment:3 by oneQ, 5 years ago

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 by oneQ, 5 years ago

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?

in reply to:  4 comment:5 by Torsten Riemer, 5 years ago

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 by oneQ, 5 years ago

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 by Torsten Riemer, 5 years ago

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 by Gerhard Waldemair, 5 years ago

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 5 years ago by Gerhard Waldemair (previous) (diff)

comment:9 by oneQ, 5 years ago

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 by Torsten Riemer, 5 years ago

Danke dir!

comment:11 by oneQ, 5 years ago

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 by Torsten Riemer, 5 years ago

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

Modify Ticket

Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.