Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 590d12dd36
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (std.mem.indexOf(u8, stripped, "(")) |paren| { | ||
| if (paren > 0) { | ||
| // Skip control flow keywords | ||
| const control = [_][]const u8{ "if ", "if(", "else ", "for ", "for(", "foreach ", "foreach(", "while ", "while(", "switch ", "switch(", "catch ", "catch(", "return ", "return(", "new ", "throw ", "lock ", "lock(" }; |
There was a problem hiding this comment.
This branch marks almost any parenthesized C# line as a .method symbol, so ordinary calls like Console.WriteLine(...), LINQ calls, or constructor invocations inside method bodies are indexed as declarations. That will significantly inflate symbol lists and degrade outline/search accuracy on real C# files, even though no new declarations exist on those lines. Method extraction needs a declaration-specific guard instead of relying on ( plus a small control-keyword denylist.
Useful? React with 👍 / 👎.
| if (startsWith(line, "using ")) { | ||
| const symbol_copy = try a.dupe(u8, line); | ||
| errdefer a.free(symbol_copy); | ||
| try outline.symbols.append(a, .{ |
There was a problem hiding this comment.
The parser treats any line beginning with using as an import and appends it to outline.imports, but C# also uses using for local resource-management statements (using var ... / using (...)). Those common in-method statements will be recorded as dependencies, which corrupts import/dependency data and downstream getImportedBy results for C# files.
Useful? React with 👍 / 👎.
Cherry-picked from justrach/codedb PR justrach#106 (unliftedq). Adds parseCSharpLine with support for using directives, namespaces, classes, interfaces, structs, enums, delegates, constants, and methods. Includes stripCSharpModifiers and lastIdent helpers. Co-Authored-By: unliftedq <unliftedq@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hey @unliftedq, thanks for adding C# support — the parser itself looks solid (using directives, namespaces, classes, interfaces, structs, enums, delegates, methods, constants with modifier stripping). Appreciate the contribution! One issue though — the PR includes ~262 lines of whitespace reformatting on existing methods (
This reformatting creates massive merge conflicts with every other open PR (we just merged PHP support in #87 and are about to merge HCL in #109), and makes it very hard to review the actual C# changes. Could you resubmit with only the C# parser additions? Specifically just:
No reformatting of existing code — keep the original indentation as-is. That way it'll rebase cleanly and we can get it merged quickly. Thanks again for the work! |
unliftedq
force-pushed
the
add-csharp-support
branch
from
590d12d to
4aaa6f0
Compare
It should be fixed now, previous unrelated change is unexpectedly introduced by the zig language server of my local zig extension. |
|
@justrach could you please take a look? |
justrach
left a comment
justrach
left a comment
There was a problem hiding this comment.
Thanks for adding C# support and for cleaning up the unrelated whitespace churn.
I still see two blocking parser correctness issues on the current head in parseCSharpLine():
- lines starting with
usingare still treated as imports unconditionally, so localusing var .../using (...)statements will polluteoutline.importsand dependency results - the generic parenthesis branch is still broad enough to classify ordinary call expressions as
.methodsymbols instead of declarations
Please tighten the declaration guard for methods and distinguish using directives from in-method using statements, then add or update tests for both cases. After that this should be much closer.
2 participants
As title, Added basic C# support to the explorer.