Skip to content

rewrite keymap system, introduce transient-like functionality#2100

Open
mahmoodsh36 wants to merge 55 commits intolem-project:mainfrom
mahmoodsh36:transient
Open

rewrite keymap system, introduce transient-like functionality#2100
mahmoodsh36 wants to merge 55 commits intolem-project:mainfrom
mahmoodsh36:transient

Conversation

@mahmoodsh36
Copy link
Copy Markdown
Contributor

this is very preliminary work. this is an opinionated rewrite of the keymap system that i think makes more sense and is more intuitive.
yet to be handled/written/re-written:

  • undefine-key
  • priorities (some stuff is broken)
  • proper transient-like functionality with proper grid displays and self-documenting keybindings
    the keymap system now allows for "dynamic" (non-static) keys or keymaps to be bound on-demand. unlike the old implementation which had nested pre-defined hashmaps, the new implementation uses a different data structure but tries to maintain backwards compatibility with the old implementation (atleast for now).
    i will be following this PR with more work and documentation/explanations. the goal was to rewrite the keymap system so that things like transient or which-key are a natural consequence of the data structure and are more intertwined with the core (unlike in emacs).

@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

mahmoodsh36 commented Jan 18, 2026

Screen-2026-01-18_21 23 33 an initial preview of the popup thingy. the double instances of unnamed keymap thing is intentional since its displaying nested keymaps.

@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

there are still things to be done such as:

  • implement "sticky" keys such as infixes
  • fix keymap priorities/precedence
  • implement undefine-key
  • some key sequences should be considered as a single prefix, such as keys "-g" that resemble commandline arguments and are composed of 2 keys not only one. this is currently not possible because a popup only displays single keys and doesnt recurse to display the longer key sequence which is the intended design, but maybe we should also allow for some sub-keymaps to be recursed into and longer key sequences to be displayed.
  • functionality isnt yet customizable beside the CLOS interface. more customization variables should be introduced that customize behavior.

@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

i need to stop the habit of force pushing, lol

@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

i introduced the following "suffix" values for now:

  • :cancel to drop the current key sequence entirely without invoking a command
  • :drop to avoid adding the current key to the key sequence, which makes the prefix act as an "infix" key
  • :back to avoid adding the current key and to pop the last recorded key which has the effect of "going back" to parent menu in the transient popup.

this behavior is customized by setting the value of the suffix to one of those (or to a symbol that is resolved later to a command that is executed). i dont think this belongs in the suffix slot, i think it would be more ideal for a suffix to always resolve to a command, but currently the function 'read-command' which is part of the event/key handler can only resolve a specific key sequence to a single command. currently the behavior of an infix (such as 'choice') is customized by overriding 'prefix-invoke' but this functionality i think belongs in the suffix itself. i have yet to think of a redesign to make this more intuitive.

i think the suffix itself shouldnt necessarily hold the special value that decides the behaviors cancel/drop/back, i think that belongs in a different defmethod (call it prefix-behavior) so that a prefix can both resolve to a command and have custom behavior.

before this, after typing / and trying to search, the following keys
would be interpreted as if they were entered in normal mode instead, and
searching functionality was broken. this is more of a bandaid than a
fix, i need to rewrite the whole 'undef-hook' thing which i think is
annoying and unintuitive
@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

mahmoodsh36 commented Jan 30, 2026

there are still things to be done such as:

  • implement "sticky" keys such as infixes
  • fix keymap priorities/precedence
  • implement undefine-key
  • some key sequences should be considered as a single prefix, such as keys "-g" that resemble commandline arguments and are composed of 2 keys not only one. this is currently not possible because a popup only displays single keys and doesnt recurse to display the longer key sequence which is the intended design, but maybe we should also allow for some sub-keymaps to be recursed into and longer key sequences to be displayed.
  • functionality isnt yet customizable beside the CLOS interface. more customization variables should be introduced that customize behavior.

first 3 are done.

the idea of "dynamic" properties/slots that i had before i think was
redundant and just overcomplicated things. this is a refactors things.
now we just rely on CLOS even for instance-specific "overrides" in
keymaps/prefixes.
@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

whats left to be done:

  • turn lambda suffixes into commands. together with rewriting the event/key-reading loop to make help keys work properly. (currently running help-key on a key sequence that is bound to a lambda doesnt function like you'd expect).
  • introduce deprecation warnings (some things were renamed and i should try not to completely break compatibility).
  • implement more types of prefixes.

currently, the keymap structure uses different concepts of a 'hierarchy' for different use cases, such as a hierarchy for keymap precedence, another for keymap inheritance, and another for keymap prefixing. the 3rd is naturally distinct but the first 2 have some form of relation and im thinking of a way to unify them (and possibly turning keymaps into types instead of just instances).

i think this is about it. this functionality is already usable even at this stage.

@mahmoodsh36
Copy link
Copy Markdown
Contributor Author

main thing thats left is to rewrite the event loop to make things more "natural". currently suffixes can be lambdas but they should be commands instead. to fix this i'd want the event reading loop rewritten.
but the code is already usable as-is, this one thing aside.
deprecation warnings instead hard renames/rewrites would be nice too.

@mahmoodsh36 mahmoodsh36 marked this pull request as ready for review March 14, 2026 22:02
@code-contractor-app
Copy link
Copy Markdown
Contributor

code-contractor-app bot commented Mar 14, 2026

❌ Code Contractor Validation: FAILED

=== Contract: contract ===

✓ Code Contractor Validation Result
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

📋 Contract Source: Repository

📊 Statistics:
  Files Changed:    35
  Lines Added:      1629
  Lines Deleted:    184
  Total Changed:    1813
  Delete Ratio:     0.10 (10%)

Status: FAILED ❌

🤖 AI Providers:
  - codex — model: gpt-5-mini

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
⚠️ Violations Found (7):

[ERROR] max_files_changed
  Too many files changed: 35 > 10
  Limit: 10
  Actual: 35

[ERROR] max_total_changed_lines
  Too many lines changed: 1813 > 400
  Limit: 400
  Actual: 1813

[ERROR] defpackage_rule
  AI check failed: "defpackage_rule"
  ❌ Reason:
    New files use `IN-PACKAGE` or other first forms instead of a top-level
    `DEFPACKAGE` or `UIOP:DEFINE-PACKAGE` as required.

[ERROR] loop_keywords_rule
  AI check failed: "loop_keywords_rule"
  ❌ Reason:
    Added code uses LOOP with unprefixed keywords (e.g. "for", "in", "do")
    instead of colon-prefixed keywords (e.g. ":for", ":in", ":do").

[WARNING] docstring_rule
  AI check failed: "docstring_rule"
  ❌ Reason:
    The diff adds exported symbols (classes, generics, and commands) without
    the required docstrings/documentation. Examples: exported generics lack
    a `:documentation` option, exported classes lack class docstrings, and
    an exported `define-command` (demo-run) is defined without a docstring.
    These violate the rule requiring docstrings for exported functions,
    methods, classes and that generic functions include a `:documentation`
    option.

[ERROR] internal_symbol_rule
  AI check failed: "internal_symbol_rule"
  ❌ Reason:
    Added code contains direct references to internal (non-exported) symbols
    via package double-colon qualifiers (e.g. `lem-core::other-keymaps`),
    which violates the rule to use exported symbols from `lem` or `lem-core`
    and avoid internal symbol access.

[ERROR] functional_style_rule
  AI check failed: "functional_style_rule"
  ❌ Reason:
    The diff adds multiple top-level dynamic state variables with `defvar`
    that appear to be used as shared state across functions, which violates
    the rule to avoid using `defvar` for state passed between functions
    (prefer explicit function arguments).
📋 Contract Configuration: contract (Source: Repository)
version: 2

trigger:
  paths:
    - "extensions/**"
    - "frontends/**/*.lisp"
    - "src/**"
    - "tests/**"
    - "contrib/**"
    - "**/*.asd"

validation:
  limits:
    max_total_changed_lines: 400
    max_delete_ratio: 0.5
    max_files_changed: 10

  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.

💬 Feedback

Reply to a violation comment with:

  • /dismiss <reason> - Report false positive or not applicable
📚 About Code Contractor

Declarative Code Standards That Learn and Improve

Define domain-specific validation rules in YAML.
Your contracts document team knowledge and evolve into more accurate AI enforcement.

Want this for your repo?
Install Code Contractor

Copy link
Copy Markdown
Contributor

@code-contractor-app code-contractor-app bot left a comment

Choose a reason for hiding this comment

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

Code Contractor validation failed ❌ — see the sticky comment for full results.

@@ -0,0 +1,77 @@
(in-package :lem/transient)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Contractor: defpackage_rule

Contract: contract

AI check failed: "defpackage_rule"

Reason:
New files use IN-PACKAGE or other first forms instead of a top-level DEFPACKAGE or UIOP:DEFINE-PACKAGE as required.

(cond
((keymap-show-p keymap)
(show-transient keymap))
((loop for mode in active-modes
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Contractor: loop_keywords_rule

Contract: contract

AI check failed: "loop_keywords_rule"

Reason:
Added code uses LOOP with unprefixed keywords (e.g. "for", "in", "do") instead of colon-prefixed keywords (e.g. ":for", ":in", ":do").

:initarg :suffix
:documentation "the suffix defined for the prefix, could be another prefix or a keymap or a function that returns one.")
(active-p
:initarg :active-p
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Contractor: docstring_rule

Contract: contract

AI check failed: "docstring_rule"

Reason:
The diff adds exported symbols (classes, generics, and commands) without the required docstrings/documentation. Examples: exported generics lack a :documentation option, exported classes lack class docstrings, and an exported define-command (demo-run) is defined without a docstring. These violate the rule requiring docstrings for exported functions, methods, classes and that generic functions include a :documentation option.

(define-key *copilot-completion-keymap* "Tab" 'copilot-accept-suggestion)
(define-key *copilot-completion-keymap* 'copilot-next-suggestion 'copilot-next-suggestion)
(define-key *copilot-completion-keymap* 'copilot-previous-suggestion 'copilot-previous-suggestion)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Contractor: internal_symbol_rule

Contract: contract

AI check failed: "internal_symbol_rule"

Reason:
Added code contains direct references to internal (non-exported) symbols via package double-colon qualifiers (e.g. lem-core::other-keymaps), which violates the rule to use exported symbols from lem or lem-core and avoid internal symbol access.

@@ -0,0 +1,531 @@
(in-package :lem/transient)

(defvar *transient-popup-window*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Contractor: functional_style_rule

Contract: contract

AI check failed: "functional_style_rule"

Reason:
The diff adds multiple top-level dynamic state variables with defvar that appear to be used as shared state across functions, which violates the rule to avoid using defvar for state passed between functions (prefer explicit function arguments).

handling of undef-hook wasnt right which caused C-n/C-p not to work for
prompt completion.
undef-hook is a leftover from previous keymap code and i should think of
a way to rewrite its functionality that is more ideal.
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.

1 participant