mozart icon indicating copy to clipboard operation
mozart copied to clipboard

Config value object instead of manually reading the file

Open coenjacobs opened this issue 4 years ago • 1 comments

I'd probably want to leave this out of 0.6.0, but haven't decided or full tested yet.

coenjacobs avatar May 23 '20 13:05 coenjacobs

I think a config object is a very good idea – it gives a good place for input sanitisation (e.g. I think relative directory paths should always have no leading slash and should have a trailing slash), defaults, and documentation. I think unit/integration tests could be tidied up with a subclass with sensible defaults which could be extended.

Your stringly typed approach misses out on a lot of benefits that could be gained with explicit getters and setters. And I'm hoping with the project PHP update and Pslam, you agree.

I've built a class with all the composer/extra/mozart options as properties. I'm very much in draft status, but for discussion:

I've been playing around with JsonMapper/JsonMapper but now I think cweiske/jsonmapper is worth a look – both for how Package.php could parse a composer.json into its typed class.

I've also looked at Composer's own Factory::create() to parse composer.json. It doesn't immediately show a huge benefit (from debugging in PhpStorm), but I'm thinking there might be benefits where e.g. the autoload key has used array rather than object (vice versa), or e.g. returns single/multiple maps like #63. What is valid for Composer is all that needs to be valid for Mozart so what has been parsed by them is accurately what Mozart should consider.

I was also looking at Composer's Factory::create() to see if it could help my #38 issue (mentioned just recently in #66) for delete_vendor_directories to use to safeguard against deleting directories with changes made – Composer itself warns me when I run composer install if I have modified any files.

BrianHenryIE avatar Feb 08 '21 03:02 BrianHenryIE