Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_Ex ファイルが存在しない場合は元ファイルのエイリアスを作成する #526

Merged
merged 18 commits into from
Apr 20, 2022

Conversation

nanasess
Copy link
Contributor

@nanasess nanasess commented Mar 2, 2022

data/class_extends 以下のファイルをカスタマイズすることで、既存のロジックをオーバーライド可能だが、ファイルを開かないと、カスタマイズされているかどうかわからない。
特に引き継ぎなどで、カスタマイズされているファイルの把握が難しい。

この修正は、カスタマイズが必要な _Ex ファイルのみ作成すれば良いため、一見してどのファイルがカスタマイズされているかわかりやすくなる

TODO

  • LC_* の対応
  • loadClassFileChange の対応
  • .gitkeep の追加
  • 以下の SC_* も対応可能か調査 app_initial.php の sql_autoload_register を登録する前に必要なので削除できない
    • data/class_extends/SC_ClassAutoloader_Ex.php
    • data/class_extends/helper_extends/SC_Helper_Plugin_Ex.php
    • data/class_extends/SC_Query_Ex.php
  • 以下の LC_* も対応可能か調査
    • data/class_extends/page_extends/frontparts/bloc/ 以下のファイル
    • data/class_extends/page_extends/mypage/LC_Page_AbstractMypage_Ex.php
      • 決済モジュールやプラグインなどで、たくさん require_once が書かれていそうなので削除しない方が良さそう
    • data/class_extends/page_extends/LC_Page_Ex.php
      • user_data 以下に生成される PHP などで、たくさん require_once が書かれていそうなので削除しない方が良さそう
    • data/class_extends/page_extends/admin/LC_Page_Admin_Ex.php
      • 決済モジュールやプラグインなどで、たくさん require_once が書かれていそうなので削除しない方が良さそう
    • data/class_extends/page_extends/error/ 以下のファイル

@nanasess nanasess marked this pull request as draft March 2, 2022 08:06
@nobuhiko
Copy link
Contributor

nobuhiko commented Mar 2, 2022

👍

@nanasess nanasess changed the title _Ex ファイルが存在しない場合は元ファイルのエイリアスを作成する [WIP] _Ex ファイルが存在しない場合は元ファイルのエイリアスを作成する Mar 2, 2022
@seasoftjapan
Copy link
Contributor

*_Ex.php を作成する決済モジュールがあった気がするので、考慮すると .gitkeep の配置が必要でしょうか。
本当は、無いほうが、分かりやすくて良いですが。

@nanasess
Copy link
Contributor Author

nanasess commented Mar 2, 2022

@seasoftjapan そうですね。 .gitkeep あった方がよさそうです。TODO に入れておきます

@nanasess
Copy link
Contributor Author

LC_* は、 html 以下の PHP ファイルに大量の require_once がベタ書きされているため、どう対応するか悩み中。

  • require_once をすべて削除するのが良いか?
    • CLASS_EX_REALDIR を変更し、変更先にダミーのファイルを配置することで、 *_Ex.php ファイルが削除されたことによるエラーは防げそう
  • [WIP]ルーティングの実装 #525 と一緒に進めるべきか?

@nanasess
Copy link
Contributor Author

nanasess commented Apr 4, 2022

LC_* は、 html 以下の PHP ファイルに大量の require_once がベタ書きされているため、どう対応するか悩み中。

2.17.0 以降は composer classmap によってオートローディングの対象となっていたため、 require_once を削除しても大きな問題はなさそうなので削除しました

find html -name '*.php' | xargs sed -i '/^require_once CLASS_EX_REALDIR/d'
find data/class/pages -name '*.php' | xargs sed -i '/^require_once CLASS_EX_REALDIR/d'

@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2022

Codecov Report

Merging #526 (aa39e80) into master (a47b46e) will increase coverage by 0.09%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master     #526      +/-   ##
==========================================
+ Coverage   54.53%   54.63%   +0.09%     
==========================================
  Files          76       76              
  Lines        9065     9058       -7     
==========================================
+ Hits         4944     4949       +5     
+ Misses       4121     4109      -12     
Flag Coverage Δ
tests 54.63% <42.85%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
data/class/api/operations/AddrFromZip.php 0.00% <ø> (ø)
data/class/api/operations/BrowseNodeLookup.php 0.00% <ø> (ø)
data/class/api/operations/CartAdd.php 0.00% <ø> (ø)
data/class/api/operations/CartClear.php 0.00% <ø> (ø)
data/class/api/operations/CartCreate.php 0.00% <ø> (ø)
data/class/api/operations/CartGet.php 0.00% <ø> (ø)
data/class/api/operations/CartModify.php 0.00% <ø> (ø)
data/class/api/operations/Default.php 0.00% <ø> (ø)
data/class/api/operations/GetVersion.php 0.00% <ø> (ø)
data/class/api/operations/ItemLookup.php 0.00% <ø> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a47b46e...aa39e80. Read the comment docs.

@nanasess
Copy link
Contributor Author

nanasess commented Apr 6, 2022

おおむね対応できましたが、エディタのコード補完がエイリアスを補完してくれないので、対応方法を検討中

@nobuhiko
Copy link
Contributor

nobuhiko commented Apr 6, 2022

削除できないファイルの理由を残しておかないとわけわからなくなりそうですね

@nanasess
Copy link
Contributor Author

nanasess commented Apr 6, 2022

@nobuhiko
削除できないファイルのコメントにしっかり書いておくとか、 .gitkeep ではなく README.md にして使い方をしっかり書いておくとかした方が良いかなと思いました

@nanasess
Copy link
Contributor Author

nanasess commented Apr 7, 2022

エディタのコード補完がエイリアスを補完してくれないので、対応方法を検討中

こんな感じのスタブを require-dev に入れておけば良さそうです。
https://github.com/nanasess/ec-cube2-class-extends-stubs

@nanasess nanasess changed the title [WIP] _Ex ファイルが存在しない場合は元ファイルのエイリアスを作成する _Ex ファイルが存在しない場合は元ファイルのエイリアスを作成する Apr 7, 2022
@nanasess nanasess marked this pull request as ready for review April 7, 2022 05:24
@nanasess
Copy link
Contributor Author

nanasess commented Apr 7, 2022

README も作成しましたので WIP はずしました

@chihiro-adachi
Copy link
Contributor

@nanasess
ありがとうございます、マージしました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants