Created
December 16, 2010 17:11
-
-
Save jmikola/743667 to your computer and use it in GitHub Desktop.
Log of #symfony-dev meeting 20101216 (all times GMT-5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Dec 16 11:01:18 <lsmith> ok let s go then | |
| Dec 16 11:01:20 <lsmith> johanness .. Progress on the security firewall: http://bit.ly/hfkdmg | |
| Dec 16 11:01:24 <lsmith> take it away :) | |
| Dec 16 11:01:26 <johanness> hehe | |
| Dec 16 11:01:35 <lsmith> i have to eat my rasberry/blueberry cake thingi :) | |
| Dec 16 11:01:53 <johanness> basically, the serialization is now almost finished, there are still a couple of issues to iron out but most of it has been merged | |
| Dec 16 11:02:05 <johanness> remember-me is still blocked by one of these issues | |
| Dec 16 11:02:20 * Felicitus (~timo@idefix.timohummel.com) has joined #symfony-dev | |
| Dec 16 11:02:29 <lsmith> johanness: you mean http://trac.symfony-project.org/ticket/9359 ? | |
| Dec 16 11:03:03 <johanness> lsmith: yeah exactly, i also made a comment on the password encoding pull-request regarding this | |
| Dec 16 11:03:21 <lsmith> johanness: for those interested .. should https://github.com/schmittjoh/DoctrineUserBundle/tree/symfony2Security work now? | |
| Dec 16 11:03:44 <johanness> lsmith: untested with latest head, i think ornicar works on merging this | |
| Dec 16 11:04:05 <johanness> for ACL, i think fabien needs still some more time to review it | |
| Dec 16 11:04:21 <lsmith> ACL should apply cleanly yo current fabpot/master? | |
| Dec 16 11:04:30 <henrikbjorn> johanness have you thought about the getProviderName() method fabien proposed in that ticket? | |
| Dec 16 11:04:38 <henrikbjorn> before we all go change our userproviders :) | |
| Dec 16 11:05:02 <johanness> henrikbjorn: the basic plan is to move this method from the token to the user | |
| Dec 16 11:05:15 <johanness> but if you use the built-in providers you won't notice the change :) | |
| Dec 16 11:05:28 <henrikbjorn> you will if your repositories are used | |
| Dec 16 11:05:33 <henrikbjorn> for entities and/or documents | |
| Dec 16 11:05:56 <johanness> yeah right | |
| Dec 16 11:06:20 <henrikbjorn> which most will (i think) to let users use their username and email address to login with | |
| Dec 16 11:07:19 <lsmith> johanness: is there anything people can do to move this stuff along? | |
| Dec 16 11:07:37 <lsmith> or is it mainly on you updating your pulls and fabien to do the merges | |
| Dec 16 11:07:38 <lsmith> ? | |
| Dec 16 11:07:39 <henrikbjorn> i also think we need to decide what to call the load method loadUserByUsername is incorrect | |
| Dec 16 11:07:43 <henrikbjorn> as its more a identifier | |
| Dec 16 11:07:59 <lsmith> henrikbjorn: yeah .. that was a topic for discussion on the list before | |
| Dec 16 11:08:11 <johanness> lsmith: i don't see how since these are not seperate things, but they all depend on eachother somehow | |
| Dec 16 11:08:11 <lsmith> spring calls it "principal" afaik | |
| Dec 16 11:08:17 <mvrhov> loadUserByUID ? | |
| Dec 16 11:08:24 <henrikbjorn> yep but no decision was made sadly and no we are touching the interfaces we should fix the naming as well | |
| Dec 16 11:08:27 <johanness> lsmith: ACL could be reviewed though, it's the only thing that is truly independent | |
| Dec 16 11:08:30 <lsmith> people have suggested handle, identifier etc | |
| Dec 16 11:08:44 <henrikbjorn> Identifier is the most neutral one i think | |
| Dec 16 11:08:51 <mvrhov> +1 | |
| Dec 16 11:09:29 <henrikbjorn> btw sorry for hijacking the topic :P | |
| Dec 16 11:09:46 <johanness> there's no real topic :) | |
| Dec 16 11:09:48 <lsmith> thats fine .. i think there isnt much more to say at this point | |
| Dec 16 11:09:48 <weaverryan> If it makes sense to change it, then I say we change it to something that at least a few people like and run with it (we could argue naming forever) | |
| Dec 16 11:09:50 <jmikola|w> johanness: are these changes likely to make the current security docs obsolete? | |
| Dec 16 11:09:52 <johanness> everything security i think | |
| Dec 16 11:09:56 <jmikola|w> or irrelevant in some respects? | |
| Dec 16 11:10:15 <henrikbjorn> jmikola|w the docs are currently obsolete | |
| Dec 16 11:10:28 <johanness> jmikola|w: docs could some updating i guess | |
| Dec 16 11:10:35 <johanness> could use* | |
| Dec 16 11:10:55 <lsmith> probably the "best" documentation in the near future is DoctrineUserBundle | |
| Dec 16 11:11:02 <johanness> regarding renaming, this will be touching quite some files | |
| Dec 16 11:11:08 <lsmith> but hopefully the actual docs will be updated too | |
| Dec 16 11:11:11 <johanness> we should be sure before we approach this | |
| Dec 16 11:11:35 <henrikbjorn> the files are only in the security right? :) | |
| Dec 16 11:11:42 <lsmith> i think we have almost universal agreement that "username" is not good in this contex | |
| Dec 16 11:11:44 <lsmith> context | |
| Dec 16 11:11:59 <lsmith> i have not heard a single person speak in favor of it .. even fabien isnt such a huge fan of it | |
| Dec 16 11:12:09 <lsmith> and said he just felt that principal was a bit freaky for php folk | |
| Dec 16 11:12:16 <weaverryan> when someone does take on the documentation, please just let us know so we don't all end up re-writing it at the same time | |
| Dec 16 11:12:28 <henrikbjorn> and that only leaves identifier | |
| Dec 16 11:12:33 * ajessu (~albert@230.Red-83-41-241.dynamicIP.rima-tde.net) has joined #symfony-dev | |
| Dec 16 11:12:38 <henrikbjorn> or is there other suggestions? | |
| Dec 16 11:12:44 <weaverryan> "principal" is the person in charge of grade schools here in the US ;) | |
| Dec 16 11:12:46 <Seldaek> there were a lot, but I can't remember | |
| Dec 16 11:12:53 <lsmith> henrikbjorn: i prefer "handle | |
| Dec 16 11:13:00 <Seldaek> principal imo makes sense, but it's gonna freak out people I guess | |
| Dec 16 11:13:00 <henrikbjorn> thats also good | |
| Dec 16 11:13:02 <jmikola|w> weaverryan: it also has context for stakeholders and investors :) | |
| Dec 16 11:13:02 <lsmith> but i can live with identifier too | |
| Dec 16 11:13:19 <Seldaek> UUID? | |
| Dec 16 11:13:23 <henrikbjorn> Seldaek: how does it make sense :) ? | |
| Dec 16 11:13:30 <jmikola|w> so, concisely, can someone explain what DUB provides? :) | |
| Dec 16 11:13:31 <weaverryan> I did kinda like identifier | |
| Dec 16 11:13:32 <mvrhov> universal user id :D | |
| Dec 16 11:13:41 <mvrhov> == UUID | |
| Dec 16 11:13:46 <Seldaek> henrikbjorn: well, it is the principal field with which you reference the user, no matter which field/value-type that is | |
| Dec 16 11:13:51 <jmikola|w> as i wrongfully assumed that since fabpot made a doctrine provider in security component, it was not needed | |
| Dec 16 11:13:57 <lsmith> jmikola|w: it provides facilities for creating users .. and it will also provide facilities for groups | |
| Dec 16 11:14:06 <jmikola|w> ok, so more admin interface | |
| Dec 16 11:14:07 <Felicitus> sorry I didn't follow the conversation and I need to go to bed (dead tired) but one thing which is maybe not optimal is that we have "username" and "password", even in third party packages. In fact, shouldn't that kept to the developer in a much more simple way as it is now? I have postponed integration of Authorization as it is very complex from my POV - so isn't a passed "user" or "authenticated" object enough for sf2 to work with? | |
| Dec 16 11:14:25 <lsmith> it also provides base entities for those | |
| Dec 16 11:14:34 <johanness> jmikola|w: sending emails, confirming emais, etc. | |
| Dec 16 11:14:35 * everzet has quit (Quit: everzet) | |
| Dec 16 11:14:38 <henrikbjorn> handle is better than identifier as most users would be entities or documents and therefor already have a identifier | |
| Dec 16 11:14:40 <lsmith> jmikola|w: no .. it also provides impelementations for frontend controllers | |
| Dec 16 11:14:55 <lsmith> password recovery, email activation for new accounts | |
| Dec 16 11:14:58 * vjousse (~vjousse@88.140.74.151) has joined #symfony-dev | |
| Dec 16 11:15:01 <jmikola|w> lsmith: and the form that would use the formlistener in security component? | |
| Dec 16 11:15:05 <henrikbjorn> Seldaek: dont even know how to translate principal into my language | |
| Dec 16 11:15:14 * russ__ has quit (Remote host closed the connection) | |
| Dec 16 11:15:21 <Seldaek> henrikbjorn: well, it's just synonym of "main" kind of.. | |
| Dec 16 11:15:22 <lsmith> jmikola|w: there are example forms and templates too | |
| Dec 16 11:15:24 <jmikola|w> Felicitus: security component definitely does not require username/password | |
| Dec 16 11:15:29 <lsmith> but most of the time you will want to do your own | |
| Dec 16 11:15:36 <jmikola|w> not sure which bundles you're looking at, but they may be antiquated | |
| Dec 16 11:15:41 <lsmith> ok .. we have reached the time box .. | |
| Dec 16 11:15:42 <ornicar> DUB also provide CLI commands for managing users | |
| Dec 16 11:16:01 <lsmith> i would say that we have agreed on changing "Username" to something else | |
| Dec 16 11:16:14 <henrikbjorn> yes handle/identifier | |
| Dec 16 11:16:21 <lsmith> ok lets move on | |
| Dec 16 11:16:21 <Seldaek> /principal | |
| Dec 16 11:16:22 <henrikbjorn> whould we make a doodle | |
| Dec 16 11:16:22 <Seldaek> :) | |
| Dec 16 11:16:30 <avalanche123> Seldaek +1 | |
| Dec 16 11:16:31 <johanness> identifier is what i'm using in the ACL component | |
| Dec 16 11:16:32 <henrikbjorn> Seldaek: go do java :) | |
| Dec 16 11:16:38 <lsmith> twig tags/filters/default template parameters: http://bit.ly/e645fJ | |
| Dec 16 11:16:41 <Felicitus> jmikola|w: yeah I need to dig into that in more detail. right now I simply login my users with a custom method and inject the final user ID to the session, which I use to re-initialize everything. drawback is that I completely bypass all ACL stuff | |
| Dec 16 11:16:50 <Seldaek> henrikbjorn: I'm porting java shit to php at the moment (jackrabbit/jcr/lala..) | |
| Dec 16 11:16:54 <lsmith> so a few things here | |
| Dec 16 11:17:11 <lsmith> fabien has stated that twig templates should not access _view .. and therefore not access the php template helpers | |
| Dec 16 11:17:51 <lsmith> Seldaek: can you tell us a bit about https://github.com/fabpot/symfony/pull/29 ? | |
| Dec 16 11:17:52 * jgintlio (~jgrier@72.242.184.210) has joined #symfony-dev | |
| Dec 16 11:17:59 <Seldaek> yes | |
| Dec 16 11:18:23 <lsmith> i should also add that currently there arent twig filters/tags for all use cases | |
| Dec 16 11:18:23 * lacyrhoades (~snakeoil@72.242.184.210) has joined #symfony-dev | |
| Dec 16 11:18:29 <Seldaek> so shortly, that bundle just allows any service that is a templating helper to easily support twig tags | |
| Dec 16 11:18:34 <lsmith> like negating if a user has a role .. or just getting the user etc | |
| Dec 16 11:18:43 <Seldaek> err, s/bundle/patch/ (tired) | |
| Dec 16 11:19:13 <Seldaek> the current state would force everyone to basically implement a templating helper + implement a twig extension to register the custom token parsers | |
| Dec 16 11:19:34 <Seldaek> but with the patch, you just implement an interface in your templating helper, hwich is a method that returns a bunch of custom token parsers | |
| Dec 16 11:19:50 <Seldaek> for those that don't know, custom token parsers are the way to define new twig tags ( {% foo %} ) | |
| Dec 16 11:19:57 <henrikbjorn> -1 it forces people to have twig bundle specifics in their app even though they dont use it and couples the general templating engine together with twig | |
| Dec 16 11:20:07 <Seldaek> henrikbjorn: come on stop with that | |
| Dec 16 11:20:14 <Seldaek> I already answered it can be fixed in the pull | |
| Dec 16 11:20:30 <Seldaek> so, the patch helps with *supporting twig*, but it doesn't solve all problems imo | |
| Dec 16 11:20:58 <lsmith> fabien indicated that he prefers to handle some things by making it easier to inject data into the templates | |
| Dec 16 11:21:06 <lsmith> specifically for example an instance of the user | |
| Dec 16 11:21:06 <Seldaek> one big issue is that the way tags are implemented in twig atm, you can not make a tag that just returns data for example | |
| Dec 16 11:21:12 <lsmith> here is a ticket on that http://trac.symfony-project.org/ticket/9349 | |
| Dec 16 11:21:14 <ornicar> +1 making a Twig extension class for each helper is a pain | |
| Dec 16 11:21:33 <henrikbjorn> ornicar: but you only need one extension class for all helpers? | |
| Dec 16 11:21:36 <lsmith> it should be noted that the view layer proposal already includes a complete and working implementation for this | |
| Dec 16 11:21:52 <lsmith> here is an example how this works for static and dynamic template parameter injection http://pastie.org/1352992 | |
| Dec 16 11:21:54 <ornicar> henrikbjorn: well at least one per bundle! | |
| Dec 16 11:22:03 <henrikbjorn> lsmith: should still be possible without the view layer and injecting the container into the view layer | |
| Dec 16 11:22:24 <lsmith> henrikbjorn: yes .. it should be possible to move this functionality to the template service | |
| Dec 16 11:22:36 <Seldaek> so, for example the issue of grabbing ALL flash messages in a template, to loop over them, is impossible to fix with tags, unless you implement a custom for loop that would automatically loop over flash messages | |
| Dec 16 11:22:44 <henrikbjorn> what about makeing a template.default_variable and add defaultParameters to the engine and merge thoose with the one passed in the renderResponse? | |
| Dec 16 11:23:52 <Seldaek> so yeah, either we inject data, and hope that fixes all use cases, or we need to figure out a way to have kind of inline tags, so you could do {{ %inlineTag%|somefilter }} or {% for var in %inlineTag% %} | |
| Dec 16 11:24:12 <Seldaek> those wouldn't be custom parsed, more like a filter that you just call by doing %filter% | |
| Dec 16 11:24:14 <lsmith> it should be noted that injection means that you need to do work .. even if you are never going to use it | |
| Dec 16 11:24:31 <Seldaek> because the only alternative atm is to do {{ null|inlineTag }} | |
| Dec 16 11:24:31 <lsmith> aka .. you are pushing stuff into the template .. rather than the template pulling it on a need to use basis | |
| Dec 16 11:24:40 <Seldaek> which calls the filter with null, and then the filter can return stuff, but that's just ugly syntax | |
| Dec 16 11:25:26 <Seldaek> I think I'll mail the twig-dev ml about the above proposal, if anyone gives a crap you should follow it | |
| Dec 16 11:25:27 <lsmith> Seldaek: does pull 29 work with current fabpot/master so that people can try it out? | |
| Dec 16 11:25:32 <Seldaek> lsmith: yes | |
| Dec 16 11:25:35 <Seldaek> I use it in my project | |
| Dec 16 11:25:40 <lsmith> ok | |
| Dec 16 11:25:43 <Seldaek> so it's never lagging too much behind | |
| Dec 16 11:26:13 <lsmith> jmikola|w: are you guys using twig yet at opensky? | |
| Dec 16 11:26:14 <Seldaek> but as I tried to explain (and apparently put everyone to sleep) it does not solve all issues imo | |
| Dec 16 11:26:23 <Seldaek> just eases the barrier of entry for twig tags a bit | |
| Dec 16 11:26:30 <jmikola|w> lsmith: yes | |
| Dec 16 11:26:35 <jmikola|w> exclusively | |
| Dec 16 11:26:42 <lsmith> so do you write lots of custom twig tags/filters? | |
| Dec 16 11:26:45 <jmikola|w> we use _view.whatever | |
| Dec 16 11:26:51 <jmikola|w> which fabpot doesn't like :) | |
| Dec 16 11:26:56 <ornicar> we do to at exercise.com :-/ | |
| Dec 16 11:27:02 <ornicar> s/to/too | |
| Dec 16 11:27:18 <jmikola|w> i honestly didn't see anything wrong with it, and that's why i was so surprised that {% ifrole %} was actually suggested as an alternative | |
| Dec 16 11:27:21 <lsmith> we do at liip.ch as well | |
| Dec 16 11:27:27 <ornicar> it's just too much work the provide all twig tokens | |
| Dec 16 11:27:31 <jmikola|w> knowing that if i was using php templates, i'd certainly use $view | |
| Dec 16 11:27:54 <lsmith> ok .. so i guess thats the take away at this point | |
| Dec 16 11:28:04 <jmikola|w> i just hope _view isn't removed anytime soon | |
| Dec 16 11:28:07 <Seldaek> ornicar: with pull 26 it becomes kind of a one liner to support a new token to call a custom method, so it's acceptable work | |
| Dec 16 11:28:12 <lsmith> unless something drastically improves .. the _view stuff is going to be used | |
| Dec 16 11:28:17 * Garfield-fr has quit (Quit: ⏏) | |
| Dec 16 11:28:25 <ornicar> Seldaek: that's why I like it :) | |
| Dec 16 11:28:31 * Felicitus has quit (Quit: Lost terminal) | |
| Dec 16 11:28:50 <Seldaek> aye, I'll go write my mail to twig-dev and we'll see what fabien thinks | |
| Dec 16 11:29:03 <lsmith> so it would be great of excerise.com/opensky could play with pull 29 | |
| Dec 16 11:29:09 <lsmith> ok cool .. so moving on? | |
| Dec 16 11:29:17 <Seldaek> sure | |
| Dec 16 11:29:32 <lsmith> RFC: Merging field groups into a form: http://bit.ly/fFJwBK | |
| Dec 16 11:29:35 <lsmith> bernhard isnt here | |
| Dec 16 11:29:42 <lsmith> and i have actually not read the RFC myself yet | |
| Dec 16 11:29:44 <jmikola|w> i can speak a bit about that | |
| Dec 16 11:29:52 <lsmith> ok great .. go! :) | |
| Dec 16 11:29:54 <Seldaek> I don't know if it makes sense to discuss it here | |
| Dec 16 11:30:15 <Seldaek> as long as there is no deadlock on the ml.. | |
| Dec 16 11:30:17 <jmikola|w> so, for those of us that have worked with forms, you might have noticed that ever field is a 1:1 correspondence with an object property | |
| Dec 16 11:30:33 <lsmith> Seldaek: its good to have people have a chance to get upto speed | |
| Dec 16 11:30:39 <jmikola|w> and as you nest fieldgroups in a form, those fieldgroup keys must correspond to objects within your main object | |
| Dec 16 11:31:16 <jmikola|w> bernhard's proposal, and something i like a lot of us are looking forward to, is a way to create fieldgroups that don't require sub-objects in your main, bound object | |
| Dec 16 11:31:30 <jmikola|w> so he presented DateRangeField, which aggregates start/endDate fields | |
| Dec 16 11:32:08 <jmikola|w> and if I had a PartyForm, and a Party object... which had direct start/endDate properties... I could use this DateRangeField (actually a FieldGroup) without having to change my domain model to accommodate the form structure | |
| Dec 16 11:32:26 <jmikola|w> the complexity is how this will relate to binding form data to the object | |
| Dec 16 11:32:32 <jmikola|w> currently, we depend on property paths | |
| Dec 16 11:32:53 <jmikola|w> in a traditional field group, before this RFC, it would be mainObject.dateRange.startDate and the same for endDate | |
| Dec 16 11:33:19 <jmikola|w> one suggestion is ignoring this special type of meta/wrapping fieldGroup in the property path, so just mainObject.startDate | |
| Dec 16 11:33:29 <jmikola|w> i raised a question of how this would relate to constraints and validation errors | |
| Dec 16 11:34:19 <jmikola|w> as we depend on property paths for mapping errors... without a property path, i'm not sure how a dateRangeField could ever have it's own error itself (not an error that was specific to just one sub-field) | |
| Dec 16 11:34:27 <jmikola|w> but that's all discussed on the mailing list thread, and i await input on that :) | |
| Dec 16 11:34:32 <jmikola|w> i think that covers the gist of things | |
| Dec 16 11:34:38 <jmikola|w> anyone have input/comments? | |
| Dec 16 11:34:43 * jmikola|w taps mic | |
| Dec 16 11:35:13 * ornicar tries to understand the RFC | |
| Dec 16 11:35:26 <Seldaek> well, I already said what I was thinking on the ml, didn't read up emails today unfortunately so I'm not up to speed | |
| Dec 16 11:35:48 <jmikola|w> i mentioned the property path / error-mapping concern in my very first reply, but i think it was glossed over :) | |
| Dec 16 11:36:06 <jmikola|w> perhaps rightfully so, bernhard might not see a need for these fieldgroups to get errors themselves | |
| Dec 16 11:36:52 <jmikola|w> i think everyone can agree this is incredibly useful though, as a feature in general - having a fieldgroup that can function transparently | |
| Dec 16 11:37:01 <jmikola|w> the DateRangeField and GmapField examples were very nice | |
| Dec 16 11:37:48 <Seldaek> yes, I think it'd be nice if you could somehow aggregate the errors of the sub-fields + the group all on the group | |
| Dec 16 11:38:17 <Seldaek> meaning you need to be able to attach validators to the group, that can only work on the group and require a bigger picture than just one field | |
| Dec 16 11:38:35 <Seldaek> but that's maybe something for the next step | |
| Dec 16 11:39:18 <lsmith> ok .. anything further on this topic? | |
| Dec 16 11:39:47 <lsmith> otherwise moving on .. | |
| Dec 16 11:39:55 <jmikola|w> moving one | |
| Dec 16 11:39:56 <jmikola|w> *on | |
| Dec 16 11:40:00 <lsmith> Extension helpers: http://bit.ly/fgFbXm | |
| Dec 16 11:40:13 <ornicar> thanks jimikola|w for raising the form point! | |
| Dec 16 11:40:16 <lsmith> so the idea here is to make it easier to handle Bundle configurations | |
| Dec 16 11:40:37 <lsmith> this is used in DoctrineUserBundle which has a ton of options | |
| Dec 16 11:40:44 <lsmith> custom classes etc | |
| Dec 16 11:40:58 <lsmith> the pull is here https://github.com/fabpot/symfony/pull/133 | |
| Dec 16 11:41:19 <lsmith> there was a +1 and a -1 camp .. | |
| Dec 16 11:41:24 <lsmith> and there it ended | |
| Dec 16 11:41:38 <lsmith> https://github.com/knplabs/DoctrineUserBundle/blob/master/DependencyInjection/DoctrineUserExtension.php | |
| Dec 16 11:42:06 <lsmith> this code handles all the parameters you can see at the bottom of the README https://github.com/knplabs/DoctrineUserBundle/ | |
| Dec 16 11:42:25 <lsmith> any opinions on this? | |
| Dec 16 11:42:55 <lsmith> .. | |
| Dec 16 11:43:05 <lsmith> is it clear what its trying to do? | |
| Dec 16 11:43:15 <lsmith> .. | |
| Dec 16 11:43:34 <jmikola|w> reading pull | |
| Dec 16 11:44:01 <jmikola|w> would this just be helper methods to use in an *Extension.php file? | |
| Dec 16 11:44:06 <Seldaek> yes | |
| Dec 16 11:44:07 <lsmith> yes | |
| Dec 16 11:44:14 <jmikola|w> great idea :) | |
| Dec 16 11:44:18 <Seldaek> use OPTIONALLY, I might add | |
| Dec 16 11:44:23 <Seldaek> in case its not clear:p | |
| Dec 16 11:44:35 <Seldaek> but somehow the -1 camp doesn't like it :p | |
| Dec 16 11:44:44 <jmikola|w> if it's not approved for sf2, you could always publish a standalone class that we could extend, and your helper class extends the base sf one | |
| Dec 16 11:45:09 <lsmith> jmikola|w: yeah .. | |
| Dec 16 11:45:09 <Seldaek> yeah.. but that's like telling that the view can be implemented in application code | |
| Dec 16 11:45:19 <Seldaek> sure it can, but then nobody uses it, and nobody benefits from it, so who cares | |
| Dec 16 11:45:21 <jmikola|w> i don't like how your configLoad() starts off though | |
| Dec 16 11:45:29 <lsmith> like DoctrineUserBundle currently has these methods inside the Extension class | |
| Dec 16 11:45:40 <jmikola|w> typically, you only reload the xml if a given definition within it isn't already existing - no reason to load the same thing twice | |
| Dec 16 11:46:04 <Seldaek> jmikola|w: that's however another issue with DUB, nothing to do with the pull :) | |
| Dec 16 11:46:16 <Seldaek> but lsmith/ornicar should take note I guess | |
| Dec 16 11:46:36 <jmikola|w> so reading the DUB extension, i maybe wouldn't use these specific protected methods | |
| Dec 16 11:46:45 <jmikola|w> but there are certain conventions that i think are useful to have | |
| Dec 16 11:47:00 <jmikola|w> as i notice a lot of duplicated array_key_exists and isset() nonsense in my own extensions | |
| Dec 16 11:47:25 * ce_afk is now known as cescalante | |
| Dec 16 11:47:25 <jmikola|w> having re-usable, well-tested methods to avoid repeating myself would help make the code more concise | |
| Dec 16 11:47:43 <lsmith> ok .. anyone else have something on this? | |
| Dec 16 11:47:57 <Seldaek> yeah obviously that was a first batch, if it got merged and used, I'm sure we'd get feedback for further improvement | |
| Dec 16 11:47:59 <jmikola|w> your remap functions are essentially mapping the config array to the arrays you define in configLoad, right? | |
| Dec 16 11:48:08 <Seldaek> yup | |
| Dec 16 11:48:11 <jmikola|w> by walking it and checking what's available | |
| Dec 16 11:48:34 <lsmith> of course you could also handle some configs like this .. and others "manually" | |
| Dec 16 11:48:40 <jmikola|w> i will say one thing i initially took for granted was that configs get merged, and then i realized that it's only if *load() is smart enough to handle it | |
| Dec 16 11:49:15 <Seldaek> you mean if it's in config.yml & config_dev.yml or what ? | |
| Dec 16 11:49:28 <jmikola|w> yeah... like simplecas.config: { ...} in two different yml files | |
| Dec 16 11:49:44 <jmikola|w> unless you walk the config array and import what's present, it's very easy to shoot yourself in the foot | |
| Dec 16 11:49:58 <Seldaek> I see, never realized that either | |
| Dec 16 11:50:13 <lsmith> yeah .. you have to use parameters in those cases | |
| Dec 16 11:50:15 <jmikola|w> granted, there are cases where you sometimes want to overwrite an entire block within the config array, but it's something to be aware of | |
| Dec 16 11:50:33 <lsmith> yeah .. just clobbering things together isnt ideal either | |
| Dec 16 11:50:37 <jmikola|w> i kind of just assumed that the config*yml files all merge themselves and then call config.load once :) | |
| Dec 16 11:50:40 <lsmith> so you have to make it explicit | |
| Dec 16 11:50:42 <jmikola|w> i was young and naive | |
| Dec 16 11:50:46 <lsmith> hehe | |
| Dec 16 11:50:49 <lsmith> ok moving on | |
| Dec 16 11:50:49 <lsmith> flash api and Message class: http://bit.ly/hkUwiB | |
| Dec 16 11:51:08 <lsmith> a couple of things here | |
| Dec 16 11:51:08 <lsmith> https://github.com/fabpot/symfony/pull/198 | |
| Dec 16 11:51:19 <lsmith> this has been partially merged and tweaks the internal flash api a bit | |
| Dec 16 11:51:38 <lsmith> i think the only thing that wasnt merged yet was a method to remove flash messages | |
| Dec 16 11:51:57 <lsmith> which i think is useful for example when a 3rd party controller is a big trigger happy with flash messages | |
| Dec 16 11:52:15 <lsmith> and it seems like a safe addition without any real drawbacks | |
| Dec 16 11:52:36 <lsmith> the bigger topic is that fabien hasnt implemented the Message class he promised a while back | |
| Dec 16 11:52:36 <lsmith> http://trac.symfony-project.org/ticket/9346 | |
| Dec 16 11:52:46 <lsmith> "I have tried to work on such a class, but this is just not possible as we need to many parameters to make it usable. Besides the obvious id, parameters, we also need to support the domain, selector, and locale." | |
| Dec 16 11:52:47 <jmikola|w> with i18n support? | |
| Dec 16 11:52:56 <lsmith> yeah | |
| Dec 16 11:53:28 <lsmith> though i actually didnt understand the issue .. i was under the impression the Message class would just use the translation layer if enabled | |
| Dec 16 11:53:49 <lsmith> so to me all the Message class needs to hold is the message | |
| Dec 16 11:53:59 <lsmith> and optionally parameters, a type and the catalog | |
| Dec 16 11:54:03 <henrikbjorn> setFlash('key', 'message') and use |trans in the view? | |
| Dec 16 11:55:02 <henrikbjorn> imo DUB are using flashes wrong | |
| Dec 16 11:55:05 <Seldaek> it's a bit nicer if the message is just translating automatically on __toString() | |
| Dec 16 11:55:08 <lsmith> henrikbjorn: yes .. or setFlash('key', new Message('message', array(..), Message::NOTICE, 'mycatalog')) | |
| Dec 16 11:55:14 <Seldaek> then it's transparent to the templates | |
| Dec 16 11:55:17 <Seldaek> they just output flash messages | |
| Dec 16 11:55:23 <Seldaek> and don't have to know what it was or where it came from | |
| Dec 16 11:55:38 <lsmith> Seldaek: right .. i was expecting the Message() to use the translation layer to do this when __toString() is called | |
| Dec 16 11:55:57 <Seldaek> means you need to inject the translation layer though :) | |
| Dec 16 11:56:17 <Seldaek> so maybe $this->container->get('message') instead of new Message.. | |
| Dec 16 11:56:23 <lsmith> Seldaek: right .. i guess this means that when unserializing the flash messages | |
| Dec 16 11:56:30 <lsmith> it would need to inject the translation layer | |
| Dec 16 11:56:39 <Seldaek> ah right they are serialized | |
| Dec 16 11:57:03 <Seldaek> setFlash() could do the check as well, if it implements ContainerAware.. | |
| Dec 16 11:57:40 <lsmith> you mean that setFlash() would do the translation on the spot | |
| Dec 16 11:57:48 <lsmith> that doesnt work | |
| Dec 16 11:57:59 <lsmith> since then the additional information will have no place | |
| Dec 16 11:59:05 <Seldaek> no | |
| Dec 16 11:59:14 <Seldaek> I mean setFlash would inject the container as well | |
| Dec 16 11:59:19 <Seldaek> in case you display them on the same page | |
| Dec 16 11:59:31 <Seldaek> they are not *always* serialized and unserialized | |
| Dec 16 11:59:35 <Seldaek> is all I'm saying | |
| Dec 16 12:00:21 <mvrhov> Seldaek, how many cases did you have when you needed to display flash message rightaway? | |
| Dec 16 12:00:34 <Seldaek> or are they really just available on the next request? never used them much in Sf2.. just talking with assumptions about my own implementation from our okapi2 project | |
| Dec 16 12:00:43 <mvrhov> because I've really had just a few | |
| Dec 16 12:01:02 <Seldaek> mvrhov: well, it's a non-issue anywayt o inject the container in setFlash, let's not focus on this detail :) | |
| Dec 16 12:01:03 * rafix (~rafix@200.14.49.3) has joined #symfony-dev | |
| Dec 16 12:01:28 <johanness> maybe we should introduce a messaging service on top of setFlash, getFlash | |
| Dec 16 12:02:00 * anatoo has quit (Remote host closed the connection) | |
| Dec 16 12:02:01 <henrikbjorn> but when is it a problem? | |
| Dec 16 12:02:05 <lsmith> sorry .. work | |
| Dec 16 12:02:18 <henrikbjorn> in 99% of cases you would have notice, success, error keys with a static message associated | |
| Dec 16 12:02:37 <henrikbjorn> that you can translate when outputting the flash message itself | |
| Dec 16 12:03:24 <Seldaek> that's "ok", but not fully flexible | |
| Dec 16 12:03:35 <Seldaek> so I don't see a reason to lock ourselves into this way of doing things | |
| Dec 16 12:03:53 <johanness> putting too much logic inside getFlash/setFlash just seems bad | |
| Dec 16 12:03:59 <henrikbjorn> there is also no reason to add it if nobody have a specific use case yet | |
| Dec 16 12:04:19 <henrikbjorn> else you can get the translate in you controller and use its trans method before setting the flash | |
| Dec 16 12:04:28 * anatoo (~anatoo@www1565.sakura.ne.jp) has joined #symfony-dev | |
| Dec 16 12:04:39 <Seldaek> well, lsmith can may be enlighten you there, it's mostly his war about flash messages, I don't use them that much | |
| Dec 16 12:05:33 <mvrhov> afair the messages that go into flash are not messages per se but keys + params to fetch them from elsewhere | |
| Dec 16 12:05:35 <henrikbjorn> i think the problem is that DUB are using them wrong | |
| Dec 16 12:06:01 <lsmith> re | |
| Dec 16 12:06:28 <lsmith> well its just that right now the messages are key+value only | |
| Dec 16 12:06:39 <lsmith> which means you dont know if its an error or notice etc | |
| Dec 16 12:06:46 <lsmith> unless you encode that into the key or value | |
| Dec 16 12:06:49 <henrikbjorn> that is what the keys are for | |
| Dec 16 12:06:56 <mvrhov> I think it would be better to put full message into the flash | |
| Dec 16 12:07:10 <lsmith> well they would then override | |
| Dec 16 12:07:12 <henrikbjorn> maybe the key part should be called type | |
| Dec 16 12:07:15 <lsmith> if you have multiple notices | |
| Dec 16 12:07:18 <mvrhov> in that case you could put already translated message inthere | |
| Dec 16 12:07:35 <lsmith> but you need to know the type in the template to decide how to style it | |
| Dec 16 12:07:40 <henrikbjorn> i dont see how you would require more than one type of each in a single request? | |
| Dec 16 12:07:40 <mvrhov> setflash would then need only two params | |
| Dec 16 12:07:41 * dennisj (~chatzilla@dslb-188-109-240-054.pools.arcor-ip.net) has joined #symfony-dev | |
| Dec 16 12:07:47 <mvrhov> $message, $messageType | |
| Dec 16 12:07:49 <johanness> lsmith: have you considered implementing a custom service on top of setFlash, getFlash which does what you want? | |
| Dec 16 12:08:00 <henrikbjorn> mvrhov: it have a $type, $message params now | |
| Dec 16 12:08:11 <lsmith> henrikbjorn: i think its quite realistic, though not usual, for multple notices to be shown | |
| Dec 16 12:08:26 <henrikbjorn> i have never seen a site with more than one in a single request :) | |
| Dec 16 12:08:34 <lsmith> johanness: well the Message class was supposed to be this thing into which i can just add additional data | |
| Dec 16 12:08:56 <Seldaek> henrikbjorn: I think stackoverflow can spam you with multiple top bars at the beginning if you get multiple achievements in one request :) | |
| Dec 16 12:08:57 <johanness> if it depends on a translation service then a service seems like a good choice too me | |
| Dec 16 12:09:02 <mvrhov> henrikbjorn, I do have a such implementation right now, well the project is still in ZF but.. | |
| Dec 16 12:09:05 <lsmith> anyway .. time is up .. and i actually have to run | |
| Dec 16 12:09:12 <johanness> this service could have the session as a dependency and then do its magic | |
| Dec 16 12:09:15 <lsmith> laters |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment