Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Sun, 18 Apr 2010 03:05:53 +0200, Dominic wrote: Am Samstag, den 17.04.2010, 18:14 +0200 schrieb Enrico Tröger: I spent a little more time on this again hoping to get it fixed. In SVN r4828, I adjusted the regular expression for parsing PHP functions a bit and I think it works now as expected or at least better than before. All function definitions I tried (with static, public, ... modifiers and some without and so on) worked and I got the expected argument list as calltip. Though, as muliple times mentioned before, I'm not a PHP guy. Feedback is welcome. Your pattern seems to work fine now. I've successfully tested different cases as attached and any combination of testcase 1-3 with testcase 4. Thanks for testing. Since we're obviously writing object-oriented PHP here (because we're using public static anything and so on), it would be nice, if Geany also would parse the so called magic methods [1]. You may noted the __construct method in the attached files. This method is called when a The __construct methods are already parsed and listed in the Symbol list. new instance is created as you see in line 24. For completeness Geany should also show calltips in that line when creating the new instance. This is not possible with the current implementation of the parser. Since it is based on regular expressions which only work line-based, there is no information about a possibly following __construct() method when it is parsing a class definition. This is also the reason why the parser often fails at ignoring statements inside multi-line comments. As I said a couple of times in the past, the best solution would be to rewrite the parser to parse the whole source file at once as most other parsers do instead of simple regexps for each line. PHP is simply to complex for such a simple approach. As usual, it needs someone to do it. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpNw8PBCpFm7.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Sun, 18 Apr 2010 03:05:53 +0200, Dominic wrote: Am Samstag, den 17.04.2010, 18:14 +0200 schrieb Enrico Tröger: On Mon, 8 Mar 2010 23:40:08 +0100, Enrico wrote: On Mon, 8 Mar 2010 12:18:39 +, Nick wrote: On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Just for the records: Yup, we talked about that yesterday afterwards on IRC and once he told me about that mistake, it was clear. I spent a little more time on this again hoping to get it fixed. In SVN r4828, I adjusted the regular expression for parsing PHP functions a bit and I think it works now as expected or at least better than before. All function definitions I tried (with static, public, ... modifiers and some without and so on) worked and I got the expected argument list as calltip. Though, as muliple times mentioned before, I'm not a PHP guy. Feedback is welcome. Your pattern seems to work fine now. I've successfully tested different cases as attached and any combination of testcase 1-3 with testcase 4. But, I'd like to point out another issue or rather a feature request. I don't feel it's that important, at least not for me. Also, it maybe is very language-specific for PHP: Since we're obviously writing object-oriented PHP here (because we're using public static anything and so on), it would be nice, if Geany also would parse the so called magic methods [1]. You may noted the __construct method in the attached files. This method is called when a new instance is created as you see in line 24. For completeness Geany should also show calltips in that line when creating the new instance. That was a great idea :). As I said, this isn't (easily) possible for the PHP parser, but it was quite easy for the Python parser and since I missed that feature already a couple of times in the past, I implemented it. Thanks for the pointer :). Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgp5wL8DUMz6C.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Mon, 8 Mar 2010 23:40:08 +0100, Enrico wrote: On Mon, 8 Mar 2010 12:18:39 +, Nick wrote: On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Just for the records: Yup, we talked about that yesterday afterwards on IRC and once he told me about that mistake, it was clear. I spent a little more time on this again hoping to get it fixed. In SVN r4828, I adjusted the regular expression for parsing PHP functions a bit and I think it works now as expected or at least better than before. All function definitions I tried (with static, public, ... modifiers and some without and so on) worked and I got the expected argument list as calltip. Though, as muliple times mentioned before, I'm not a PHP guy. Feedback is welcome. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpSkNiaAda1R.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Wed, 14 Apr 2010 15:14:47 +0200, Harold wrote:
2010/3/7 Enrico Tröger enrico.troe...@uvena.de:
On Fri, 5 Mar 2010 12:21:25 -0800 (PST), Can wrote:
Hi,
The following patch adds call tips for functions in PHP code.
The regex pattern for function parameters is not optimal, but should
work for most cases. When you have a problem, send the relevant
function definition so that I can improve the patch.
Awesome (though I still think it'd be better in thelong term to write
a C-based, real parser insteaf of fiddling with regexps).
...
For the meantime, committed.
I just found a 'bug' in the code:
function my_func($var1, $var2 = array(), $var3 = NULL) { ... } only
shows my_func($var1, $var2 = array()
So it stops at the first ) it finds instead of the real closing ).
Should be fixed in SVN r4830.
Thanks for testing and reporting.
I also tried to fix the remaining bugs with the function parsing regexp
which I think I did in SVN r4828.
I guess you'll tell me if I'm wrong :).
Regards,
Enrico
--
Get my GPG key from http://www.uvena.de/pub.asc
pgpJwvQU7rf8v.pgp
Description: PGP signature
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
2010/3/7 Enrico Tröger enrico.troe...@uvena.de:
On Fri, 5 Mar 2010 12:21:25 -0800 (PST), Can wrote:
Hi,
The following patch adds call tips for functions in PHP code.
The regex pattern for function parameters is not optimal, but should
work for most cases. When you have a problem, send the relevant
function definition so that I can improve the patch.
Awesome (though I still think it'd be better in thelong term to write
a C-based, real parser insteaf of fiddling with regexps).
...
For the meantime, committed.
I just found a 'bug' in the code:
function my_func($var1, $var2 = array(), $var3 = NULL) { ... } only
shows my_func($var1, $var2 = array()
So it stops at the first ) it finds instead of the real closing ).
-H-
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
From: Nick Treleaven nick.trelea...@btinternet.com Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 12:18 PM On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Actually, I'm very uncomfortable with my name being recorded in that patch's log (r4729). That regex is funny, it exhibits an ignorance in the subject and it's not what I submitted. So, I demand my name removed from the logs for that patch. -- Can Koy ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
well, who the hell are you? --- On Mon, 3/8/10, Thomas Martitz thomas.mart...@student.htw-berlin.de wrote: From: Thomas Martitz thomas.mart...@student.htw-berlin.de Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 7:46 PM Am 08.03.2010 20:23, schrieb Can Koy: From: Nick Treleavennick.trelea...@btinternet.com Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 12:18 PM On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Trögerenrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Actually, I'm very uncomfortable with my name being recorded in that patch's log (r4729). That regex is funny, it exhibits an ignorance in the subject and it's not what I submitted. So, I demand my name removed from the logs for that patch. -- Can Koy Then please submit a patch to the subversion project to enable post-editing of commit messages, since this is not possible currently. But honestly, I don't see why this is a big deal. Maybe we can concentrate on improving Geany and not insisting on minor things like this. Best regards. ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
Am Montag, den 08.03.2010, 12:15 -0800 schrieb Can Koy: well, who the hell are you? Can, Thomas is contributing to the Geany project for a very long time. As developer as well as a tester. I do not see any reason to ask such questions here. Especially not in that caustically way. Actually, I'm very uncomfortable with my name being recorded in that patch's log (r4729). That regex is funny, it exhibits an ignorance in the subject and it's not what I submitted. So, I demand my name removed from the logs for that patch. Maybe it could be, Enrico modified some things on your patch before he commited it, I don't know. If this is the case, he maybe forgot to write the commit message as based on patch from ... as he usually does. Well, that could happen to anyone, we're all just humans. So, thanks very much for your understanding. Anyway, Enrico has every right to modify code before applying it onto Geany. As you may have noticed he is one of the main authors of Geany and initiated the project. He still is one of the last ones who decide if code makes it into Geany or not. Geany is Free Software under GPL and any contribution should be too. Thus I see no reason why patches may not be modified before applying them to the project. If the modification is an improvement or not is a technical question which I am not asking for at present. We're open to discuss any technical issue. :) [snip] Then please submit a patch to the subversion project to enable post-editing of commit messages, since this is not possible currently. But honestly, I don't see why this is a big deal. Maybe we can concentrate on improving Geany and not insisting on minor things like this. The fact that your name is mentioned in the commit message can not be undone due to the technical reasons Thomas wrote. I'm afraid you will have to live with that. Please keep in mind that Free Software projects are not mainly intended to make someones name public or not. Such projects are intended to provide good software for the user. - Thus a lot of your patches we're very appreciated. - In most Free Software projects it is the case, that the developers actually are people who have very long working days and do their efforts for the projects in their free time after work. This actually is also the case for the Geany project. I think, a bit of understanding from your side would be appreciated by anyone here. Mistakes can happen to anyone. Regards, Dominic -- Dominic Hopf dma...@googlemail.com http://dominichopf.de/ Key Fingerprint: A7DF C4FC 07AE 4DDC 5CA0 BD93 AAB0 6019 CA7D 868D signature.asc Description: Dies ist ein digital signierter Nachrichtenteil ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
Gentlemen please, no flame wars. @Can, Thomas is correct, a commit can't be undone, even if its revented the message is still in the records. Better to concentrate on explaining why the regex needs changing. Everyone makes mistakes. @Enrico, Nick and Can are right, the regex [abc|def] matches any one of characters 'a', 'b', 'c', '|', 'd', 'e', 'f' not the sequences abc or def that (abc|def) matches. There is no way of grouping without saving a match unless the regex engine supports the non-capturing (?: ) operator. Cheers Lex On 9 March 2010 07:15, Can Koy can...@ymail.com wrote: well, who the hell are you? --- On Mon, 3/8/10, Thomas Martitz thomas.mart...@student.htw-berlin.de wrote: From: Thomas Martitz thomas.mart...@student.htw-berlin.de Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 7:46 PM Am 08.03.2010 20:23, schrieb Can Koy: From: Nick Treleavennick.trelea...@btinternet.com Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 12:18 PM On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Trögerenrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Actually, I'm very uncomfortable with my name being recorded in that patch's log (r4729). That regex is funny, it exhibits an ignorance in the subject and it's not what I submitted. So, I demand my name removed from the logs for that patch. -- Can Koy Then please submit a patch to the subversion project to enable post-editing of commit messages, since this is not possible currently. But honestly, I don't see why this is a big deal. Maybe we can concentrate on improving Geany and not insisting on minor things like this. Best regards. ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Mon, 8 Mar 2010 12:18:39 +, Nick wrote: On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Just for the records: Yup, we talked about that yesterday afterwards on IRC and once he told me about that mistake, it was clear. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpNtcmfd6qL1.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Mon, 8 Mar 2010 12:15:39 -0800 (PST), Can wrote: well, who the hell are you? As Dominic said, Thomas is a regular Geany user and contributor, a nice guy and not less important, just a subscriber to this list as you and I are as well. This is a public mailing list, where anyone can state his opinion. Please keep this in mind. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpyynhdulEEv.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
On Mon, 8 Mar 2010 11:23:30 -0800 (PST), Can wrote: From: Nick Treleaven nick.trelea...@btinternet.com Subject: Re: [Geany-devel] [Patch] - Improve PHP parser (call tips) To: geany-devel@uvena.de Date: Monday, March 8, 2010, 12:18 PM On Sun, 7 Mar 2010 19:20:37 +0100 Enrico Tröger enrico.troe...@uvena.de wrote: You put a weird pattern: [public|protected|private|static]* If you wanted to group, you should use () I didn't want to group, we don't need to know the actual modifier, we just want to allow only this subset. And yes, it was a group before but there is just no need to. I think Can is right, [] brackets are only for character ranges, not string matching. So () brackets are necessary for matching even when ignoring the group. Actually, I'm very uncomfortable with my name being recorded in that patch's log (r4729). That regex is funny, it exhibits an ignorance in the subject and it's not what I submitted. So, I demand my name Well, yes, obviously I made a mistake in the commited regexp. One sentence about my motivation for the changes between the current version in trunk and those of your patch: before your patch, 'public function foo' was matched, after your patch those functions didn't match anymore. I tried to improve this by the changes we are talking about. Without a doubt, I did it wrong. Sorry for crediting my changes to your changes with your name. Usually, people appreciate this more than not being credited at all. But in any case, it's too late. The only thing I don't understand is your indignation now, after the yesterday's commit, you showed up in IRC and you probably remember we talked about exactly this then. For me, it sounded like you want to fix my mistake, though probably I misunderstood you. Just for the records and those who were not on IRC, here is the relevant log (the full log can be found on http://irc.geany.org/logs/log_20100307.html): 18:25 cankoy has joined #geany 18:43 cankoy eht16: a bracket expression in regex matches single char. ,not strings. 18:43 eht16 dammit, you are right 18:44 cankoy actually your pattern works, but pnly by luck :-) 18:44 eht16 gimme a second to answer to your other patches, then we'll fix it 18:44 eht16 hates regexps 18:44 eht16 not because they are bad but because eht16 always use them wrong 18:44 cankoy well, that's why I asked you not to do everything :-) 18:45 eht16 haha 18:58 eht16 cankoy: is there a difference between [[:space:]]* and [ \t]* ? 18:59 cankoy yes by definition, but not in current mode of regex in ctags 18:59 eht16 I think we should use one of both then 19:00 cankoy I was thinking about doing \n matches in the future, that's why I kept [:space:] 19:00 cankoy feel free to change 19:01 eht16 I don't get it working for function foo() 19:01 cankoy eht16: my initial patch was matching that. 19:01 eht16 no 19:02 eht16 I thought I tested this 19:02 eht16 well, I try it again to get sure 19:04 eht16 ah, yeah, of course 19:04 eht16 but it didn't work with modifiers 19:05 cankoy eht16: let me take this over. You don't have to commit until it matures. 19:05 eht16 ok, just fix it as you think it should be and sorry for destroying your work :( 19:05 cankoy np, I know you don't have much time. 19:31 cankoy eht16: Harold's regex is also not correct, e.g. it does not match public static function... I'll try to fix that too. 19:32 cankoy eht16: no, sorry. It matches that actually. 19:32 eht16 I didn't even know that's possible in PHP :) I thought we agreed that I made a mistake and this can be fixed. I don't understand why you now raise such a hard voice. removed from the logs for that patch. Done, at least for the ChangeLog. As the others said, removing it from the SVN commit message isn't possible. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpyWWfethX9n.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [Patch] - Improve PHP parser (call tips)
Ok, here's the patch.
From c6b573fa7813969a3fd363bbbac2d6536e684090 Mon Sep 17 00:00:00 2001
From: Can Koy can...@ymail.com
Date: Fri, 5 Mar 2010 22:14:12 +0200
Subject: [PATCH] Improve PHP parser (call tips)
This patch adds call tips for functions in PHP code.
---
tagmanager/php.c | 31 ---
1 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/tagmanager/php.c b/tagmanager/php.c
index ac67759..6e2f39c 100644
--- a/tagmanager/php.c
+++ b/tagmanager/php.c
@@ -19,7 +19,7 @@
#include general.h /* must always come first */
#include string.h
-
+#include main.h
#include parse.h
#include read.h
#include vstring.h
@@ -67,6 +67,7 @@ static kindOption PhpKinds [] = {
#define ALPHA [:alpha:]
#define ALNUM [:alnum:]
+static void function_cb(const char *line, const regexMatch *matches, unsigned int count);
static void installPHPRegex (const langType language)
{
@@ -78,8 +79,9 @@ static void installPHPRegex (const langType language)
\\1, m,macro,macros, NULL);
addTagRegex(language, ^[ \t]*const[ \t]*([ ALPHA _][ ALNUM _]*)[ \t]*[=;],
\\1, m,macro,macros, NULL);
- addTagRegex(language, ^[ \t]*((public|protected|private|static)[ \t]+)*function[ \t]+?[ \t]*([ ALPHA _][ ALNUM _]*),
- \\3, f,function,functions, NULL);
+ addCallbackRegex(language,
+ ^[ \t]*function[ \t]+?[ \t]*([ ALPHA _][ ALNUM _]*)[[:space:]]*(\\([^)]*\\)),
+ NULL, function_cb);
addTagRegex(language, ^[ \t]*(\\$|::\\$|\\$this-)([ ALPHA _][ ALNUM _]*)[ \t]*=,
\\2, v,variable,variables, NULL);
addTagRegex(language, ^[ \t]*((var|public|protected|private|static)[ \t]+)+\\$([ ALPHA _][ ALNUM _]*)[ \t]*[=;],
@@ -94,6 +96,29 @@ static void installPHPRegex (const langType language)
\\3, j,jsfunction,javascript functions, NULL);
}
+static void function_cb(const char *line, const regexMatch *matches, unsigned int count)
+{
+ char *name, *arglist;
+ static char *kindName = function;
+ tagEntryInfo e;
+
+ if (count == 3) {
+
+ name = xMalloc(matches[1].length + 1, char);
+ strncpy(name, line + matches[1].start, matches[1].length);
+ *(name+matches[1].length) = '\x0';
+ arglist = xMalloc(matches[2].length + 1, char);
+ strncpy(arglist, line + matches[2].start, matches[2].length);
+ *(arglist+matches[2].length) = '\x0';
+
+ initTagEntry (e, name);
+ e.kind = 'f';
+ e.kindName = kindName;
+ e.extensionFields.arglist = arglist;
+ makeTagEntry (e);
+ }
+}
+
/* Create parser definition structure */
extern parserDefinition* PhpParser (void)
{
--
1.6.3.3
___
Geany-devel mailing list
Geany-devel@uvena.de
http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
