rewrite keymap system, introduce transient-like functionality#2100
rewrite keymap system, introduce transient-like functionality#2100mahmoodsh36 wants to merge 63 commits intolem-project:mainfrom
Conversation
|
there are still things to be done such as:
|
d980ec3 to
e3f6d6a
Compare
|
i need to stop the habit of force pushing, lol |
|
i introduced the following "suffix" values for now:
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. |
…plicit function suffixes
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
first 3 are done. |
9d5f5ec to
30d4ea5
Compare
|
❌ Code Contractor Validation: FAILED 📋 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:
📚 About Code ContractorDeclarative Code Standards That Learn and Improve Define domain-specific validation rules in YAML. Want this for your repo? |
| @@ -0,0 +1,77 @@ | |||
| (in-package :lem/transient) | |||
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) | ||
|
|
There was a problem hiding this comment.
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* | |||
There was a problem hiding this comment.
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.
|
rebased on main, tho there wasnt really a necessary reason to |
|
The PR being ready is great news. IIRC you said the breaking changes are minimal, mostly name changes.
that would be nice. Also we kinda need a document that explains what are these hard renames. Aaaaand we kinda need a README for the transient system. I see there's a demo.lisp, but we need some english text with a presentation and simple examples (and screenshots). Can you do this? I want to try everything, and we'll ping cxxxr for review. |
| (when (or (typep suffix 'keymap) | ||
| (typep suffix 'prefix)) | ||
| (f suffix))))) | ||
| (f (node) |
There was a problem hiding this comment.
I'd avoid single-letters function names.
| (when (or (typep suffix 'keymap) | ||
| (typep suffix 'prefix)) | ||
| (f suffix))))))) | ||
| (f keymap))) |
There was a problem hiding this comment.
there are no more labels so was it necessary??
| (funcall (fdefinition (list 'setf property-method)) props object)))))))) | ||
|
|
||
| (defun parse-transient (bindings) | ||
| "defines a transient menu. args yet to be documented." |
There was a problem hiding this comment.
documentation (at least first parts) welcome.
|
notable renames
defining keys remains the same, but the internal code in it would be easy to maintain backwards compatibility for things like the |
ee24264 to
6526f3c
Compare
|
Thank you for the readme and the small updates. This PR looks good to me. Let's have a couple more testers?! |

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:
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).