Save cursor state when changing history in repl

[JesperAxelsson]

Fixes a few issues with history navigation. And are probably the root cause to #2214.

• You can now step through multi line history items
• Fix issue where starting repl line were lost when trying to search past end of history

I split out a helper to share the logic of previous and next. Might we worth moving the
searching to the history package at a later date.

[ THE BUG: Forcing an old position ]
You start with a longer string, cursor at the end.
Buffer:  cl-user> 'hello
Cursor:                 ^ (Old position saved)

Code replaces the text, then forcefully shoves 
the cursor back to that saved position.
Buffer:  cl-user> 'he
Cursor:                 ^ 💥 CRASH: Timer wakes up
                             and sees cursor in the void!

------------------------------------------

[ THE FIX: Safe linear offset ]
Buffer starts the same.
Buffer:  cl-user> 'hello

Code safely anchors to the start, then steps 
forward exactly `prefix-len`.
Buffer:  cl-user> 'he
Cursor:             ^ (Perfectly safe!)
==========================================

[code-contractor-app]

❌ Code Contractor Validation: FAILED

=== Contract: contract ===

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

📋 Contract Source: Repository

📊 Statistics:
  Files Changed:    1
  Lines Added:      35
  Lines Deleted:    38
  Total Changed:    73
  Delete Ratio:     0.52 (52%)

Status: FAILED ❌

🤖 AI Providers:
  - codex — model: gpt-5.4

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

[WARNING] max_delete_ratio
  Delete ratio too high: 0.52 > 0.50
  Limit: 0.50
  Actual: 0.52

[ERROR] naming_conventions_rule
  AI check failed: "naming_conventions_rule"
  ❌ Reason:
    The added function name uses `startwith` instead of kebab-cased word
    separation, violating the naming convention for functions/variables.

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

---

💬 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

[code-contractor-app]

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

[code-contractor-app]

Code Contractor: naming_conventions_rule

Contract: contract

AI check failed: "naming_conventions_rule"

Reason:
The added function name uses startwith instead of kebab-cased word separation, violating the naming convention for functions/variables.

---

💬 Reply /dismiss <reason> to dismiss this violation.

[theangelperalta]

Really nice fix — the diagram in the description made the root cause click immediately. Anchoring to input-start-point and stepping by prefix-len instead of forcing a saved column is exactly the kind of change that makes the whole thing robust for multi-line items. 👏

I confirmed in src/common/history.lisp that previous-history/next-history only move the index on success and leave it untouched at the boundary — so your steps-counting + rewind logic lines up perfectly. That's a real improvement over the old path that walked the index to 0 and left it stranded.

A few small things I was curious about, mostly to make sure I understand the intent:

1. Package qualifiers — the file is (:use :cl :lem) and everything around your change uses unqualified move-point / character-offset / buffer-end. Was there a reason to reach for lem-core:move-point and lem-core:character-offset here specifically? Dropping the prefix would match the surrounding style (and the contract.yml preference for lem: over deeper packages).

2. The rewind on the :next path — since restore-edit-string already sets index back to (length data) unconditionally, does the (dotimes (i steps) (funcall rewind-fn ...)) just before it ever have an effect when direction is :next? If I'm reading it right it only matters for :previous — would it be clearer to guard it or note that?

3. Naming — tiny one: the helper is %search-history-startwith while the commands are …-startswith-input. Intentional, or a stray missing s?

4. A regression test? — this fixes a crash with a lovely concrete repro (multi-line item, cursor past EOL). Would it be worth capturing that so it can't quietly come back? The cursor logic lives in listener-mode rather than the already-tested common/history, so I understand it's a bit more setup.

None of these are blockers from my side — the core fix is sound and I'd be happy to see it land. Thanks for splitting out the helper too; the previous/next duplication had been bugging me. 🙂

[JesperAxelsson]

Appreciate the feed back!

A few small things I was curious about, mostly to make sure I understand the intent:

1. **Package qualifiers** — the file is `(:use :cl :lem)` and everything around your change uses unqualified `move-point` / `character-offset` / `buffer-end`. Was there a reason to reach for `lem-core:move-point` and `lem-core:character-offset` here specifically? Dropping the prefix would match the surrounding style (and the `contract.yml` preference for `lem:` over deeper packages).

No real reason. I was a bit unsure about the conventions, the history functions did use qualifier so I thought it were standard.
Removed the qualifiers now, and will remember to skip them for core lem functions.

2. **The rewind on the `:next` path** — since `restore-edit-string` already sets `index` back to `(length data)` unconditionally, does the `(dotimes (i steps) (funcall rewind-fn ...))` just before it ever have an effect when `direction` is `:next`? If I'm reading it right it only matters for `:previous` — would it be clearer to guard it or note that?

True, changed it. Caused a bit of an avalanche of refactorings. The function do look cleaner now.

3. **Naming** — tiny one: the helper is `%search-history-startwith` while the commands are `…-startswith-input`. Intentional, or a stray missing `s`?

Add the -input suffix. Pondered fixing the kebab casing but thought it better to match the command names. Easier to grep.

4. **A regression test?** — this fixes a crash with a lovely concrete repro (multi-line item, cursor past EOL). Would it be worth capturing that so it can't quietly come back? The cursor logic lives in listener-mode rather than the already-tested `common/history`, so I understand it's a bit more setup.

Added a small test as well.

None of these are blockers from my side — the core fix is sound and I'd be happy to see it land. Thanks for splitting out the helper too; the previous/next duplication had been bugging me. 🙂

Your welcome :)