[Phpmyadmin-devel] Column comments: First patch.
Salutations, phpmyadmin-devel! I just did a first patch because I felt like I touched many files already and didn't want too much trouble when importing them. I will, however, put the MIME-Changes in next week, so probably you decide it is better to wait with all changes. Most changes made by me yet are not critical. If they won't get in for 2.4.x release I won't be mad, but I would appreciate if no bigger changes are then made to the pages I edited. Not that I would think, anyone would do that. ;) Here's my Changelog: TODO-Items solved: http://sourceforge.net/tracker/index.php?func=detail&aid=577328&group_id=230... http://sourceforge.net/tracker/index.php?func=detail&aid=571934&group_id=230... http://sourceforge.net/tracker/index.php?func=detail&aid=544361&group_id=230... * Updated verticaldisplay, javascript rowpointer now available * Integrated comments where possible (like as a mouse-hover in table properties, made visible via a dashed underline and also in SQL-Dump modes) * New display mode: horizontal with flipped table headers (see todo-item) * Display Query time (see todo-item) * Made basic integritychecks for comments-table: Updates, Deletes and Inserts rows if tables get deleted, updated or moved. This does not yet work for the relation tables, I didn't dare to touch them because I never used those I did not put the new feature into documentation, neither did I translate the new strings, and just put them into english language. But I think there is a simple bash-script which compares the new translations, right? I will make MIME-editing available on the same pages as the comments are now placed in. For the diff please see http://www.supergarv.de/phpmyadmin.diff.zip (created with 'cvs -n diff -R -N >> phpmyadmin.diff') And, uhm, if you think that the general code style is appropriate for your project, could it be possible to get a cvs account? This would at last take the pressure of me to stay in sync with the cvs repository. ;) -- Bye, ...[ icq #21392242 | Garvin ...[ www.supergarv.de | ... *The earth is in jail and we're plotting this incredible jailbreak.*
Hi Garvin, Not trying to make your life difficult, but could you please: 1. Generate unified diffs with context instead of the RCS format. (use 'cvs -n diff -udHwbRN'). They are MUCH easier to review. 2. Seperate patches for each feature/todoitem/bugfix - this is to make your submissions easier to review and add in incrementally. This is partially important with touchy patches like your changes to 'libraries/display_tbl.lib.php3' that change a LOT of code scattered about. 3. for your new strings, please go into the lang/ directory and run: "./add_message.sh '$strMessageName' 'Message contents'" the single quotes are very important for this. It will add your string to all of the files. For this, do NOT use arrays like you are doing presently, sorry, but they cause some problems with the translations system. Then just attach a patch for the whole language directory as a single patch. Reviewing your code, some possibly bugs: libraries/build_dump.lib.php3: - if (PMA_MYSQL_INT_VERSION >= 32321) { + if (PMA_MYSQL_INT_VERSION >= 932321) { typo ? sql.php3: + list($usec, $sec) = explode(" ",microtime()); please use ' instead of " in your PHP code wherever expansion of contents is NOT needed, it leads to a performance post and more security. libraries/common.lib.php3: + } elseif ($char == ';' and !empty($charbuff)) { ... + if ($append AND $i != strlen($string)) { Is explicit use of 'AND' portable to PHP3 ? (Rabus?) Possibly stick to '&&' ? Other than those issues, your code looks good from a programming standpoint and style standpoint. I haven't applied it to my tree yet, because I'm busy on something else... Write up some documentation once Michal has checked in his new documentation layout, as I think this very important for the integrity checks amongst other things. Doe the checks and so forth also work if tables or columns are renamed or moved between databases? Just make sure it conforms to the PEAR coding guidelines, and all should be fine. (do NOT follow their suggestion about using indent on the webpage, it destroys code if you aren't careful). +1 from me on you getting a CVS account, yay or nay anybody else ? -- Robin Hugh Johnson E-Mail : robbat2@orbis-terrarum.net Home Page : http://www.orbis-terrarum.net/?l=people.robbat2 ICQ# : 30269588 or 41961639 GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Hi On Saturday 01 February 2003 16:14, Robin Johnson wrote:
Write up some documentation once Michal has checked in his new documentation layout, as I think this very important for the integrity checks amongst other things. Doe the checks and so forth also work if tables or columns are renamed or moved between databases?
New docs checked in, so add documentation to all new configuration options...
Just make sure it conforms to the PEAR coding guidelines, and all should be fine. (do NOT follow their suggestion about using indent on the webpage, it destroys code if you aren't careful).
And set your editor not to use tabs, use 4 spaces instead...
+1 from me on you getting a CVS account, yay or nay anybody else ?
Also +1, in the worst case we can cancel it and reverse the changes in cvs ;-)). -- Regards Michal Cihar nijel at users dot sourceforge dot net http://cihar.liten.cz
Hi Michael!
And set your editor not to use tabs, use 4 spaces instead...
Geez, you really use those? I hate that. It makes block-indenting a pain in the ass. I personally use Homesite for editing, I will have to see if it supports this kind of indenting...
Also +1, in the worst case we can cancel it and reverse the changes in cvs ;-)).
Yeah, uhm. Sure, that would be nice. ;-)) Regards, Garvin.
On Sat, Feb 01, 2003 at 06:26:34PM +0100, garvin_mailings@supergarv.de wrote:
Hi Michael!
And set your editor not to use tabs, use 4 spaces instead... Geez, you really use those? I hate that. It makes block-indenting a pain in the ass. I personally use Homesite for editing, I will have to see if it supports this kind of indenting... Most of us have gone thru many many editors finding what is right for us. I started out there, then went to pico/nano on UNIX, then NEDIT on IRIX, came back to windows editing when DreamWeaver first supported PHP, since then I've migrated to VIM, and now i'm learning Emacs just to compare it to vim for producitivity and ease of coding.
Most of the windows editors have nothing on VIM and Emacs in terms of what can really be done. You don't even have to run them on UNIX in reality. Most of my editing is done in a Win2K enviroment, running gvim, editing files mounted via NFS.
Also +1, in the worst case we can cancel it and reverse the changes in cvs ;-)). Yeah, uhm. Sure, that would be nice. ;-)) LOL.
-- Robin Hugh Johnson E-Mail : robbat2@orbis-terrarum.net Home Page : http://www.orbis-terrarum.net/?l=people.robbat2 ICQ# : 30269588 or 41961639 GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Hi Robin!
1. Generate unified diffs with context instead of the RCS format. (use 'cvs -n diff -udHwbRN'). They are MUCH easier to review.
Yes, no problem for me. I didn't submit diff's yet, so I didn't know which specific commands to use with.
2. Seperate patches for each feature/todoitem/bugfix - this is to make your submissions easier to review and add in incrementally. This is partially important with touchy patches like your changes to 'libraries/display_tbl.lib.php3' that change a LOT of code scattered about.
This will get kind of heavy because I will have to seperate the whole code again...
3. for your new strings, please go into the lang/ directory and run: "./add_message.sh '$strMessageName' 'Message contents'" the single quotes are very important for this. It will add your string to all of the files.
Allright, will do that.
For this, do NOT use arrays like you are doing presently, sorry, but they cause some problems with the translations system. Then just attach a patch for the whole language directory as a single patch.
I just found out that I did submit the wrong diff, which was done from my backup-directory instead of my latest working directory -- I diffed from the dir, where I also made the first tries for the MIME-stuff, for which the array-strings where for. I didn't look into the translation system yet and would have noticed that a bit too late. So I can structure the whole thing a bit different; even though arrays would be great for descriptions of each transformation rule.
libraries/build_dump.lib.php3: - if (PMA_MYSQL_INT_VERSION >= 32321) { + if (PMA_MYSQL_INT_VERSION >= 932321) { typo ?
Bug from my wrongly diffed working directory, yes. Shame on me. *darn*
sql.php3: + list($usec, $sec) = explode(" ",microtime()); please use ' instead of " in your PHP code wherever expansion of contents is NOT needed, it leads to a performance post and more security.
Allright - I took this snipped from the php.net website. Normaly I use ' throughout my code because of this reason. Thanks for noticing.
+ } elseif ($char == ';' and !empty($charbuff)) { ... + if ($append AND $i != strlen($string)) { Is explicit use of 'AND' portable to PHP3 ? (Rabus?) Possibly stick to '&&' ?
My own scripts normally all use "AND" and "OR" and so on, because I find that easier to read. I tried to stick to your rules at most, but sometimes it slipped through. I will take care on this for future patches.
Write up some documentation once Michal has checked in his new documentation layout, as I think this very important for the integrity checks amongst other things. Doe the checks and so forth also work if tables or columns are renamed or moved between databases?
I will put documentation in that, as soon as I find out how to do that. Haven't looked into it. The checks do apply for moving/copying around databases, yes. Works pretty smoothly. :-)
Just make sure it conforms to the PEAR coding guidelines, and all should be fine. (do NOT follow their suggestion about using indent on the webpage, it destroys code if you aren't careful).
My personal coding style (mostly the position of '}') is a bit different, but yes, I will stick to them. :) Thanks so far and have a nice week-end, Garvin.
On Sat, Feb 01, 2003 at 06:25:05PM +0100, garvin_mailings@supergarv.de wrote:
2. Seperate patches for each feature/todoitem/bugfix - this is to make your submissions easier to review and add in incrementally. This is partially important with touchy patches like your changes to 'libraries/display_tbl.lib.php3' that change a LOT of code scattered about. This will get kind of heavy because I will have to seperate the whole code again... It doesn't matter too much if some little bits slip thru the cracks, but it definetly helps keeping stuff seperate.
For this, do NOT use arrays like you are doing presently, sorry, but they cause some problems with the translations system. Then just attach a patch for the whole language directory as a single patch. I just found out that I did submit the wrong diff, which was done from my backup-directory instead of my latest working directory -- I diffed from the dir, where I also made the first tries for the MIME-stuff, for which the array-strings where for. I didn't look into the translation system yet and would have noticed that a bit too late. So I can structure the whole thing a bit different; even though arrays would be great for descriptions of each transformation rule. Hmm, we hadn't really needed translation arrays before, what might be possible is in the language files, have it formatted like: $strTransform_foo = '...' and then write some code in the main system to transform that into $strTransform['foo'] = '...'
Can anybody remember what the original problem with putting arrays into the translation system was? I think it may have been some translator translating the array indices, either that or some bug with the translation handling scripts.
libraries/build_dump.lib.php3: - if (PMA_MYSQL_INT_VERSION >= 32321) { + if (PMA_MYSQL_INT_VERSION >= 932321) { typo ? Bug from my wrongly diffed working directory, yes. Shame on me. *darn* Code review really helps most of the time :-). We really should practice more Extreme Programming techniques. From my experience they work well, but the only problem is the amount of extra time needed, and the lack of refactoring tools for PHP.
The checks do apply for moving/copying around databases, yes. Works pretty smoothly. :-) So if I rename a db/column/table, the relations table gets updated automatically?
Just make sure it conforms to the PEAR coding guidelines, and all should be fine. (do NOT follow their suggestion about using indent on the webpage, it destroys code if you aren't careful). My personal coding style (mostly the position of '}') is a bit different, but yes, I will stick to them. :) We all have small style quirks. I really wish that the indent tool worked properly for PHP, as it would make life much easier. My usual practice with C++ code is that all files should include a tailer with /* indent: (options here) */ that specify the defaults for what the file is to be kept in. Then when you want to edit the file, you run it thru indent with your own personal prefences, and use the defaults on it again when you are done.
Thanks so far and have a nice week-end, I really should be doing more studying and assignments for my exam week, but coding is more relaxing and fun.
-- Robin Hugh Johnson E-Mail : robbat2@orbis-terrarum.net Home Page : http://www.orbis-terrarum.net/?l=people.robbat2 ICQ# : 30269588 or 41961639 GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
Garvin Hicking wrote:
And, uhm, if you think that the general code style is appropriate for your project, could it be possible to get a cvs account? This would at last take the pressure of me to stay in sync with the cvs repository. ;)
+1 for me too. Garvin, what is your sourceforge username? Marc
Hi Marc!
+1 for me too.
Thanks for that. I will try to contribute to some other todo-items, as soon as the whole mime-thing is done. I'm having a three-weeks vacation, and as I know myself I won't be able to get myself away from coding. ;)
Garvin, what is your sourceforge username?
It's 'garvinhicking'. Creativity fades. ;-) Regards, Garvin.
participants (5)
-
garvin_mailings＠supergarv.de -
Marc Delisle -
me＠supergarv.de -
Michal Cihar -
Robin Johnson