Skip to content

[RFC] Added possibility to configure PresenterFactory mapping via array#119

Closed
ghost wants to merge 1 commit into
masterfrom
unknown repository
Closed

[RFC] Added possibility to configure PresenterFactory mapping via array#119
ghost wants to merge 1 commit into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Feb 14, 2016

Copy link
Copy Markdown

BC break: no
Documentation: needed

Solves: #101

@ghost

ghost commented Feb 14, 2016

Copy link
Copy Markdown
Author

(Still in progress.)

I realize this must be written differentlly, Parts of array must be validated and this must be allowed:

$factory->setMapping([
    '*' => ['App\\', '*'],
]);
Assert::same('App\RedDwarf', $factory->formatPresenterClass('RedDwarf'));

@ghost

ghost commented Feb 15, 2016

Copy link
Copy Markdown
Author

Ok, there is a proof of concept solution (I moved from red to greeen). Plase give me some feedback if I didn't miss any case or if there any missing functionality.

Then I'll do some code cleaning & refactoring. I would like to extract the logic of parsing into some "PresenterMappingParser".

@ghost ghost changed the title Added possibility to configure PresenterFactory mapping via array [RFC] Added possibility to configure PresenterFactory mapping via array Feb 15, 2016
@milo

milo commented Mar 29, 2016

Copy link
Copy Markdown
Member

Ref #101

@milo milo added this to the 2.4 milestone Mar 29, 2016
@matej21

matej21 commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

@achse can you please simplify this? is an array with 1 and 2 items really necessary? I think the version with 3 items is sufficient.

@ghost

ghost commented Mar 31, 2016

Copy link
Copy Markdown
Author

I am very busy right now. Please, anyone is free to fork my PR and finish it or write it from scratch. Just tell here, to prevent simultanous work. I dont know when I get time to finish this.

@dg dg closed this in fa3ba64 Apr 1, 2016
dg added a commit that referenced this pull request Apr 3, 2016
dg added a commit that referenced this pull request Apr 6, 2016
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.

2 participants