fix: describe-bindings freeze and crash#2190
Merged
theangelperalta merged 3 commits intoMay 19, 2026
Merged
Conversation
Two regressions from PR lem-project#2100: - help.lisp: describe-bindings-internal looped forever because the (setf keymap (keymap-parent keymap)) advance was removed but the (loop :while keymap ...) was left in place. Terminate after one iteration with (setf keymap nil). - keymap.lisp: traverse-keymap yielded transient closures (toggle, choice infixes) to callbacks, causing (symbol-name command) to signal a type error in describe-bindings-internal. Guard the callback at the contract boundary by gating on (symbolp suffix).
Contributor
|
✅ Code Contractor Validation: PASSED ℹ️ 1 violation(s) dismissed 📚 About Code ContractorDeclarative Code Standards That Learn and Improve Define domain-specific validation rules in YAML. Want this for your repo? |
- traverse-keymap-skips-non-symbol-suffixes: builds a keymap whose prefix suffix is a closure (the shape produced by transient toggle/choice infixes) and asserts traverse-keymap only yields symbol commands. - describe-bindings-internal-terminates: end-to-end check that the function returns (with a bordeaux-threads timeout guard) on a keymap containing a closure suffix, and that the rendered output includes the symbol binding.
![code-contractor-app[bot]](https://avatars.githubusercontent.com/in/2165592?s=60&v=4){kind=small}
Adds rationale docstrings to both deftest forms describing the regressions they guard against, and an inline comment explaining why the test reaches lem-core/commands/help::describe-bindings-internal through the internal symbol path.
code-contractor-app
Bot
dismissed
their stale review
All violations have been dismissed via resolved conversations.
Catsquotl
pushed a commit
to Catsquotl/lem
that referenced
this pull request
May 31, 2026
* fix: describe-bindings freeze and crash after keymap rewrite Two regressions from PR lem-project#2100: - help.lisp: describe-bindings-internal looped forever because the (setf keymap (keymap-parent keymap)) advance was removed but the (loop :while keymap ...) was left in place. Terminate after one iteration with (setf keymap nil). - keymap.lisp: traverse-keymap yielded transient closures (toggle, choice infixes) to callbacks, causing (symbol-name command) to signal a type error in describe-bindings-internal. Guard the callback at the contract boundary by gating on (symbolp suffix). * test: cover describe-bindings/traverse-keymap regressions - traverse-keymap-skips-non-symbol-suffixes: builds a keymap whose prefix suffix is a closure (the shape produced by transient toggle/choice infixes) and asserts traverse-keymap only yields symbol commands. - describe-bindings-internal-terminates: end-to-end check that the function returns (with a bordeaux-threads timeout guard) on a keymap containing a closure suffix, and that the rendered output includes the symbol binding. * test: docstring deftests; document internal access Adds rationale docstrings to both deftest forms describing the regressions they guard against, and an inline comment explaining why the test reaches lem-core/commands/help::describe-bindings-internal through the internal symbol path.
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
1 participant
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.
Fix:
describe-bindingsfreezes and crashesSummary
Running
M-x describe-bindingswould freeze LEM (infinite loop) and, once that was fixed, crash with a type error on buffers with transient keybindings.Root Cause
Two bugs, both regressions from PR #2100 ("rewrite keymap system, introduce transient-like functionality"), merged 2026-04-24.
Bug 1: Infinite loop in
describe-bindings-internalFile:
src/commands/help.lispCommit
3793971cremoved the line(setf keymap (keymap-parent keymap))from the loop body indescribe-bindings-internalbecausekeymap-parentno longer existed after the keymap rewrite. However, the surrounding(loop :while keymap ...)was left in place, sokeymapwas never set toniland the loop never terminated.Fix: Added
(setf keymap nil)at the end of the loop body to terminate after one iteration.Bug 2: Type error on transient closures in
traverse-keymapFile:
src/keymap.lispThe transient keymap system allows
prefix-suffixto return closures (fortoggleandchoiceinfixes) rather than command symbols.traverse-keymappassed these to its callback unconditionally, and callers likedescribe-bindings-internalcalled(symbol-name command)on the result — which fails on function objects with:The value #<FUNCTION ...> is not of type SYMBOL.Fix: Changed the catch-all
tbranch intraverse-prefix(withintraverse-keymap) to(symbolp suffix), so only symbol commands are yielded to callbacks. This protects all callers at the contract boundary rather than requiring guards at each call site.Files Changed
src/commands/help.lisp(setf keymap nil)to terminate the loopsrc/keymap.lisptraverse-keymapto skip non-symbol suffixes