Skip to content

Conversation

@b4n
Copy link
Member

@b4n b4n commented Nov 24, 2014

Don't merge it yet! It requires review from people that actually know the Vim syntax :)

@b4n
Copy link
Member Author

b4n commented Nov 24, 2014

One particularly weird thing is the use of strcasecmp() to match keywords in one single place: vim.c:289. If it's not actually useful is could be replaced with a simpler wordMatchLen() call, but if everything should be case-insensitive the rest of the code should be fixed.

vim.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the help it has to be END (typical use case), or end:

                                                        :aug :augroup         
:aug[roup] {name}               Define the autocmd group name for the         
                                following ":autocmd" commands.  The name "end"
                                or "END" selects the default group.           

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so this is simply a special name and not a syntax construct, right?

Anyway a problem here is that it will prevent handling of anything that starts with (caseless) end, like endsomething. IMO, this should rather check if the name it extracted was end (or END, or simply caseless check if this can be EnD or the likes), or if really we don't want to fill the name in this case, use (! wordMatchLen(cp, "end", 3) && !wordMatchLen(cp, "END", 3)).

Anyway, I'd write something like this (untested):

if (*cp)
{
    const unsigned char *end = skipWord (cp);

    if (end > cp && /* avoid creating an empty tag if there was actually no word */
        strncmp (cp, "end", end - cp) != 0 &&
        strncmp (cp, "END", end - cp) != 0)
    {
        vStringNCatS (name, cp, end - cp);
        vStringTerminate (name);
        makeSimpleTag (name, VimKinds, K_AUGROUP);
        vStringClear (name);
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked into Vim's source, and END is caseless:

do_augroup(arg, del_group)
    char_u  *arg;
    int del_group;
{
    …
    else if (STRICMP(arg, "end") == 0)   /* ":aug end": back to group 0 */
        current_augroup = AUGROUP_DEFAULT;

@b4n
Copy link
Member Author

b4n commented Dec 11, 2014

Thanks for digging into this! I addressed your comments in the additional commits, next round can start :)

@blueyed
Copy link
Contributor

blueyed commented Dec 11, 2014

👍
Feel free to merge it.
Thanks!

b4n added a commit that referenced this pull request Dec 11, 2014
@b4n b4n merged commit 8918884 into universal-ctags:master Dec 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants