[C] Improve Goto Definition and Goto Reference #1831

ismell · 2019-01-09T19:06:33Z

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 · 2019-01-23T14:49:58Z

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

ismell · 2019-01-31T17:47:21Z

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

ismell · 2019-02-27T16:52:02Z

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

So correctly fixing this involved a lot more changes then I expected. Fixes sublimehq#1830 Signed-off-by: Raul E Rangel <rrangel@chromium.org>

ismell · 2019-03-04T18:02:26Z

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.

I chose to mark the struct name in a typedef as an entity because it's still possible to use `struct MyStruct` in code so we should be able to navigate to the symbol if it's referenced that way.

ismell · 2019-03-05T16:50:15Z

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.

The struct|union|enum keyword scopes now act like any other type. This reduces the number of specific scopes required.

ismell · 2019-03-11T19:09:31Z

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 · 2019-03-11T20:10:04Z

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 · 2019-03-11T21:01:03Z

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.

This doesn't support disabling a block with `#if 0`. That requires that we redefine `preprocessor-data-structures` since enum body has a different context.

linux kernel defines __macro as a prprocessor macro for setting attributes. I also found this style in other firmware type C code.

We don't want to gobble up actual function definitions.

Also made it so the assignment expressions highlight constants.

I think the function params block might need to be rewritten. It can't handle the case: fn(int var __attribute__((unused).

These are very helpful when debugging the syntax. If you run into problems, revert this so you know what the context chain looks like. Signed-off-by: Raul E Rangel <rrangel@chromium.org>

ismell · 2019-12-16T19:01:23Z

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

jrappen · 2021-08-18T17:58:32Z

@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 · 2021-11-16T15:10:37Z

Oh no! Merge conflicts. 😞

ismell · 2021-11-17T20:03:12Z

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 · 2021-11-18T05:22:29Z

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.

Had to fix some integer naming changes.

Conflicts: C++/C.sublime-syntax C++/syntax_test_c.c Adapted to the new structure and updated tests.

My patch already had robust typedef support. I just added the additional test case.

ismell · 2021-11-19T23:34:16Z

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

This file never conflicted in the chain of merges, so the merge algorithm missed it somehow. Signed-off-by: Raul E Rangel <rrangel@chromium.org>

ismell · 2021-11-20T00:13:03Z

All tests are passing.

ismell · 2021-12-21T16:11:55Z

Can one of the new reviewers take a look?

ismell · 2022-02-08T20:43:57Z

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 · 2022-02-09T16:58:21Z

C++/C.sublime-syntax

+          comment: Struct definition
+          set: struct-body
+        - match: '{{identifier}}'
+          scope: support.type.c


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 · 2022-02-09T16:59:36Z

C++/C.sublime-syntax

@@ -395,23 +437,43 @@ contexts:

  global:
    - include: early-expressions
-    - match: '^\s*(?=\w+)'
-      push: global-modifier


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

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

ismell · 2022-02-09T17:24:20Z

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 · 2022-02-09T18:00:09Z

Merging requires approval from at least 2 authorized contributors.

ismell force-pushed the fix-1830 branch from 82c3be6 to 2b81840 Compare January 9, 2019 19:18

rwols previously approved these changes Jan 23, 2019

View reviewed changes

ismell force-pushed the fix-1830 branch from 2b81840 to 60b1ad7 Compare January 31, 2019 17:43

[C] Refactor struct handling

e0e37d7

So correctly fixing this involved a lot more changes then I expected. Fixes sublimehq#1830 Signed-off-by: Raul E Rangel <rrangel@chromium.org>

ismell force-pushed the fix-1830 branch from 60b1ad7 to e0e37d7 Compare March 4, 2019 17:54

Fix typedef with struct

26d0b65

I chose to mark the struct name in a typedef as an entity because it's still possible to use `struct MyStruct` in code so we should be able to navigate to the symbol if it's referenced that way.

Simplify all the data structure handling

ebbf5a2

The struct|union|enum keyword scopes now act like any other type. This reduces the number of specific scopes required.

Fix highlighting for macro function calls

9f1e464

enums need to support pre-processor as well

fa0f473

This doesn't support disabling a block with `#if 0`. That requires that we redefine `preprocessor-data-structures` since enum body has a different context.

This was referenced Mar 18, 2019

Goto Symbol in Project is slow for large projects sublimehq/sublime_text#2672

Closed

[C] Falsely marking macro closing parentheses as red when using braces #1913

Open

Raul E Rangel added 11 commits April 1, 2019 12:28

Fixed preprocessor attributes

9a2c107

linux kernel defines __macro as a prprocessor macro for setting attributes. I also found this style in other firmware type C code.

Fix function pointer parameter

8022504

Fix function pointer return type highlighting.

eb4283c

Limit the __attr to only single parameters

81217fc

We don't want to gobble up actual function definitions.

Scope enum values as entity.name.constant

ba713d9

Also made it so the assignment expressions highlight constants.

Add support for function attributes

3614d1c

Add more debug scopes to preprocessor macros

5d3efbf

Fix macro as part of variable definition.

b307a83

Allow macro calls to define variable/function type

a0ebd41

Support attributes in method params

88700a9

I think the function params block might need to be rewritten. It can't handle the case: fn(int var __attribute__((unused).

Fix more pre-processor macro invocations.

8969bdc

Remove debug scopes

f500961

These are very helpful when debugging the syntax. If you run into problems, revert this so you know what the context chain looks like. Signed-off-by: Raul E Rangel <rrangel@chromium.org>

This was referenced Mar 6, 2020

[C] Bug with “register” keyword #2047

Open

[RFC] Scopes for module/namespace access #1842

Open

wbond added significant and removed changes requested labels Jul 22, 2020

keith-hall dismissed rwols’s stale review via f500961 August 17, 2021 07:53

michaelblyons mentioned this pull request Nov 16, 2021

Autocomplete missing some indexed functions sublimehq/sublime_text#5041

Open

Raul E Rangel added 8 commits November 19, 2021 15:25

Merge commit '16a0a25860f5f4f91de96bc61c8e73d498ec4422' into fix-1830

e55e7a0

Merge commit 'a7bf2794cb34236da3b21e8b94774f3d08de75f2' into fix-1830

6d9cfbe

Had to fix some integer naming changes.

Merge commit 'b045d4f5703e76c8bf09b0d35428d3117d9d0511' into fix-1830

93b4d38

Conflicts: C++/C.sublime-syntax C++/syntax_test_c.c Adapted to the new structure and updated tests.

Merge commit 'cb5ceb5d5ffa871ea7126c2cc2503e40b9ed9f63' into fix-1830

7b11c8e

Merge commit '4d12d2f946085cd992eef79d1dd048e94b9a5621' into fix-1830

c12ac17

Merge commit '4b3204da059b29bd34dc9a3107d3ca064555a671' into fix-1830

aedd70c

My patch already had robust typedef support. I just added the additional test case.

Merge commit '3629e8bce1d25fbbc66528ed212f94597cbb8800' into fix-1830

4e533b9

Merge remote-tracking branch 'upstream/master' into fix-1830

344be70

Fix broken cpp test after merge

36561ed

This file never conflicted in the chain of merges, so the merge algorithm missed it somehow. Signed-off-by: Raul E Rangel <rrangel@chromium.org>

maximzakharov pushed a commit to maximzdev/Packages that referenced this pull request Nov 20, 2021

Pull request sublimehq#1831 and fixes for enum in header files works

49f3884

deathaxe reviewed Feb 9, 2022

View reviewed changes

michaelblyons mentioned this pull request Feb 8, 2024

[C] Add more scopes for variable assignment #3920

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[C] Improve Goto Definition and Goto Reference #1831

[C] Improve Goto Definition and Goto Reference #1831

ismell commented Jan 9, 2019 •

edited

wbond commented Jan 23, 2019

ismell commented Jan 31, 2019

ismell commented Feb 27, 2019

ismell commented Mar 4, 2019

ismell commented Mar 5, 2019

ismell commented Mar 11, 2019

ismell commented Mar 11, 2019

ismell commented Mar 11, 2019

ismell commented Dec 16, 2019

jrappen commented Aug 18, 2021

michaelblyons commented Nov 16, 2021

ismell commented Nov 17, 2021

jrappen commented Nov 18, 2021

ismell commented Nov 19, 2021

ismell commented Nov 20, 2021

ismell commented Dec 21, 2021

ismell commented Feb 8, 2022

deathaxe left a comment

deathaxe Feb 9, 2022

deathaxe Feb 9, 2022

ismell Feb 9, 2022

ismell commented Feb 9, 2022

deathaxe commented Feb 9, 2022

[C] Improve Goto Definition and Goto Reference #1831

Are you sure you want to change the base?

[C] Improve Goto Definition and Goto Reference #1831

Conversation

ismell commented Jan 9, 2019 • edited

Before

After

wbond commented Jan 23, 2019

ismell commented Jan 31, 2019

ismell commented Feb 27, 2019

ismell commented Mar 4, 2019

ismell commented Mar 5, 2019

ismell commented Mar 11, 2019

ismell commented Mar 11, 2019

ismell commented Mar 11, 2019

ismell commented Dec 16, 2019

jrappen commented Aug 18, 2021

michaelblyons commented Nov 16, 2021

ismell commented Nov 17, 2021

jrappen commented Nov 18, 2021

ismell commented Nov 19, 2021

ismell commented Nov 20, 2021

ismell commented Dec 21, 2021

ismell commented Feb 8, 2022

deathaxe left a comment

Choose a reason for hiding this comment

deathaxe Feb 9, 2022

Choose a reason for hiding this comment

deathaxe Feb 9, 2022

Choose a reason for hiding this comment

ismell Feb 9, 2022

Choose a reason for hiding this comment

ismell commented Feb 9, 2022

deathaxe commented Feb 9, 2022

ismell commented Jan 9, 2019 •

edited