Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vim: Cleanup word matching code #146

Merged
merged 3 commits into from
Dec 11, 2014

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.

@@ -286,7 +286,9 @@ static void parseAutogroup (const unsigned char *line)

if (*cp)
{
if (strncasecmp ((const char*) cp, "end", (size_t) 3) != 0)
if (/* FIXME: does this really has to be caseless? Also, can't it
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