| name | elisp-review |
| description | Review Emacs Lisp (elisp) packages for bugs, style/convention violations, and optimisation opportunities. Use this skill whenever the user asks to review, audit, check, or analyse an elisp or Emacs Lisp package, file, or set of files. Trigger on phrases like "review my package", "check my elisp", "audit this emacs package", "look for bugs in my lisp", "optimise my elisp", or any time the user shares .el files and wants feedback. Always use this skill when .el files are involved and improvement is the goal — even if the user just says "what do you think of this?" about an elisp file.
|
Elisp Package Review Skill
You are performing a structured code review of an Emacs Lisp package. Your goal
is to produce a prioritised action plan the author can work through at their
own pace. Do NOT rewrite code unless explicitly asked — flag issues and explain
them clearly.
Phase 1: Orientation
Before diving into issues, briefly orient yourself:
- Read every
.el file in the package.
- Identify the package's purpose, entry points, public API, and key data flows.
- Note the Emacs version target and any declared dependencies (
Package-Requires).
- Check whether
lexical-binding is enabled in every file.
State a one-paragraph summary of what the package does before listing any issues.
Phase 2: Review Dimensions
Work through each dimension below in order. Collect ALL findings before
presenting — do not present dimension-by-dimension.
1. Correctness & Bugs
Look for logic errors and runtime hazards:
- Unguarded
car/cdr on potentially-nil values — use when, if-let, or
and to guard.
- Off-by-one errors in list/string indexing.
- Mutation of shared structure — accidental aliasing via
setcar/setcdr.
- Process/buffer leaks — processes or temp buffers created but never cleaned
up; missing
unwind-protect.
- Async race conditions — timer or sentinel callbacks that assume buffer/
process state that may have changed.
- Wrong equality predicate —
eq vs equal vs string= misuse.
- Incorrect use of
mapcar vs mapc — using mapcar when return value is
discarded (wasteful allocation).
save-excursion / save-restriction misuse — forgetting to restore state
after buffer modifications.- Hook not removed on cleanup — hooks added in init but never removed in
teardown/disable path.
- Advice not removed —
advice-add without corresponding advice-remove in
disable/unload path.
- Missing
require — symbols used from libraries not explicitly required.
- Circular or redundant
require — files requiring each other or requiring
things already guaranteed by dependencies.
2. Emacs Lisp Style & Conventions
Check against established community conventions:
File header:
;;; package-name.el --- Short description -*- lexical-binding: t -*-;;; Commentary: section present and informative.;;; Code: marker present.;;; package-name.el ends here footer present.Package-Version, Package-Requires, Author, Keywords, URL headers
present and accurate.
Naming:
- All public symbols prefixed with
package-name- (or agreed namespace).
- Internal/private symbols prefixed with
package-name-- (double dash).
- Constants use
defconst not defvar.
- Booleans named with
-p suffix (package-name-verbose-p).
Docstrings:
- Every
defun, defvar, defcustom, defface, define-minor-mode has a
docstring.
- First line of docstring is a complete sentence ending in
. and ≤80 chars.
- Interactive commands document their argument in the first line if applicable.
defcustom docstrings describe valid values.
Customisation:
- User-facing variables use
defcustom, not defvar.
defcustom has correct :type, :group, and :safe where appropriate.- A
defgroup exists for the package.
Functions:
- Prefer
cl-lib over deprecated cl package (cl-loop, cl-destructuring-bind, etc.).
- Prefer
seq- functions over manual recursion for sequence operations.
- Avoid
flet/labels — use cl-flet/cl-labels.
- Avoid
lexical-let — unnecessary with lexical-binding: t.
interactive spec is correct and uses modern forms (e.g. (interactive "r")
not deprecated forms).- Functions that modify buffers use
with-current-buffer rather than relying on
implicit current buffer.
Control flow:
- Prefer
when/unless over (if ... nil) / (if ... t).
- Prefer
cond over deeply nested if.
- Prefer
pcase over complex cond matching on structure.
- Avoid
(not (not x)) — use (and x t) or just trust truthiness.
3. Performance & Optimisation
- Repeated
buffer-substring / buffer-string in tight loops — cache the
result.
re-search-forward in loops without narrow-to-region — can be
O(n²); consider reorganising.append in loops — quadratic; prefer push + nreverse.length on a list to check emptiness — use null or consp instead.- Uncompiled lambdas in hot paths — prefer named functions or ensure byte
compilation.
- Large
defconst data — consider lazy initialisation if not always needed.
sit-for 0 / redisplay in loops — usually a sign of a design smell;
flag and explain.- Synchronous process calls blocking UI — prefer async with sentinels or
make-process.
- Unnecessary
with-temp-buffer — if only string operations are needed,
avoid buffer allocation.
- Timer granularity — timers firing too frequently (< 0.1s) without clear
need.
font-lock-add-keywords called repeatedly — should be called once, not on
every mode activation.
4. Autoloads & Load-Time Cost
- All entry-point commands and public functions the user calls directly should
have
;;;###autoload cookies.
- No expensive computation at top level (i.e. outside any function) — this runs
at load time.
defvar / defcustom at top level is fine; defun bodies running at load
time are not.require at top level is acceptable but flag heavy requires that could be
deferred with with-eval-after-load or autoload.
5. Compatibility & Portability
- Flag use of functions introduced after the declared minimum Emacs version in
Package-Requires.
- Flag OS-specific code paths without appropriate guards (
system-type checks).
- Flag hard-coded paths.
- Flag any use of
(require 'cl) — must use (require 'cl-lib).
6. Error Handling & Robustness
condition-case used where failures are plausible (network, file I/O,
subprocess).- Error messages are user-readable (not raw Lisp objects).
user-error used for user-facing mistakes (not error), so Edebug doesn't
trap them.unwind-protect used wherever resources (buffers, processes, overlays) are
allocated.
Phase 3: Output Format
Present findings as a structured action plan using the following format.
Group by severity. Within each group, order by file then by approximate line
number.
## Package Review: <package-name>
### Summary
<One paragraph: what the package does, overall impression, headline numbers>
---
### 🔴 Critical — Fix Before Release
Issues that will cause errors, data loss, or broken behaviour.
#### C1. <Short title>
**File:** `foo.el` **~Line:** 42
**Issue:** <Clear explanation of the problem and why it matters>
**Suggestion:** <What to do — no code rewrite, just direction>
#### C2. ...
---
### 🟠 Important — Strong Recommendation
Style violations, missing conventions, or meaningful inefficiencies.
#### I1. <Short title>
...
---
### 🟡 Minor — Worth Addressing
Small style issues, minor optimisations, nitpicks.
#### M1. <Short title>
...
---
### 💡 Optimisation Opportunities
Performance improvements worth considering, ordered by estimated impact.
#### O1. <Short title>
...
---
### ✅ Strengths
Brief list of things done well — keep this honest and specific.
Phase 4: Closing Note
After the plan, add a short paragraph:
"This is a plan for you to action at your own pace — not all items need to be
addressed. Prioritise 🔴 Critical items first. Feel free to ask me to elaborate
on any specific finding or to help implement a fix."
Review Principles
- Flag, don't fix. Explain the problem and point in a direction. The author
decides what to do.
- Be specific. Always cite the file and approximate line number.
- Be proportionate. A one-file utility and a major package deserve different
levels of rigour — calibrate accordingly.
- No ERT / testing review. Do not comment on presence or absence of tests.
- Respect intent. If a pattern looks unusual but is clearly deliberate,
note it as a question rather than a violation.
