Skip to content

Adopt useful changes from kiennq/ghostel fork#103

Merged
dakra merged 2 commits intomainfrom
fork/adopt-kiennq-improvements
Apr 14, 2026
Merged

Adopt useful changes from kiennq/ghostel fork#103
dakra merged 2 commits intomainfrom
fork/adopt-kiennq-improvements

Conversation

@dakra
Copy link
Copy Markdown
Owner

@dakra dakra commented Apr 14, 2026

Summary

Cherry-picks two useful improvements from the kiennq/ghostel fork, skipping Windows ConPTY, dyn-loader, and changes that conflict with our architecture.

  • ghostel-ignore-cursor-change defcustom: When non-nil, terminal requests to change cursor shape/visibility are ignored. Useful when editor-owned cursor behavior should take precedence.
  • Platform tag arch normalization: Normalizes amd64 -> x86_64 and arm64 -> aarch64 in ghostel--module-platform-tag so module auto-download works on systems where system-configuration uses non-Zig arch names.

Changes reviewed and skipped

Fork change Reason skipped
Scroll primitives (ghostel--scroll, ghostel--scroll-top) Already removed in 156a714 — dead code since scrollback is materialized
Copy mode viewport scrolling Unnecessary with materialized scrollback
pixel-scroll-precision-mode suppression Already solved better in 3b6c980
Readonly-safe rendering Cross-cutting; inhibit-read-only approach is more Emacs-conventional
ghostel-cursor-follow Our viewport-start detection + marker save is superior
Face-remap caching Benchmarked at ~1.5 us/redraw savings — not worth extra state
Title tracking removal Regression
Windows ConPTY / dyn-loader Out of scope

Test plan

  • make -j4 all passes (66/66 tests)
  • Verify ghostel-ignore-cursor-change blocks cursor shape changes when set to t
  • Verify ghostel--module-platform-tag returns correct tag on ARM systems

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Cherry-picks two improvements from the kiennq/ghostel fork to better align cursor behavior with editor preferences and to improve native module auto-download compatibility across platform naming differences.

Changes:

  • Added ghostel-ignore-cursor-change defcustom to ignore terminal-driven cursor shape/visibility changes (while copy mode continues to manage cursor state).
  • Normalized platform arch strings in ghostel--module-platform-tag (amd64x86_64, arm64aarch64) to match release asset naming.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ghostel.el Outdated
Comment thread ghostel.el Outdated
Comment thread ghostel.el
Comment thread ghostel.el
@dakra dakra force-pushed the fork/adopt-kiennq-improvements branch from 994c238 to 4e8269e Compare April 14, 2026 07:32
dakra added 2 commits April 14, 2026 09:34
When non-nil, terminal requests to change cursor shape or visibility
are ignored.  Useful when editor-owned cursor behavior should take
precedence.  Copy mode always forces a visible cursor regardless.

Inspired by kiennq/ghostel fork.
Some systems report "amd64" or "arm64" in system-configuration
instead of the Zig target triple names "x86_64" / "aarch64" used
in release asset filenames.  Normalize before building the tag so
auto-download works on those systems.

Inspired by kiennq/ghostel fork.
@dakra dakra force-pushed the fork/adopt-kiennq-improvements branch from 4e8269e to 27dcec0 Compare April 14, 2026 07:35
@dakra
Copy link
Copy Markdown
Owner Author

dakra commented Apr 14, 2026

@kiennq jFYI, I looked at what other changes besides the dynloader and windows support you have in your fork.

I implemented the 2 that where imho useful.

I also tried the face caching but that only saved 1ns but adds to local defvars, so imho the state + extra code( / if branch) is not worth that nanosecond.

@dakra dakra merged commit 27dcec0 into main Apr 14, 2026
20 checks passed
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.

2 participants