Skip to content

Conversation

@y0m0
Copy link

@y0m0 y0m0 commented Jan 23, 2026

Hi, first of all thanks a lot for the amazing work with the al plugin for treesitter.
Finally I can try to have a fully working AL setup for neovim in combination with al.nvim
With that said the al.nvim project included the queries which were missing from this repo, but in my opinion it's more natural for them to be part of the al treesitter.

I added queries for:

  • highlights
  • locals
  • folds (not fully working yet)
  • indents (not fully working yet)

Here's the healthcheck for treesitter:
image

And here's how the highlight works for an .al file in neovim:
image

y0m0 added 5 commits January 23, 2026 14:07
1. Added a new inline helper function match_keyword_case_insensitive() at line ~250:
   - Takes an identifier buffer, length, and keyword string
   - Performs case-insensitive comparison
   - Returns boolean result
   - Uses standard C99/C11 syntax compatible with MSVC
2. Replaced two problematic macros that used GNU C statement expressions (({ ... })):
   - MATCH_KW macro (line ~869-881) → replaced with function call
   - MATCH_KEYWORD macro (line ~1271-1283) → replaced with function call
Results
- Build: ✅ Compiles successfully with MSVC on Windows (no errors, no warnings)
- Tests: ✅ All 1198 tests passing
- Compatibility: The code now works with both MSVC and GCC/Clang compilers
- Functionality: Parser behavior unchanged - all existing tests pass
Technical Details
Problem: The original code used GNU C statement expressions (a GCC/Clang extension):
    bool _m = (strlen(term) == id_len); \
    /* ... */ \
    _m; \
})
This syntax is not supported by MSVC, causing compilation errors like:
- error C2059: syntax error: '{'
- error C2065: '_m': undeclared identifier
Solution: Converted to a standard C static inline function:
static inline bool match_keyword_case_insensitive(
    const char *identifier,
    int id_len,
    const char *keyword
) {
    // Standard C implementation
}
This approach:
- Works on all C99+ compilers (MSVC, GCC, Clang)
- Maintains performance (inlined by compiler)
- Improves code maintainability and debuggability
The tree-sitter parser is now fully compatible with Windows MSVC build environments!
Copy link
Owner

@SShadowS SShadowS left a comment

Choose a reason for hiding this comment

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

Code Review: Query Files for Editor Integration

Thanks for this contribution! Adding query files significantly improves editor support for AL. The coverage is comprehensive and the documentation within the files is excellent.

✅ What's Good

  • Comprehensive coverage - All AL object types, sections, and constructs are covered
  • Well-documented - Clear section headers and explanatory comments
  • Scanner fix - Replacing GCC statement expressions (({ })) with match_keyword_case_insensitive() improves portability (MSVC compatibility)
  • README updates - Good documentation of the new query files

⚠️ Issues to Address

1. Keyword captures are too broad (High Priority)

Many patterns capture entire nodes instead of specific keyword tokens:

; Current - highlights entire declaration including body
(table_declaration) @keyword
(if_statement) @keyword.control

; Should be - highlights only the keyword token
(table_declaration "table" @keyword)
(if_statement "if" @keyword.control)
(if_statement "then" @keyword.control)
(if_statement "else" @keyword.control)

This affects most entries in the "KEYWORDS" sections of highlights.scm.

2. Conflicting indent rules (Medium Priority)

indents.scm has three different captures for }:

"}" @indent_end
"}" @dedent
"}" @outdent

Most editors expect only one pattern. Recommend keeping just @indent / @dedent (nvim-treesitter convention).

3. package-lock.json regression (Medium Priority)

The PR downgrades tree-sitter-cli from 0.24.4 to 0.24.3. This appears unintentional - please revert.

4. AL scoping semantics (Low Priority)

In locals.scm:

(code_block) @local.scope
(if_statement) @local.scope

AL has procedure-level variable scoping, not block scoping. Variables in a var section are visible throughout the entire procedure. This could cause incorrect scope highlighting, though it may be acceptable for initial implementation.

5. README test count (Low Priority)

README says "1,198 tests" but current count is 1,194. Minor, but worth correcting.

💡 Suggestions

  1. Consider adding a few test .al files with expected highlighting to catch regressions
  2. For the "not fully working" folds/indents, consider adding TODO comments noting specific issues

Verdict

This is a valuable addition! The main blocker is the overly broad keyword captures which will cause entire code blocks to be highlighted as keywords. Once that's fixed, this should be good to merge.

Happy to help test or review a follow-up commit!

@y0m0 y0m0 marked this pull request as draft January 26, 2026 08:36
y0m0 added 9 commits January 26, 2026 10:44
Grammar change: parameter_list now includes '(' and ')' like argument_list,
enabling tree-sitter indent rules to properly indent first parameter in
multi-line parameter lists.

Changes:
- Modified parameter_list rule to include parens (seq('(', ..., ')'))
- Updated 10 locations in grammar.js that used '(' optional(parameter_list) ')'
- Simplified indent rules for parameter_list (removed workaround comments)
- Updated 278 test expectations (structural changes only, no ERRORs)
- Moved indent test files to test_data/indents/ to avoid query file conflicts

Results:
- Parameter indentation now works correctly (8 spaces instead of 4)
- Indent diff lines reduced from 18 to 16 (remaining are if-else patterns)
- All 1198 tests pass
- Parser size: ~36MB

[Indentation: 16 diff lines, 99.14% success rate]
- Updated success rate to 99.14% (16 diff lines)
- Documented Phase 4: parameter_list grammar change
- Added testing instructions and file locations
- Clarified remaining if-else limitation
Expose the 'else' keyword as a separate node in the parse tree by
creating an else_clause rule (following Rust's approach). This allows
tree-sitter indent queries to target the 'else' keyword for proper
dedentation.

Changes:
- grammar.js: Add else_clause rule wrapping 'else' keyword and body
- grammar.js: Modify if_statement to use optional($.else_clause)
- queries/indents.scm: Update indent rules to use else_clause
- test/corpus/*.txt: Update 4 test files for structural changes

Results:
- 1198 parser tests passing
- Indentation diff reduced from 16 to 2 lines (spacing only)
- If-else patterns now indent correctly

[BC.History: 80 errors, 99.48% success]
Documented Phase 5 (else_clause grammar fix) which reduced diff
lines from 16 to 2. Only remaining issue is spacing, not indentation.

[BC.History: 80 errors, 99.48% success]
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