[Phpmyadmin-devel] Refactoring: Displaying query results

Sun May 13 12:34:17 CEST 2012

On Fri, May 11, 2012 at 6:17 PM, Chanaka Dharmarathna <
pe.chanaka.ck at gmail.com> wrote:

>
>
> On Fri, May 11, 2012 at 4:18 PM, Rouslan Placella <rouslan at placella.com>wrote:
>
>> On 11/05/12 08:26, Chanaka Dharmarathna wrote:
>> > On Fri, May 11, 2012 at 11:12 AM, Chanaka Dharmarathna<
>> > pe.chanaka.ck at gmail.com>  wrote:
>> >
>> >>
>> >>
>> >> On Fri, May 11, 2012 at 7:05 AM, Chanaka Dharmarathna<
>> >> pe.chanaka.ck at gmail.com>  wrote:
>> >>
>> >>>
>> >>>
>> >>> On Fri, May 11, 2012 at 12:39 AM, Rouslan Placella<
>> rouslan at placella.com>wrote:
>> >>>
>> >>>> On 09/05/12 21:08, Chanaka Dharmarathna wrote:
>> >>>>> On Wed, May 9, 2012 at 2:27 PM, Rouslan Placella<
>> rouslan at placella.com
>> >>>>> wrote:
>> >>>>>
>> >>>>>> On 09/05/12 09:39, Chanaka Dharmarathna wrote:
>> >>>>>>> On Wed, May 9, 2012 at 9:21 AM, Chanaka Dharmarathna<
>> >>>>>>> pe.chanaka.ck at gmail.com>    wrote:
>> >>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> On Wed, May 9, 2012 at 1:19 AM, Rouslan Placella<
>> >>>> rouslan at placella.com
>> >>>>>>> wrote:
>> >>>>>>>>
>> >>>>>>>>> On 08/05/12 20:32, Chanaka Dharmarathna wrote:
>> >>>>>>>>>> On Tue, May 8, 2012 at 7:23 PM, Chanaka Dharmarathna<
>> >>>>>>>>>> pe.chanaka.ck at gmail.com>     wrote:
>> >>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>> On Tue, May 8, 2012 at 7:08 PM, Marc Delisle<
>> marc at infomarc.info>
>> >>>>>>>>>     wrote:
>> >>>>>>>>>>>
>> >>>>>>>>>>>> Le 2012-05-08 09:25, Chanaka Dharmarathna a écrit :
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> See "Class Variables and Methods" in [0].
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> [0] http://pear.php.net/manual/en/standards.naming.php
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>> Hi Marc,
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> I went through that document.
>> >>>>>>>>>>>>> If the function is global, it will be like
>> >>>>>>>>> 'PMA_getDivForSliderEffect()'
>> >>>>>>>>>>>>> And if it is a local function (for particular class), it
>> will
>> >>>> be
>> >>>>>> like
>> >>>>>>>>>>>>> 'getDivForSliderEffect()'.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> If this is a public method, yes. If a private one, prefix its
>> >>>> name
>> >>>>>>>>> with
>> >>>>>>>>>>>> an underscore.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Though I went through that document before this, I couldn't
>> get
>> >>>>>> that
>> >>>>>>>>>>>> point.
>> >>>>>>>>>>>>> Thanks for pointing out that.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> --
>> >>>>>>>>>>>> Marc Delisle
>> >>>>>>>>>>>> http://infomarc.info
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>
>> >>>>
>> ------------------------------------------------------------------------------
>> >>>>>>>>>>>> Live Security Virtual Conference
>> >>>>>>>>>>>> Exclusive live event will cover all the ways today's security
>> >>>> and
>> >>>>>>>>>>>> threat landscape has changed and how IT managers can respond.
>> >>>>>>>>> Discussions
>> >>>>>>>>>>>> will include endpoint security, mobile security and the
>> latest
>> >>>> in
>> >>>>>>>>> malware
>> >>>>>>>>>>>> threats.
>> >>>> http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> >>>>>>>>>>>> _______________________________________________
>> >>>>>>>>>>>> Phpmyadmin-devel mailing list
>> >>>>>>>>>>>> Phpmyadmin-devel at lists.sourceforge.net
>> >>>>>>>>>>>>
>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>> Got it Marc.
>> >>>>>>>>>>> Thanks.
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>> --
>> >>>>>>>>>>> ____________________________________
>> >>>>>>>>>>>
>> >>>>>>>>>>> Chanaka Indrajith
>> >>>>>>>>>>> Bsc.Computer Engineering Undergraduate
>> >>>>>>>>>>> Faculty of Engineering
>> >>>>>>>>>>> University of Peradeniya
>> >>>>>>>>>>> Sri Lanka
>> >>>>>>>>>>> ____________________________________
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>> Hi Michal,
>> >>>>>>>>>>
>> >>>>>>>>>> I just did my first commit to my repository [0].
>> >>>>>>>>>> In that commit I have done some refactoring on
>> >>>>>>>>> "PMA_displayTableHeaders()"
>> >>>>>>>>>> function in "display_tbl.php" file.
>> >>>>>>>>>> All HTML renderings by "echo"s were removed and those HTML was
>> >>>>>> rendered
>> >>>>>>>>> at
>> >>>>>>>>>> the bottom of the function at once.
>> >>>>>>>>>> As well some related functions in "common.lib.php" file also
>> were
>> >>>>>>>>> modified
>> >>>>>>>>>> in order to return a string of HTML content.
>> >>>>>>>>>> But the tests were not yet modified according to my
>> modifications.
>> >>>>>>>>>> Since there was a conflict after my first local commit [1] (git
>> >>>>>>>>> commit), I
>> >>>>>>>>>> had to resolve conflicts and commit again. (that is [0])
>> >>>>>>>>>>
>> >>>>>>>>>> I have already installed phpUnit framework.
>> >>>>>>>>>> It will be very helpful if you can suggest me some
>> documentation
>> >>>> to
>> >>>>>>>>> setup
>> >>>>>>>>>> and run tests in PMA (selenium etc).
>> >>>>>>>>>>
>> >>>>>>>>>> I'm intending to do the above refactoring procedure to all the
>> >>>>>>>>> functions in
>> >>>>>>>>>> "display_tbl.lib.php" file before moving to any other task.
>> >>>>>>>>>>
>> >>>>>>>>>> [0] :
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>
>> >>>>
>> https://github.com/Chanaka/phpmyadmin/commit/dfc73d3494445ff9430e5d0b4f33f800c6e480e0
>> >>>>>>>>>> [1] :
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>
>> >>>>
>> https://github.com/Chanaka/phpmyadmin/commit/004f4f5258a0820975e2da9fa02a6d3ba6622368
>> >>>>>>>>>>
>> >>>>>>>>>> Regards !
>> >>>>>>>>>
>> >>>>>>>>> I had a very quick look at your commit. I think that you're
>> >>>> indenting
>> >>>>>>>>> some lines incorrectly (but I stand to be corrected on this
>> one).
>> >>>>>> AFAIK,
>> >>>>>>>>> you don't need to line up the operators, just indent by 4
>> spaces.
>> >>>>>> Example:
>> >>>>>>>>>
>> >>>>>>>>> // THIS IS BAD
>> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">'
>> >>>>>>>>>                . $choice_label
>> >>>>>>>>>                . '</label>';
>> >>>>>>>>>
>> >>>>>>>>> // THIS IS GOOD
>> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">'
>> >>>>>>>>>        . $choice_label
>> >>>>>>>>>        . '</label>';
>> >>>>>>>>>
>> >>>>>>>>> // THIS IS ALSO GOOD
>> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">';
>> >>>>>>>>> $radio_html .= $choice_label;
>> >>>>>>>>> $radio_html .= '</label>';
>> >>>>>>>>>
>> >>>>>>>>> Bye,
>> >>>>>>>>> Rouslan
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>
>> >>>>
>> ------------------------------------------------------------------------------
>> >>>>>>>>> Live Security Virtual Conference
>> >>>>>>>>> Exclusive live event will cover all the ways today's security
>> and
>> >>>>>>>>> threat landscape has changed and how IT managers can respond.
>> >>>>>> Discussions
>> >>>>>>>>> will include endpoint security, mobile security and the latest
>> in
>> >>>>>> malware
>> >>>>>>>>> threats.
>> http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> >>>>>>>>> _______________________________________________
>> >>>>>>>>> Phpmyadmin-devel mailing list
>> >>>>>>>>> Phpmyadmin-devel at lists.sourceforge.net
>> >>>>>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Hi Rouslan,
>> >>>>>>>>
>> >>>>>>>> Thanks for your suggestion.
>> >>>>>>>> I'll follow the way you mentioned as "THIS IS GOOD".
>> >>>>>>>>
>> >>>>>>>> Regards !
>> >>>>>>>>
>> >>>>>>>> --
>> >>>>>>>> ____________________________________
>> >>>>>>>>
>> >>>>>>>> Chanaka Indrajith
>> >>>>>>>> Bsc.Computer Engineering Undergraduate
>> >>>>>>>> Faculty of Engineering
>> >>>>>>>> University of Peradeniya
>> >>>>>>>> Sri Lanka
>> >>>>>>>> ____________________________________
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>> Hi Michal,
>> >>>>>>>
>> >>>>>>> In some functions there are pure HTML code snippets. (Ex :-
>> >>>>>>> PMA_displayTableNavigationOneButton() function in
>> >>>> display_tbl.lib.php)
>> >>>>>>>
>> >>>>>>> As I feel, its not good to convert all these snippets as PHP
>> string
>> >>>> and
>> >>>>>>> render them. So I'm intending to get those pure HTML snippets into
>> >>>>>> separate
>> >>>>>>> function. That function can be named as
>> >>>> 'PMA_renderTableNavigationForm'
>> >>>>>>> (for above case).
>> >>>>>>> What do you think about this ?
>> >>>>>>>
>> >>>>>>> As well is there any reason for the phrase 'One' in the function
>> >>>>>>> "PMA_displayTableNavigation*One*Button()" ?
>> >>>>>>>
>> >>>>>>> Regards !
>> >>>>>>
>> >>>>>> I think that the above example is *exactly* the kind of stuff that
>> >>>> needs
>> >>>>>> to be converted to strings.
>> >>>>>>
>> >>>>>> Bye,
>> >>>>>> Rouslan
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>
>> ------------------------------------------------------------------------------
>> >>>>>> Live Security Virtual Conference
>> >>>>>> Exclusive live event will cover all the ways today's security and
>> >>>>>> threat landscape has changed and how IT managers can respond.
>> >>>> Discussions
>> >>>>>> will include endpoint security, mobile security and the latest in
>> >>>> malware
>> >>>>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> >>>>>> _______________________________________________
>> >>>>>> Phpmyadmin-devel mailing list
>> >>>>>> Phpmyadmin-devel at lists.sourceforge.net
>> >>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>> >>>>>>
>> >>>>>
>> >>>>> Hi Rouslan,
>> >>>>>
>> >>>>> I have little doubt of the way I understand your point.
>> >>>>>
>> >>>>> You are suggesting to render HTML content by PHP string, instead of
>> >>>> using
>> >>>>> pure HTML, though there is nicely placed HTML code snippets. (around
>> >>>> 5, 6
>> >>>>> lines)
>> >>>>> Am I correct Rouslan ? Sorry if I'm repeat the wheel.
>> >>>>
>> >>>> Yes, that's correct. And I don't thing that those lines are nice at
>> all.
>> >>>>
>> >>>> Bye,
>> >>>> Rouslan
>> >>>>
>> >>>>
>> >>>>
>> ------------------------------------------------------------------------------
>> >>>> Live Security Virtual Conference
>> >>>> Exclusive live event will cover all the ways today's security and
>> >>>> threat landscape has changed and how IT managers can respond.
>> Discussions
>> >>>> will include endpoint security, mobile security and the latest in
>> malware
>> >>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> >>>> _______________________________________________
>> >>>> Phpmyadmin-devel mailing list
>> >>>> Phpmyadmin-devel at lists.sourceforge.net
>> >>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>> >>>>
>> >>>
>> >>> Hi Rouslan,
>> >>>
>> >>> I was already thinking of do the same thing you pointed.
>> >>> It will cause some sort of problems if I do the previous thing I
>> >>> suggested.
>> >>> Thanks for your help.
>> >>>
>> >>>
>> >>> Regards !
>> >>> --
>> >>> ____________________________________
>> >>>
>> >>> Chanaka Indrajith
>> >>> Bsc.Computer Engineering Undergraduate
>> >>> Faculty of Engineering
>> >>> University of Peradeniya
>> >>> Sri Lanka
>> >>> ____________________________________
>> >>>
>> >>>
>> >> Hi Michal,
>> >>
>> >> The function "PMA_displayTableBody()" in display_tbl.lib.php is always
>> >> return true.
>> >> But it does not seems like, useful for other place where it have been
>> >> used. (Used only in "PMA_displayTable()" function in same file)
>> >>
>> >> Inside "PMA_displayTable()" function it just assign the return value
>> >> (true) to variable called '$clause_is_unique'.
>> >> And variable is only used at one place. it is,
>> >>
>> >> echo '<input type="hidden" name="clause_is_unique"'
>> >>              .' value="' . *$clause_is_unique* . '" />' . "\n";
>> >>
>> >> So, if this always return true, why don't we hard code true in the
>> above
>> >> case.
>> >> And modify the "PMA_displayTableBody()" function to return string of
>> html
>> >> content.
>> >>
>> >> Am I going wrong ?
>> >>
>> >>
>> >> Regards !
>> >> --
>> >> ____________________________________
>> >>
>> >> Chanaka Indrajith
>> >> Bsc.Computer Engineering Undergraduate
>> >> Faculty of Engineering
>> >> University of Peradeniya
>> >> Sri Lanka
>> >> ____________________________________
>> >>
>> >>
>> > Hi Michal,
>> >
>> > I think I was wrong.
>> > Though the block comments of PMA_displayTableBody function, mentioned
>> that
>> > it always return true,
>> > when digging the code, and looking at the bottom comments, seems like
>> its
>> > very important.
>> >
>> > I'll modify the function to return an array, having two elements which
>> are
>> > string of HTML content and boolean ($clause_is_unique)
>> > The name will change from "PMA_displayTableBody" to
>> > "PMA_getTableBodyParams".
>> > I hope to see your suggestions.
>> >
>> > Regards !
>>
>> It looks to me like you could easily get a value for $clause_is_unique
>> outside of that function, so that you can leave its name and purpose as
>> is. And you'll be able return a single string from it.
>>
>> Bye,
>> Rouslan
>>
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> _______________________________________________
>> Phpmyadmin-devel mailing list
>> Phpmyadmin-devel at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>>
>
> Hi Rouslan,
>
> You talking about the function "PMA_getUniqueCondition" in "common.lib.php
> file" right.
> Exactly, I can directly get the value for $clause_is_unique from the
> output array of that function.
> I'll do the necessary modifications.
>
> Thanks for pointing out that.
>
> Regards !
> --
> ____________________________________
>
> Chanaka Indrajith
> Bsc.Computer Engineering Undergraduate
> Faculty of Engineering
> University of Peradeniya
> Sri Lanka
> ____________________________________
>
>
Hi Michal,

I set up the testing environment in my machine and now evolving with the
improvements of unit tests for some modified function.

When I run unit test for "PMA_showMessage" function in "common.lib.php"
file, it gives me some fatal errors. (Call to undefined function
PMA_warnMissingExtension() ...)
I used the command : $phpunit
test/libraries/common/PMA_showMessage_test_disabled.php

Do you have any idea on this ?

My testing environment :
PHPUnit 3.6.10
PEAR Version: 1.9.4
PHP Version: 5.3.2-1ubuntu4.9

Regards !
-- 
____________________________________

Chanaka Indrajith
Bsc.Computer Engineering Undergraduate
Faculty of Engineering
University of Peradeniya
Sri Lanka
____________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120513/722c65b4/attachment.html>