#890 In Javascript function inside method is not recognized.

v1.23
closed-fixed
Filetypes (68)
5
2012-09-24
2012-09-24
No

In JavaScript code a function inside a method is not listed under it in class browser section "Classes", but is listed in the list "Functions". For example, for:

function MyClass() {
}

MyClass.prototype.method2 = function() {
function nestedFunction1() {
};
};

in the left panel I see

- Classes
-- MyClass
--- method2

- Functions
-- MyClass.function.nestedFunction1

It would be better if this function were under method2, i.e.:

- Classes
-- MyClass
--- method2
---- nestedFunction1

I attach the patch fixing this issue.

I apologize if I violate the code style or do something stupid. It's the first time I see the ctags code.

(See discussion here: https://sourceforge.net/tracker/?func=detail&atid=787791&aid=3570192&group_id=153444\)

Discussion

  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24
     
  • Colomban Wendling

    Wow, you're the man! I tried your patch on a few JS files I had under the hand (some script.aculo.us, protoype, lightbox, jit and a few others) and it didn't seem to introduce any issue -- the few tags it changed without making them correct were already incorrect anyway. Bravo!

    A few things on your patch I'd like to see fixed before committing it:

    1) provide a git-format-patch patch, e.g. with author and description. To do this, create a new branch (git checkout -b my-new-branch), commit your changes (git commit [files]) and give a nice description (see HACKING if you don't know what's a good description) and make sure Git uses *your real name and email address* (to keep track of the authors), and finally generate the git-format-patch patch (git format-patch HEAD^).

    2) You declare token2 in the middle of a block, but Geany tries to follow C89 standard which doesn't allow that. So, just move the token2 declaration a to the start of it's block (e.g. 4 lines up).

    Also, if I read the patch correctly, the first block your patch changes (in parseFunction() around line 854) has no real meaning with the rest of the patch (actually, it seems to only simplify a construct). Generally it's not a good idea for a patch to do more than it is needed, so one might argue this part shouldn't be in the patch. However since this doesn't actually changes anything and the code is more readable IMO, I don't mind and will accept the patch with it, that's just a remark for further patches. This said, if you want to change the whole parser to fix all the bugs, that'd be most welcome :p

    About the style, we don't bother much with style issues in the tagmanager/ sub-directoy because it's already a mess, so just follow what's already there -- and you did that just fine :)

     

    Related

    Files: files

  • Colomban Wendling

    • assigned_to: nobody --> codebrainz
     
  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24

    I've attached the new patch. I've moved the declaration of token2 to the start of the function and gave it a more meaningful name.

    As you correctly noted, the first fragment was indented as a small clean-up. Because it wasn't a full-fledged refactoring, but just a minor fix, I decided not to make a separate change for it.

     
  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24

    I've attached the new patch. I've moved the declaration of token2 to the start of the function and gave it a more meaningful name.

    As you correctly noted, the first fragment was intended as a small clean-up. Because it wasn't a full-fledged refactoring, but just a minor fix, I decided not to make a separate change for it.

     
  • Colomban Wendling

    • assigned_to: codebrainz --> colombanw
     
  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24
    • assigned_to: colombanw --> codebrainz
     
  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24

    Sorry, it looks like I've attached the wrong patch. Will attach the right one in a sec.

     
  • Oleg Eterevsky

    Oleg Eterevsky - 2012-09-24

    The correct patch is: 0001-In-ctags-JavaScript-parser-fix-recognizing-functions.patch

     
  • Colomban Wendling

    • assigned_to: codebrainz --> colombanw
     
  • Colomban Wendling

    Committed, thanks a lot!

     
  • Colomban Wendling

    • milestone: --> v1.23
    • status: open --> closed-fixed
     

Log in to post a comment.