[geeklog-devel] Static Pages Plugin and advanced editor

[Samuel Leathers]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dirk, thanks for your reply.

I changed it to use the $_USER value instead of doing the extra query.
Thanks for letting me know about that. I also moved the { up to the end
of the line.

I did a diff -u this time, and I'll add a ticket to bugtrack with a
suggested patch soon. Just want to make sure I get the diff command
correct for the patch. I've attached the latest attempt at making a patch.

Also, to generate the patch, I've been copying the original file to
<orig>_old and then running a diff that way. Is this the best way to do
it, or should I have an intact copy of the tree and always do a diff -r?
Or better yet, since I'm using the mercurial repo is there some way I
can have it generate a diff of the changes I've made so far?


Sam

On 4/30/10 4:08 PM, Dirk Haun wrote:
> Samuel Leathers wrote:
> 
>> Please look it over when you get the chance and give me
>> any feedback regarding coding style or functionality.
> 
> Haven't check the functionality, but here are some comments on the patch
> itself:
> 
> - Please use unified diffs[1], i.e diff -u, since that provides some
> context and improves chances that the patch can be applied when the line
> numbers change
> 
> - Our Coding Guidelines[2] ask for the opening { to go at the end of a line
> 
> - When requesting only one value from the db, instead of DB_query
> ("SELECT advanced_editor FROM {$_TABLES['userprefs']} WHERE uid = {$_USER
> ['uid']};"); use
> 
>     DB_getItem($_TABLES['userprefs'], 'advanced_editor', "uid = {$_USER
> ['uid']}");
> 
> - You don't actually need to do that, though, since $_USER
> ['advanced_editor'] already contains that value (even for the Anonymous user)
> 
> Sorry for finding so much to criticise for so little code ;-) You may
> want to open an issue in our bugtracker for that, btw, as it's a bit
> late to include it in 1.7.0 and we don't want it to get lost.
> 
> Thanks for your efforts!
> 
> bye, Dirk
> 
> [1] http://wiki.geeklog.net/index.php/Submitting_Patches
> [2] http://wiki.geeklog.net/index.php/Coding_Guidelines
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.13 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJL3taTAAoJEF7J+Z7XDXx/AgIH/Revf4EDXR1r//Yl2179RbuG
SrVUnp7wVdDpRatZgrtMs3SQRxeFVVMxbg5impthfr1iZsHBDfocdmKUtCILGeYr
YPWl6rHRePBldjGdYeNwPTCHQ+VO65gVyfookCjYhNB19EQWFlIYjJNKWDZ5FJ7b
1aN9sTOrEmglEgjp7Glemp3NEURp6bAnoj8TsGaOTfv1nctUk2XVlQ2/ZZs1vjGP
F2IEPcjJkaQdilvxAG2yrsnl2265ijC0QyfkZdo+iXAcGrMD5ZOLQpiImOR4fioz
+/FG9QfXS1o6vH1jEum58pRlkC3Qd0vXGOAlUqJbxSaKyeYumwu++DypnYMOp5k=
=gQT9
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: adv_edit.patch
URL: <https://pairlist8.pair.net/pipermail/geeklog-devel/attachments/20100503/5c3243fd/attachment.ksh>