Re: cvs: pear /Structures_DataGrid DataGrid.php

On 3/1/06, Olivier Guilyardi <olivierg@php.net> wrote: > Hi > > Justin Patrin wrote: > > On 2/27/06, Olivier Guilyardi <olivierg@php.net> wrote: > >>You just need to drop some $renderer =& $datagrid->getRenderer() and do a > >>s/datagrid->renderer/renderer/ > > > > Of course I understand. :-| It makes much more sense to use getRenderer(). > > This renderer property should have never been made public... > > I don't know what to think about this anymore. I just don't want to break BC. > There is an easy fix for both PHP4 and PHP5, but that may waste some disk > access/memory for people who : > - do not use the HTMLTable renderer. > - and do not use the third parameter of the DataGrid constructor > > For example, people who do something like : $datagrid->fill($xls); > > > Yep, I hadn't looked. That's very dirty. I agree, this isn't a good > > way to do it. (It should be using $table->updateAttributes(), of > > course.) > > There is not such method. AFAICS it is only possible to set the <table> > attributes in the HTML_Table constructor. This could be considered as a > HTML_Table bug, but with the new SDG design, it is not. > There is, actually. HTML_Table extends HTML_Common. HTML_Common has this method, always has. > > > >>Now, how to set a table attribute ? Here we go... > >> > >>$table = new HTMLTable(array('border' => '1')); > >>$datagrid->fill($table); > >> > > Yep, sounds good. I had no idea what fill() was supposed to do, > > honestly. Sounds like you guys are doing a great job updating the API. > > SDG *is* beta, so it's still allowed to break BC for good reasons. > > Note that the fill() method is not a replacement for the old render() way.. It is > just an alternative for people who need some flexibility. > Ok. > >>Anyway, dont worry, we're not likely to remove this setTableAttribute() method. > >>It will just not appear anymore in the docs. > > > > I suggest removing the decorator methods and the ->renderer hack since > > these are deprecated due to the restructuring. For now you could leave > > them but throw a notice so that people...well, *notice* that these are > > deprecated. Then in the release after next you can just remove them. > > I don't agree. I think that SDG has a quite large user base, and we should not > heavily break BC. Some of these methods are handy, although the new fill() > method allows to make a better use of PEAR components and their existing methods. > > The only argument in favor of removing these decorator methods is to make the > HTMLTable driver cleaner and easier to read so that people can easily subclass > it. But we can arrange the code so that deprecated methods appear at the end, > with some smart notice. > Yep, as I said, it's your call. -- Justin Patrin