fix(lisp-syntax): don't apply macro indentation to keyword symbols#2228
Merged
theangelperalta merged 1 commit intoJun 14, 2026
Merged
Conversation
When looking up an indentation method, the leading colon of a keyword symbol (e.g. :use) was stripped as if it were a package prefix, so a user-defined function or macro with the same name (e.g. use) incorrectly governed the indentation of that keyword clause. Only strip the package prefix when an actual (non-empty) package name precedes the colon, leaving keywords untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
✅ Code Contractor Validation: PASSED 📋 Contract Configuration: contract (Source: Repository)version: 2
trigger:
paths:
- "extensions/**"
- "frontends/**/*.lisp"
- "src/**"
- "tests/**"
- "contrib/**"
- "**/*.asd"
head_branches:
exclude:
- 'revert-*'
validation:
limits:
max_total_changed_lines: 400
max_delete_ratio: 0.5
max_files_changed: 10
severity: warning
ai:
system_prompt: |
You are a senior Common Lisp engineer reviewing code for Lem editor.
Lem is a text editor with multiple frontends (ncurses, SDL2, webview).
Focus on maintainability, consistency with existing code, and Lem-specific conventions.
rules:
# === File Structure ===
- name: defpackage_rule
prompt: |
First form must be `defpackage` or `uiop:define-package`.
Package name should match filename (e.g., `foo.lisp` → `:lem-ext/foo` or `:lem-foo`).
Extensions must use `lem-` prefix (e.g., `:lem-python-mode`).
- name: file_structure_rule
prompt: |
File organization (top to bottom):
1. defpackage
2. defvar/defparameter declarations
3. Key bindings (define-key, define-keys)
4. Class/struct definitions
5. Functions and commands
# === Style ===
- name: loop_keywords_rule
prompt: |
Loop keywords must use colons: `(loop :for x :in list :do ...)`
NOT: `(loop for x in list do ...)`
- name: naming_conventions_rule
prompt: |
Naming conventions:
- Functions/variables: kebab-case (e.g., `find-buffer`)
- Special variables: *earmuffs* (e.g., `*global-keymap*`)
- Constants: +plus-signs+ (e.g., `+default-tab-size+`)
- Predicates: -p suffix for functions (e.g., `buffer-modified-p`)
- Do NOT use -p suffix for user-configurable variables
# === Documentation ===
- name: docstring_rule
prompt: |
Required docstrings for:
- Exported functions, methods, classes
- `define-command` (explain what the command does)
- Generic functions (`:documentation` option)
Important functions should explain "why", not just "what".
severity: warning
# === Lem-Specific ===
- name: internal_symbol_rule
prompt: |
Use exported symbols from `lem` or `lem-core` package.
Avoid `lem::internal-symbol` access.
If internal access is necessary, document why.
- name: error_handling_rule
prompt: |
- `error`: Internal/programming errors
- `editor-error`: User-facing errors (displayed in echo area)
Always use `editor-error` for messages shown to users.
- name: frontend_interface_rule
prompt: |
Frontend-specific code must use `lem-if:*` protocol.
Do not call frontend implementation directly from core.
severity: warning
# === Functional Style ===
- name: functional_style_rule
prompt: |
Prefer explicit function arguments over dynamic variables.
Avoid using `defvar` for state passed between functions.
Exception: Well-documented cases like `*current-buffer*`.
- name: dynamic_symbol_call_rule
prompt: |
Avoid `uiop:symbol-call`. Rethink architecture instead.
If unavoidable, document the reason.
# === Libraries ===
- name: alexandria_usage_rule
prompt: |
Alexandria utilities allowed: `if-let`, `when-let`, `with-gensyms`, etc.
Avoid: `alexandria:curry` (use explicit lambdas)
Avoid: `alexandria-2:*` functions not yet used in codebase
# === Macros ===
- name: macro_style_rule
prompt: |
Keep macros small. For complex logic, use `call-with-*` pattern:
```lisp
(defmacro with-foo (() &body body)
`(call-with-foo (lambda () ,@body)))
```
Prefer `list` over backquote outside macros.📚 About Code ContractorDeclarative Code Standards That Learn and Improve Define domain-specific validation rules in YAML. Want this for your repo? |
Collaborator
|
Thank you for this fix @skyizwhite 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When computing indentation,
find-indent-methodstrips the package prefix of a package-qualified symbol (e.g.alexandria:with-gensyms→with-gensyms) so it can look up an indentation rule by the bare symbol name.The previous regex
(?<=:)[^:]+had no start anchor, so it also matched the part after the leading colon of a keyword symbol. For:useit returned"use". If the user (or a loaded system) had defined a macro namedusewith a&body-style indentation rule, that rule was incorrectly applied to the:useclause ofdefpackage, breaking its indentation.This replaces the lookbehind with an anchored, capturing regex
^[^:]+:{1,2}([^:].*)$that requires a non-empty package prefix before the:/::separator. Keywords (which start with:and thus have no prefix) and unqualified symbols are now left untouched, while genuine package-qualified symbols still resolve correctly.Test
Adds
defpackage-use-clause-not-affected-by-use-macrototests/lisp-syntax/indent-test.lisp. It temporarily registers a&bodyindentation rule for the nameuse, then asserts that re-indenting adefpackageform with a:useclause leaves it unchanged. The registration is restored viaunwind-protectto avoid leaking global indent-table state into other tests.Verified the new test fails against the old regex (
:use→"use") and passes with the fix; the fulllem-tests/lisp-syntax/indent-testsuite is green.