[Phpmyadmin-devel] 3 patch suggestions for master
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git): - Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php - Commit b161f43ec9089e0d2d5fb8a619056e6f9577a380 When reloading (flushing) privileges on the privileges tab you get a 5 second long success message. This commit omits the success message and removes the loading message as soon as the ajax request is done. This behavior seems more consistent with my patch in April that removes messages once the ajax request is completed. - Commit 99775c9bad552aac8b904996d673f1023821ab13 Removes the initials table on the privileges tab if there is 20 or less privileges to show. With 1-20 privileges you can easily see all host names and don't need any filtering by initials imo. I would even increase that limit to 30 or so.
Tyron Madlener a écrit :
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php
Tyron, I don't see a timepicker in jQuery UI's Datepicker. It only handles the date, not the time.
- Commit b161f43ec9089e0d2d5fb8a619056e6f9577a380 When reloading (flushing) privileges on the privileges tab you get a 5 second long success message. This commit omits the success message and removes the loading message as soon as the ajax request is done. This behavior seems more consistent with my patch in April that removes messages once the ajax request is completed.
Merged, thanks.
- Commit 99775c9bad552aac8b904996d673f1023821ab13 Removes the initials table on the privileges tab if there is 20 or less privileges to show. With 1-20 privileges you can easily see all host names and don't need any filtering by initials imo. I would even increase that limit to 30 or so.
Merged, thanks. P.S. For these commits you have Author: unknown <Tyron@.(none)> -- Marc Delisle http://infomarc.info
On Mon, May 30, 2011 at 6:47 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php
Tyron, I don't see a timepicker in jQuery UI's Datepicker. It only handles the date, not the time.
Oh right. But it is not being used. tab_change.js uses PMA_addDatepicker() from functions.js which uses $.datepicker(), not $.timepicker() from what I can see. And tbl_select.js interestingly doesn't initialize any date/timepicker widget at all. Shall I try and implement it instead?
- Commit b161f43ec9089e0d2d5fb8a619056e6f9577a380 When reloading (flushing) privileges on the privileges tab you get a 5 second long success message. This commit omits the success message and removes the loading message as soon as the ajax request is done. This behavior seems more consistent with my patch in April that removes messages once the ajax request is completed.
Merged, thanks.
- Commit 99775c9bad552aac8b904996d673f1023821ab13 Removes the initials table on the privileges tab if there is 20 or less privileges to show. With 1-20 privileges you can easily see all host names and don't need any filtering by initials imo. I would even increase that limit to 30 or so.
Merged, thanks. P.S. For these commits you have Author: unknown <Tyron@.(none)>
Yea, I've made commits before adding my ssh key on my Laptop. I've fixed it already. But thanks.
-- Marc Delisle http://infomarc.info
------------------------------------------------------------------------------ vRanger cuts backup time in half-while increasing security. With the market-leading solution for virtual backup and recovery, you get blazing-fast, flexible, and affordable data protection. Download your free trial now. http://p.sf.net/sfu/quest-d2dcopy1 _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
Tyron Madlener a écrit :
On Mon, May 30, 2011 at 6:47 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php Tyron, I don't see a timepicker in jQuery UI's Datepicker. It only handles the date, not the time.
Oh right. But it is not being used. tab_change.js uses PMA_addDatepicker() from functions.js which uses $.datepicker(), not $.timepicker() from what I can see.
Tyron, js/jquery/timepicker.js is currently used. To prove it, rename the file and try to pick a time when inserting or searching on a table with a time column: it won't work.
And tbl_select.js interestingly doesn't initialize any date/timepicker widget at all.
Shall I try and implement it instead?
-- Marc Delisle http://infomarc.info
On Mon, May 30, 2011 at 10:18 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
On Mon, May 30, 2011 at 6:47 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php Tyron, I don't see a timepicker in jQuery UI's Datepicker. It only handles the date, not the time.
Oh right. But it is not being used. tab_change.js uses PMA_addDatepicker() from functions.js which uses $.datepicker(), not $.timepicker() from what I can see.
Tyron,
js/jquery/timepicker.js is currently used. To prove it, rename the file and try to pick a time when inserting or searching on a table with a time column: it won't work.
Ow, now I see. I didn't check it with a DATETIME type. Commit is reverted on my repos now. Thanks for explaining!
And tbl_select.js interestingly doesn't initialize any date/timepicker widget at all.
Shall I try and implement it instead?
-- Marc Delisle http://infomarc.info
------------------------------------------------------------------------------ vRanger cuts backup time in half-while increasing security. With the market-leading solution for virtual backup and recovery, you get blazing-fast, flexible, and affordable data protection. Download your free trial now. http://p.sf.net/sfu/quest-d2dcopy1 _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
One more patch suggestion: - Commit 9385012b97b4c188618ee8658d4e7a3b9200aa5d The status page displays percentual and averaged numbers, which for very low values displays 0.0% or 0.0 which is mathematically seen incorrect. The supplied patch changes such values into "<0.1", or verbally expressed: "Smaller than the smallest value I can display". It's calculated with "(1/PMA_pow(10,$comma))", so its always the smallest displayable value. On Mon, May 30, 2011 at 10:59 PM, Tyron Madlener <tyronx@gmail.com> wrote:
On Mon, May 30, 2011 at 10:18 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
On Mon, May 30, 2011 at 6:47 PM, Marc Delisle <marc@infomarc.info> wrote:
Tyron Madlener a écrit :
Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php Tyron, I don't see a timepicker in jQuery UI's Datepicker. It only handles the date, not the time.
Oh right. But it is not being used. tab_change.js uses PMA_addDatepicker() from functions.js which uses $.datepicker(), not $.timepicker() from what I can see.
Tyron,
js/jquery/timepicker.js is currently used. To prove it, rename the file and try to pick a time when inserting or searching on a table with a time column: it won't work.
Ow, now I see. I didn't check it with a DATETIME type. Commit is reverted on my repos now. Thanks for explaining!
And tbl_select.js interestingly doesn't initialize any date/timepicker widget at all.
Shall I try and implement it instead?
-- Marc Delisle http://infomarc.info
------------------------------------------------------------------------------ vRanger cuts backup time in half-while increasing security. With the market-leading solution for virtual backup and recovery, you get blazing-fast, flexible, and affordable data protection. Download your free trial now. http://p.sf.net/sfu/quest-d2dcopy1 _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
Hi Dne Tue, 31 May 2011 10:43:01 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
One more patch suggestion:
- Commit 9385012b97b4c188618ee8658d4e7a3b9200aa5d The status page displays percentual and averaged numbers, which for very low values displays 0.0% or 0.0 which is mathematically seen incorrect. The supplied patch changes such values into "<0.1", or verbally expressed: "Smaller than the smallest value I can display". It's calculated with "(1/PMA_pow(10,$comma))", so its always the smallest displayable value.
Makes sense. But please for next time keep formatting changes in separate commit, to ease bug finding in future (you can use some graphical tool or git add -i to choose which hunks you want to be part of the commit). -- Michal Čihař | http://cihar.com | http://blog.cihar.com
Hi Dne Tue, 31 May 2011 10:51:35 +0200 Michal Čihař <michal@cihar.com> napsal(a):
Hi
Dne Tue, 31 May 2011 10:43:01 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
One more patch suggestion:
- Commit 9385012b97b4c188618ee8658d4e7a3b9200aa5d The status page displays percentual and averaged numbers, which for very low values displays 0.0% or 0.0 which is mathematically seen incorrect. The supplied patch changes such values into "<0.1", or verbally expressed: "Smaller than the smallest value I can display". It's calculated with "(1/PMA_pow(10,$comma))", so its always the smallest displayable value.
Makes sense.
Er, actually not, because you should display 0 when then value is 0 and not <1 in such case. -- Michal Čihař | http://cihar.com | http://blog.cihar.com
On Tue, May 31, 2011 at 10:51 AM, Michal Čihař <michal@cihar.com> wrote:
Hi
Dne Tue, 31 May 2011 10:43:01 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
One more patch suggestion:
- Commit 9385012b97b4c188618ee8658d4e7a3b9200aa5d The status page displays percentual and averaged numbers, which for very low values displays 0.0% or 0.0 which is mathematically seen incorrect. The supplied patch changes such values into "<0.1", or verbally expressed: "Smaller than the smallest value I can display". It's calculated with "(1/PMA_pow(10,$comma))", so its always the smallest displayable value.
Makes sense. But please for next time keep formatting changes in separate commit, to ease bug finding in future (you can use some graphical tool or git add -i to choose which hunks you want to be part of the commit).
Alright, will do. (it was done more or less automatically by my text editor)
Er, actually not, because you should display 0 when then value is 0 and not <1 in such case.
You mean for values without comma? Are there cases where such behavior is required in pma?
-- Michal Čihař | http://cihar.com | http://blog.cihar.com
------------------------------------------------------------------------------ Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Data protection magic? Nope - It's vRanger. Get your free trial download today. http://p.sf.net/sfu/quest-sfdev2dev _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
Hi Dne Tue, 31 May 2011 11:54:17 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
On Tue, May 31, 2011 at 10:51 AM, Michal Čihař <michal@cihar.com> wrote:
Er, actually not, because you should display 0 when then value is 0 and not <1 in such case.
You mean for values without comma? Are there cases where such behavior is required in pma?
I mean when there is 0 as a value, for example in status variables, let's say Com_alter_db_upgrade or all Ssl_* variables if ssl is not being used. In such case displaying 0 is right thing to do, but not <1 as your patch does. -- Michal Čihař | http://cihar.com | http://blog.cihar.com
On Tue, May 31, 2011 at 1:38 PM, Michal Čihař <michal@cihar.com> wrote:
Hi
Dne Tue, 31 May 2011 11:54:17 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
On Tue, May 31, 2011 at 10:51 AM, Michal Čihař <michal@cihar.com> wrote:
Er, actually not, because you should display 0 when then value is 0 and not <1 in such case.
You mean for values without comma? Are there cases where such behavior is required in pma?
I mean when there is 0 as a value, for example in status variables, let's say Com_alter_db_upgrade or all Ssl_* variables if ssl is not being used. In such case displaying 0 is right thing to do, but not <1 as your patch does.
Oh yes, thats not right. Commit 226ca36c9cfa6f15b79ec058f92bc5c36675541f fixes that Here's a patch that includes both commits: http://www.tyron.at/tmp/number_format.diff
-- Michal Čihař | http://cihar.com | http://blog.cihar.com
------------------------------------------------------------------------------ Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Data protection magic? Nope - It's vRanger. Get your free trial download today. http://p.sf.net/sfu/quest-sfdev2dev _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
Hi Dne Tue, 31 May 2011 14:19:14 +0200 Tyron Madlener <tyronx@gmail.com> napsal(a):
Oh yes, thats not right. Commit 226ca36c9cfa6f15b79ec058f92bc5c36675541f fixes that
Thanks, cherry picked both of them to master. -- Michal Čihař | http://cihar.com | http://blog.cihar.com
One more suggestion, I'm still not happy with PMA_formatNumber() ;) While replacing pChart in the profiling info I also wanted to format the numbers in the table a bit nicer, however that is with the current version of PMA_formatNumber not really possible, as it returns numbers like '0.003 m' which are much harder to read than '30 µ' And its general behavior seems kinda odd to me. My rewrite is a lot shorter and simpler too. Comparison with the supplied example parameters, old: * echo PMA_formatNumber(123456789, 6); // 123,457 k * echo PMA_formatNumber(-123456789, 4, 2); // -123.46 M * echo PMA_formatNumber(-0.003, 6); // -3 m * echo PMA_formatNumber(0.003, 3, 3); // 0.003 * echo PMA_formatNumber(0.00003, 3, 2); // 0.03 m * echo PMA_formatNumber(0, 6); // 0 And with my new code: 123,457 k -123.46 M -3,000 µ 3m 30 µ 0 I've also changed the parameters slightly Old: function PMA_formatNumber($value, $length = 3, $comma = 0, $only_down = false); New: function PMA_formatNumber($value, $digits_left = 3, $digits_right = 0, $only_down = false, $noTrailingZero = true); digits_left,digits_right behave the same as length,comma noTrailingZero removes zeros right of the comma, e.g. 3.4000 => 3.4 - this will be helpful if a bunch of numbers should be formatted all the same. Commit id is 86323a781560b4526b5f5010a1c93adeb410ba51 on my repos. And e913dcc6e115a52b2a3305e53cc74fcc3d4d1a46 replaces the tabs with spaces.
On Sat, May 28, 2011 at 12:05 AM, Tyron Madlener <tyronx@gmail.com> wrote: Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php
- Commit b161f43ec9089e0d2d5fb8a619056e6f9577a380 When reloading (flushing) privileges on the privileges tab you get a 5 second long success message. This commit omits the success message and removes the loading message as soon as the ajax request is done. This behavior seems more consistent with my patch in April that removes messages once the ajax request is completed.
- Commit 99775c9bad552aac8b904996d673f1023821ab13 Removes the initials table on the privileges tab if there is 20 or less privileges to show. With 1-20 privileges you can easily see all host names and don't need any filtering by initials imo. I would even increase that limit to 30 or so.
On Tue, Jun 7, 2011 at 8:59 PM, Tyron Madlener <tyronx@gmail.com> wrote:
One more suggestion, I'm still not happy with PMA_formatNumber() ;)
While replacing pChart in the profiling info I also wanted to format the numbers in the table a bit nicer, however that is with the current version of PMA_formatNumber not really possible, as it returns numbers like '0.003 m' which are much harder to read than '30 µ' And its general behavior seems kinda odd to me. My rewrite is a lot shorter and simpler too.
Comparison with the supplied example parameters, old: * echo PMA_formatNumber(123456789, 6); // 123,457 k * echo PMA_formatNumber(-123456789, 4, 2); // -123.46 M * echo PMA_formatNumber(-0.003, 6); // -3 m * echo PMA_formatNumber(0.003, 3, 3); // 0.003 * echo PMA_formatNumber(0.00003, 3, 2); // 0.03 m * echo PMA_formatNumber(0, 6); // 0
And with my new code: 123,457 k -123.46 M -3,000 µ 3m 30 µ 0
I've also changed the parameters slightly
Old: function PMA_formatNumber($value, $length = 3, $comma = 0, $only_down = false);
New: function PMA_formatNumber($value, $digits_left = 3, $digits_right = 0, $only_down = false, $noTrailingZero = true);
digits_left,digits_right behave the same as length,comma noTrailingZero removes zeros right of the comma, e.g. 3.4000 => 3.4 - this will be helpful if a bunch of numbers should be formatted all the same.
Commit id is 86323a781560b4526b5f5010a1c93adeb410ba51 on my repos. And e913dcc6e115a52b2a3305e53cc74fcc3d4d1a46 replaces the tabs with spaces.
Patch 0d1872595f6f1c14602a7edd1d652bc84ffe0b3f does the calculation without any loops now \o/
On Sat, May 28, 2011 at 12:05 AM, Tyron Madlener <tyronx@gmail.com> wrote: Since I always have a love for details, here's 3 small suggestions that maybe can be merged into the master branch. They are all pushed to my fork (http://repo.or.cz/w/phpmyadmin/tyronm.git):
- Commit a0302c666c1bf9f9715364a6301238b474aedac1 Removes timepicker.js. It is already included with the jquery ui. The file was included only in tbl_change.php and tbl_select.php
- Commit b161f43ec9089e0d2d5fb8a619056e6f9577a380 When reloading (flushing) privileges on the privileges tab you get a 5 second long success message. This commit omits the success message and removes the loading message as soon as the ajax request is done. This behavior seems more consistent with my patch in April that removes messages once the ajax request is completed.
- Commit 99775c9bad552aac8b904996d673f1023821ab13 Removes the initials table on the privileges tab if there is 20 or less privileges to show. With 1-20 privileges you can easily see all host names and don't need any filtering by initials imo. I would even increase that limit to 30 or so.
participants (3)
-
Marc Delisle -
Michal Čihař -
Tyron Madlener