Language PHP Support

Escuder Nicolas n.escuder at intra-links.com
Tue Apr 19 17:45:05 UTC 2005


Hello

- ok i will add new file at the end of Makefile.am in the next patch

- kdevdeepcopy.h is a copy of languages/cpp/kdevdeepcopy.h I think I don't
need deepcopy I will see this.

- phpconfigparser* no more need all is in phpconfigwidget.cpp

- Yes I remove the licence for phperrorview.* for putting the licence from
Roberto Raggi because it's based on it's code but I omit to put it in .h i
Will add it 

- Yes I will put the GPL Licence for my own files

Question :

I must rebuild a full patch or just a new patch ? ;o)

See You 

Nicolas

----------------------------------------------------------------------------

A few nitpicks: 

- If you've added new files, and it looks like you have, please add them to 
the end of the Makefile.am file. This is just a style thing really, but it 
makes it easier to see what files are new, and you don't have to do it if
you 
don't want to. :)

- the kdevdeepcopy.h file is wrong. use QDeepCopy instead of rolling your
own. 
However, you shouldn't need deep copy for QStrings, and if you do, you 
definately should be using QDeepCopy instead.

- Are phpconfigparserwidget.h and phpconfigparserwidget.cpp no longer needed

and hence why they are deleted in the patch?

- You removed the license header from phperrorview.h. please add it back.
You 
can add your own copyright there if you want, but the license headers need
to 
be there.

- The license headers are missing from phpfile.h and phpfile.cpp. You should

add license headers to those files. You can copy a header straight from 
another file and modify it for your own needs.

I won't comment on the functionality in the patch since i don't know PHP, so

if nobody else has any other objects, and the questions I've raised are 
answered, then i imagine you'll probably see your work in kdevelop in the 
next release. (btw, the encapsulation of the new options in phpconfigdata.
{h,cpp} looks much better. thanks for implementing my suggestion.)

Thanks for the patch,

Matt






More information about the KDevelop-devel mailing list