emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Fwd: Minor bug in cc-menus.el: cc-imenu-java-generic-expression does


From: Nathaniel Flath
Subject: Re: Fwd: Minor bug in cc-menus.el: cc-imenu-java-generic-expression does not match all Java 1.5+ function definitions.
Date: Sat, 22 Aug 2009 09:48:55 -0700

Don't worry about the long delay- I understand that nobody's being paid for Emacs, I just wanted to make sure that my patch hasdn't been lost.  I hope the personal stuff that came up ends up OK for you.  Good luck!

I have not signed any copyright assignment forms to the FSF - I'll print them out and mail them in on Monday. 

The problems with the initial regexp was that it could count whitespace as a type in variables inside words, so it would accept
    else if(  a ) {
        ....

as a function, even though it would reject
    else if( a ) {
        ...

I fixed this by modifying the regexp to split out checking for annotations, types, and variable name seperately, while the original essentially had the mashed together(It was probably based too closely of the regexp I was modifying).

I'm pretty sure the new regexp works for expressions with < and > tokens - I have the test file that I sent, and have been using this regexp since I last emailed it without any false positives or missed functions.

Thanks for taking the time reviewing this patch!

Thanks,
Nathaniel Flath
On Sat, Aug 22, 2009 at 7:17 AM, Alan Mackenzie <address@hidden> wrote:
Hi, Nathaniel,

On Sun, Aug 16, 2009 at 10:33:03PM -0700, Nathaniel Flath wrote:
> ---------- Forwarded message ----------
> From: Nathaniel Flath <address@hidden>
> Date: Sun, Aug 16, 2009 at 10:32 PM
> Subject: Re: Minor bug in cc-menus.el: cc-imenu-java-generic-_expression_ does
> not match all Java 1.5+ function definitions.
> To: Chong Yidong <address@hidden>


> And again.

Sorry.  I've had some heavy personal stuff to deal with in the last few
weeks.  None of us actually gets paid for maintaining Emacs.  I'm still
not fully up to scratch for doing software.

Also, it looks like your change is quite a substantial one, so I'll have
to ask you to sign (paper) copyright assignment forms (to the Free
Software Foundation) before installing it into CC Mode.  Or have you
already been through this?  This is standard FSF policy, and though
ostensibly you're giving something away, in reality you're not.  You
retain the right to do as you wish with your own stuff, but you get the
protection of FSF's lawyers should anybody ever violate your copyright.

> There did turn out to be a few problems with that regexp - the updated
> one is:

Could you tell me please what those problems were (perhaps with a
fragment of Java source where the problems became apparent).

> (defvar cc-imenu-java-generic-_expression_
>   `((nil
>      ,(concat
>        "[" c-alpha "_][\]\[." c-alnum "_<> ]+[ \t\n\r]+" ; type spec
>        "\\([" c-alpha "_][" c-alnum "_]+\\)" ; method name
>        "[ \t\n\r]*"
>        ;; An argument list htat is either empty or contains any number
>        ;; of arguments.  An argument is any number of annotations
>        ;; followed by a type spec followed by a word.  A word is an
>        ;; identifier.  A type spec is an identifier, possibly followed
>        ;; by < typespec > possibly followed by [].
>        (concat "("
>                "\\("
>                   "[ \t\n\r]*"
>                   "\\("
>                      "@"
>                      "[" c-alpha "_]"
>                      "[" c-alnum "._]""*"
>                      "[ \t\n\r]+"
>                   "\\)*"
>                   "\\("
>                      "[" c-alpha "_]"
>                      "[\]\[" c-alnum "_.]*"
>                      "\\("
>                         "<"
>                         "[ \t\n\r]*"
>                         "[\]\[.," c-alnum "_<> \t\n\r]*"
>                         ">"
>                      "\\)?"
>                      "\\(\\[\\]\\)?"
>                      "[ \t\n\r]+"
>                   "\\)"
>                  "[" c-alpha "_]"
>                  "[" c-alnum "_]*"
>                  "[ \t\n\r,]*"
>                "\\)*"
>               ")"
>            "[ \t\n\r]*"
>        "{"
>        )) 1))
>   "Imenu generic _expression_ for Java mode.  See
> `imenu-generic-_expression_'.")

> I ended up just splitting out the annotations from the type from the
> identifier name to make it easier in the argument list.  A file that
> displays some of the matches/non-matches is:

> //(setq imenu-generic-_expression_ cc-imenu-java-generic-_expression_)

> public class Test {

>     void fun1() { }
>     void fun2( int a ) { }
>     void fun3( int a, int b ) { }
>     List<String > fun4() { }
>     Map< String,String > fun5() { }
>     void fun6( @NonNull int a ) { }
>     void fun7( @NonNull int b, @NonNull int c ) { }
>     void fun8( @NonNull List<String> a ) { }
>     void fun9( @NonNull List<List<String >> a ) { }
>     void fun10( @NonNull int[] a) { }
>     void fun11( List< class.innerclass > foo ) { }
>     voif fun12( class.innerclass< Integer> foof ) { }

>     else if( a ) { }
>     else if( a < b ) { }
>     else if( a < b && b > a ) { }
>     else if(  a ) { }
>     else if( a.b ) { }
> }

> the 'funX' should all be matched, with no 'else if's

OK.  Again, how sure are you that the new regexp won't spuriously match
things with "less than" or "greater than" tokens?

Thanks for taking the trouble with this patch, despite all the tedium
I've been causing you.

--
Alan Mackenzie (Nuremberg, Germany).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]