{"issue":{"created_at":"2011-09-22T02:53:45Z","state":"open","updated_at":"2011-12-02T19:39:43Z","comments":4,"user":{"gravatar_id":"681e6d4d4cf91760e5e1d8ac9d6d7fc4","id":121003,"url":"https://api.github.com/users/weaverryan","avatar_url":"https://secure.gravatar.com/avatar/681e6d4d4cf91760e5e1d8ac9d6d7fc4?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png","login":"weaverryan"},"body":"Hey Fabien!\r\n\r\nThis is up for discussion, and obviously the docs would need to be greatly updated and this breaks BC. Fortunately, I think we may be the only two people using this extension... for now. My hope is that we can make the extension even easier to use and more people will hop on.\r\n\r\nAnyways, this separates the interfaces into 2 usable interfaces (and a base interface). One allows you to use the extension just like before - by dealing directly with the Tag objects. The other, however, allows you to interact directly with a comma-separated tag string instead.\r\n\r\nI think the architecture with the if statements near the bottom of the `TagManager` to check for the interface are a bit messy, but quite functional - I don't really see the need to abstract further right now. Also, I realize that I've hardcoded the comma as being the tag string separator, with no way to override this. If you can think of a good solution, let me know. I'd rather not require another method (e.g. `getTagSeparator`) on the `TaggableStringInterface` if we don't have to.\r\n\r\nOh, also, `loadOrCreateTags` now returns an `ArrayCollection` instead of an array. This makes it easier to get a result directly from that method and set it on your resource, since the `addTags()` method on the manager is now gone.\r\n\r\nThanks!","assignee":null,"labels":[],"id":1707006,"pull_request":{"patch_url":"https://github.com/FabienPennequin/DoctrineExtensions-Taggable/pull/3.patch","html_url":"https://github.com/FabienPennequin/DoctrineExtensions-Taggable/pull/3","diff_url":"https://github.com/FabienPennequin/DoctrineExtensions-Taggable/pull/3.diff"},"number":3,"title":"Refactor to use tag strings or tag objects","closed_at":null,"html_url":"https://github.com/FabienPennequin/DoctrineExtensions-Taggable/issues/3","url":"https://api.github.com/repos/FabienPennequin/DoctrineExtensions-Taggable/issues/3","milestone":null},"action":"created","comment":{"created_at":"2011-12-02T19:39:43Z","updated_at":"2011-12-02T19:39:43Z","user":{"gravatar_id":"681e6d4d4cf91760e5e1d8ac9d6d7fc4","id":121003,"url":"https://api.github.com/users/weaverryan","avatar_url":"https://secure.gravatar.com/avatar/681e6d4d4cf91760e5e1d8ac9d6d7fc4?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-140.png","login":"weaverryan"},"body":"I'm obviously still +1 for this PR :). It certainly doesn't add anything, but it's much friendlier. It represents a layer that I had to build into my application before this existed.\r\n\r\nIf this is a no-go, we could also perhaps at least consider creating a custom form type inside the bundle, which would contain some transformers like @Problematic suggested.\r\n\r\nThanks!","id":2994726,"url":"https://api.github.com/repos/FabienPennequin/DoctrineExtensions-Taggable/issues/comments/2994726"}}