[C] Improve Goto Definition and Goto Reference

[ismell]

In my journey to fix #1830 I ended up re-writing a lot of the syntax. It now more accurately scopes entity.name.* and preprocessor macros. My goal is to make Goto Definition and Goto Reference work a lot better then they currently do.

Function parameters are also scoped better, so we should remove source.c from the Monokai Function argument style.

Tests have been greatly improved. I mostly tested this syntax on the linux kernel and various firmware projects.

My goal is to implement #1861. This will require theme changes though. I think we should blacklist entity.name.variable from the Entity name style.

Before

After

[wbond]

Could we add tests for this construct to the other C variants to make sure it get inherited?

[ismell]

Done. I need to find some time to give the C syntax some love.

[ismell]

Don't merge this yet... I have a couple other fixes for this.

[ismell]

So actually fixing struct required a lot more work. I'm pushing up my WIP to get some initial feedback. You can ignore all the debug.* scopes. I will remove those for the final PR. I also haven't fixed the enum, or union keywords completely.That's the reason I have the struct-keyword context added everywhere data-structures is checked. This will eventually be cleaned up once I get around to re-writing union and enum. I want to do that work in another PR thought.

Feature Request, It would be cool if I could enable a syntax developer mode where the context that applied a scope could be determined. Then I could avoid adding all the `debug.* scopes manually.

I was also wondering why the Monokai theme doesn't highlight function arguments?

        {
            "name": "Function argument",
            "scope": "variable.parameter - (source.c | source.c++ | source.objc | source.objc++)",
            "foreground": "var(orange)",
            "font_style": "italic"
        },

I actually almost got them scoped correctly now, so it would be nice to see them highlighted by default.

[ismell]

I fixed typedef used with struct declarations and function pointers. Let me know if I'm missing any other cases. I might start cleaning up union next.

[ismell]

So I simplified my changes. This reduces the number of top level struct scopes to 1 instead of the three I had. I also fixed union and enum. I replaced the existing function/variable declaration mechanism with the one I had written for the struct body. This now correctly tags multiple variable declarations and multiple function prototype declarations.

There are still some failing cases that I need to work through. Thought I wonder how legit they are.

Expressions in a function declaration?

static string foo(bar() + ';');

Macro invocation return types. Should figure out a way to support this.

MACRO_CALL(int) macro_prefixed_func(){}

Function without the ( on the same line. I'm not sure if I want to support this one because it would make things have to be declared as a function by default instead of a variable. I would rather not pollute the function namespace if it isn't actually a function.

MACRO int
funcname2
()
{
}

Still a WIP, but if anyone wants to test it that would be appreciated :)

[ismell]

Looks like the preprocessor-practical-workarounds are causing issues. I don't think they are needed anymore. I'll play around with removing them.

[ismell]

So I fixed the highlighting for macro function calls and macros as types. I think it's looking pretty good except for the couple of edge cases I posted above.

[ismell]

Merged in upstream/master. Should be ready for review again.

[jrappen]

@ismell maybe you could:

• provide some numbers regarding performance changes using Tools > Build with ... > Syntax Tests - Performance?
• re-merge upstream/master to re-run tests with the Build 4112 test binary

[michaelblyons]

Oh no! Merge conflicts. 😞

[ismell]

Yeah, I stopped working on this since Will told me he wanted to fix C++ and ObjC as well instead of just C. FWIW, I've been using this for 2 years now doing linux kernel, coreboot, and other firmware projects and it's pretty solid. If there is actual interest in merging this PR I can make some time to do some minor clean up.

[jrappen]

That would be appreciated. Will isn't working for SublimeHQ anymore. The remaining team seems to be focused on other stuff (currently) and has assigned some collaborators to take care of syntaxes (for them). So this would be useful until somebody tackles a c languages rewrite using inheritance.

[ismell]

Oops, I should have tested the cpp syntax as well. I'll push a fix.

[ismell]

All tests are passing.

[ismell]

Can one of the new reviewers take a look?

[ismell]

Any hopes of getting this merged in? It will unblock others from improving upon it.

[deathaxe]

Maybe not enough package maintainers use C/C++ to feel a value in reviewing. There are other C/C++ related PRs pending.

Another reason might be the long pending idea of fully rewriting C family syntaxes from scratch to reduce duplication by ST4's inheritance feature and to fix all the open issues. The content of this PR would very likely be subject of heavy changes as well then.

I can't find major flaws with a shallow view at the sources, but don't use the syntaxes enough to judge the changes.

[deathaxe]

Now that we use keyword.declaration for struct keyword, storage.type or storage.type.struct for user defined data types. But why do you distinguish declaration from definition? I'd call it entity.name.struct in both cases.

[deathaxe]

Named contexts help with inheritance. So turning them into anonyomous ones feels like a step backward.

[ismell]

Yeah, this was all written before ST4 when the names weren't really useful for debugging. I can pull it out.

[ismell]

Maybe not enough package maintainers use C/C++ to feel a value in reviewing. There are other C/C++ related PRs pending.

Right, there has been a lot of duplicated effort since this PR has sat for so long.

Another reason might be the long pending idea of fully rewriting C family syntaxes from scratch to reduce duplication by ST4's inheritance feature and to fix all the open issues. The content of this PR would very likely be subject of heavy changes as well then.

I agree, it could use some modernization, but getting all the test cases landed would be a huge benefit to aiding in the refactor.

Side note, I have no idea how to use Github's PR interface...

[deathaxe]

Merging requires approval from at least 2 authorized contributors.