Add advice to mark-set command for multicursor selection

[Plipus]

Hello,
While using lem to learn common lisp, I noticed that marks doesn't seem to work with multicursor.
Adding this advice seems to fix the issue, but as I’m still exploring both the codebase and Common Lisp, I might have overlooked something.
Is there are any implications or edge cases I should be aware of?

[code-contractor-app]

✅ Code Contractor Validation: PASSED

=== Contract: contract ===

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

📋 Contract Source: Repository

📊 Statistics:
  Files Changed:    1
  Lines Added:      1
  Lines Deleted:    1
  Total Changed:    2
  Delete Ratio:     0.50 (50%)

Status: PASSED ✅

🤖 AI Providers:
  - codex — model: (Codex default)

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
🎉 No violations detected. Great job!

📋 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 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

[code-contractor-app]

Code Contractor validation passed ✅ — see the sticky comment for full results.

[theangelperalta]

Thanks for digging into this! I looked into it and I don't think this change is the right fix — I believe it's redundant, and the real bug is elsewhere.

mark-set already wraps itself in process-each-cursors via its own execute :around method (src/commands/mark.lisp:38-41, present since 2023). editable-advice supplies that same process-each-cursors wrapper, so this PR just stacks a second, nested one.

I ran it headless (fake-interface) to check:

• On main, marks are already set for every cursor — mark-set with 3 cursors returns ((CURSOR T) (FAKE-CURSOR T) (FAKE-CURSOR T)).
• The multicursor failure is actually a crash in (message "Mark set"): it's called once per cursor inside process-each-cursors with the buffer point rebound to a fake cursor, and the :follow-cursor popup positioning then compares points across buffers → %always-same-buffer assertion. Dropping the message call makes it pass.
• Applying this PR did not stop that crash in my repro.

So the fix is probably to stop calling message per-cursor — e.g. move it out of the command body so it runs once after all cursors are processed. Could you share your exact repro (frontend + keys)? I wasn't able to find a case where adding editable-advice changes behavior.

[theangelperalta]

This is redundant: mark-set already has an execute :around doing process-each-cursors at line 38-41, and editable-advice adds the same wrapper — so this nests two process-each-cursors passes. In a headless test it didn't change the multicursor result (still crashes in the per-cursor (message "Mark set") below).

[Plipus]

Thanks for your response !
After another try this afternoon, it seems that I can't reproduce the bug on the main branch...
However, after multiple additional attempts, I did encounter a crash. I think my bug fix was just an illusion caused by insufficient testing from my side, sorry.

I will try to fix the message problem, moreover I have another branch where I try to fix multicursor for vi-mode, and I have the same kind of problem with part of commands that can't by runned multiple times (With my fix, d with multiple cursors will ask a motion for each cursor, instead of applying the same motion for every cursor).

But extracting the point related stuff in another command will expose two commands in M-x, the wrapper and the subcommand, it's not a beautiful solution. Or can we hide a command from M-x ?

I see that there is a post-command-hook, can it be the solution for extracting the "run only once" part ?
But I the command must be aware that an advice can be applied and adapt its implementation, it's removing the beauty of the advice in my opinion.
I'm new in common lisp, is there a pattern that I can apply in this situation ?

[theangelperalta]

No worries at all — thanks for confirming, and these are good questions. A few thoughts:

Hiding a command from M-x / avoiding two commands. You don't need a second command — only define-command registers into the M-x table (register-command → add-command, unconditionally), and there's no hidden/visible flag. So the idiom is to factor the internal piece into a plain defun and keep a single command. The advice already gives you the per-cursor loop, so a helper doesn't need to be a command.

The "run once" part (the message). The body re-runs once per cursor because process-each-cursors calls it for each fake cursor plus once for the real cursor. You can use the point type as the discriminator:

(unless (typep (current-point) 'fake-cursor)
  (message "Mark set"))

On the fake-cursor passes (current-point) is the fake cursor; only the final real pass isn't — so the message fires exactly once, and it also fixes the popup crash since :follow-cursor then anchors to the real cursor.

*post-command-hook*. It does run exactly once (it's in call-command, outside the process-each-cursors :around), so it is technically a "run once" spot. But it's a global hook — the command would have to register it and check (this-command), which is more machinery and less local than the guard above. I'd keep it for cross-cutting concerns rather than per-command output.

The vi-mode d-prompts-per-cursor problem. That's the harder, general version: an argument/prompt should be read once and reused. Arguments are read inside the per-cursor body, so naive prompting re-prompts. The pattern is to read the motion once, before iterating cursors, then apply the captured value with do-each-cursors. (This is roughly what Emacs multiple-cursors does: run for the real cursor first, record its input, then replay the recorded input for the fake cursors so they don't re-prompt.) Note that vi-mode doesn't use process-each-cursors at all today, so you have room to structure it that way from the start.

Happy to take a look at the vi-mode branch if you push it as draft PR

[Plipus]

I was thinking about second command because I wanted to re-use existing advice, but it's clear that it's not a good solution.
I rewrote the PR with the cursor type check, it's seems to be the simpler solution. And as I will probably use it for the vi-mode, I extract it in a macro

[theangelperalta]

Right scope — each cursor still gets its mark, only the echo runs once. Fixes both the duplicate message and the :follow-cursor crash. Load order checks out (cursors → command-advices → commands/mark), so the macro compiles before use.

Two nits: add a trailing newline to cursors.lisp, and a docstring on the now-public macro.

Heads up for vi-mode reuse: this is a "run once" primitive — the d-prompts-per-cursor case needs input read once and replayed, which this won't cover. 👍