martin/emacs-doom
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

patches: address review B1-B4 and N1,N3

Browse Source 
B4: Shorten all subject lines to <=50 chars (preference from CONTRIBUTE).
B3: Fix intermediate BUF_CHARS_MODIFF state in 0002: use BUF_MODIFF
    from the start, eliminating the wrong-then-corrected pattern across
    patches 0002+0007.
B2: Wrap long NEWS line in Zoom entry (was 80 chars, now <=79).
B1: Long block_input comment already fixed by 0004 in both branches;
    confirmed no change needed in final state.
N1: Fix DEFVAR doc in 0001 to reflect auto-detection at startup.
N3: Convert VoiceOver NEWS bullet list to prose paragraphs.
N2: Skip (existing file uses 80-char separators throughout; changing
    only new ones would be inconsistent).
This commit is contained in:
Daneel
2026-03-04 15:28:02 +01:00
parent 61f629350c
commit 2ab4468ca0
10 changed files with 434862 additions and 1676 deletions
Download Patch File Download Diff File Expand all files Collapse all files

483
patches/0007-ns-announce-overlay-completions-to-VoiceOver.patch Normal file
View File

@@ -0,0 +1,483 @@
From 7659545ccac02135e17c61836b21f68bc25efb04 Mon Sep 17 00:00:00 2001
From: Martin Sukany <martin@sukany.cz>
Date: Wed, 4 Mar 2026 15:23:56 +0100
Subject: [PATCH 8/9] ns: announce overlay completions to VoiceOver
Completion frameworks such as Vertico, Ivy, and Icomplete render
candidates via overlay before-string/after-string properties. Without
this change VoiceOver cannot read overlay-based completion UIs.
* src/nsterm.m (ns_ax_face_is_selected): New static function; matches
'current', 'selected', 'selection' in face symbol names.
(ns_ax_selected_overlay_text): New function; scan overlay strings in
the window for a line with a selected face; return its text.
(ensureTextCache): Switch cache-validity counter from BUF_CHARS_MODIFF
to BUF_MODIFF. Fold/unfold commands (org-mode, outline-mode,
hideshow-mode) change the 'invisible text property via
`put-text-property', which bumps BUF_MODIFF but not BUF_CHARS_MODIFF.
Using BUF_CHARS_MODIFF would serve stale AX text across fold/unfold.
The rebuild is O(visible-buffer-text) but ensureTextCache is called
exclusively from AX getters at human interaction speed, never from the
redisplay notification path; font-lock passes cause zero rebuild cost.
(postAccessibilityNotificationsForFrame:): Handle BUF_OVERLAY_MODIFF
changes independently of text changes. Use BUF_CHARS_MODIFF to gate
ValueChanged. Do not call ensureTextCache from the cursor-moved branch:
the granularity detection uses cachedText directly (falling back to
granularity_unknown when the cache is absent), so font-lock passes
cannot trigger O(buffer-size) rebuilds via the notification path.
---
src/nsterm.h | 1 +
src/nsterm.m | 301 ++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 262 insertions(+), 40 deletions(-)
diff --git a/src/nsterm.h b/src/nsterm.h
index d9ae6efc2e..ff81675bb5 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -510,6 +510,7 @@ typedef struct ns_ax_visible_run
@property (nonatomic, assign) ptrdiff_t cachedOverlayModiff;
@property (nonatomic, assign) ptrdiff_t cachedTextStart;
@property (nonatomic, assign) ptrdiff_t cachedModiff;
+@property (nonatomic, assign) ptrdiff_t cachedCharsModiff;
@property (nonatomic, assign) ptrdiff_t cachedPoint;
@property (nonatomic, assign) BOOL cachedMarkActive;
@property (nonatomic, copy) NSString *cachedCompletionAnnouncement;
diff --git a/src/nsterm.m b/src/nsterm.m
index 4261886974..90c59c47b4 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7276,11 +7276,126 @@ Accessibility virtual elements (macOS / Cocoa only)
/* ---- Helper: extract buffer text for accessibility ---- */
+/* Extract the currently selected candidate text from overlay display
+ strings. Completion frameworks render candidates as overlay
+ before-string/after-string and highlight the current candidate
+ with a face whose name contains "current" or "selected"
+ (e.g. vertico-current, icomplete-selected-match, ivy-current-match).
+
+ Scan all overlays in the buffer region [BEG, END), find the line
+ whose face matches the selection heuristic, and return it (already
+ trimmed of surrounding whitespace).
+
+ Also set *OUT_LINE_INDEX to the 0-based visual line index of the
+ selected candidate (for Zoom positioning), counting only non-trivial
+ lines. Set to -1 if not found.
+
+ Returns nil if no selected candidate is found. */
+static NSString *
+ns_ax_selected_overlay_text (struct buffer *b,
+ ptrdiff_t beg, ptrdiff_t end,
+ int *out_line_index)
+{
+ *out_line_index = -1;
+
+ Lisp_Object ov_list = Foverlays_in (make_fixnum (beg),
+ make_fixnum (end));
+
+ for (Lisp_Object tail = ov_list; CONSP (tail); tail = XCDR (tail))
+ {
+ Lisp_Object ov = XCAR (tail);
+ Lisp_Object strings[2];
+ strings[0] = Foverlay_get (ov, Qbefore_string);
+ strings[1] = Foverlay_get (ov, Qafter_string);
+
+ for (int s = 0; s < 2; s++)
+ {
+ if (!STRINGP (strings[s]))
+ continue;
+
+ Lisp_Object str = strings[s];
+ ptrdiff_t slen = SCHARS (str);
+ if (slen == 0)
+ continue;
+
+ /* Scan for newline positions using SDATA for efficiency.
+ The data pointer is used only in this loop, before any
+ Lisp calls (Fget_text_property etc.) that could trigger
+ GC and relocate string data. */
+ const unsigned char *data = SDATA (str);
+ ptrdiff_t byte_len = SBYTES (str);
+ /* 512 lines is sufficient for any completion UI;
+ vertico-count defaults to 10. */
+ ptrdiff_t line_starts[512];
+ ptrdiff_t line_ends[512];
+ int nlines = 0;
+ ptrdiff_t char_pos = 0, byte_pos = 0, lstart = 0;
+
+ while (byte_pos < byte_len && nlines < 512)
+ {
+ if (data[byte_pos] == '\n')
+ {
+ if (char_pos > lstart)
+ {
+ line_starts[nlines] = lstart;
+ line_ends[nlines] = char_pos;
+ nlines++;
+ }
+ lstart = char_pos + 1;
+ }
+ if (STRING_MULTIBYTE (str))
+ byte_pos += BYTES_BY_CHAR_HEAD (data[byte_pos]);
+ else
+ byte_pos++;
+ char_pos++;
+ }
+ if (char_pos > lstart && nlines < 512)
+ {
+ line_starts[nlines] = lstart;
+ line_ends[nlines] = char_pos;
+ nlines++;
+ }
+
+ /* Find the line whose face indicates selection. Track
+ visual line index for Zoom (skip whitespace-only lines
+ like Vertico's leading cursor-space). */
+ int candidate_idx = 0;
+ for (int li = 0; li < nlines; li++)
+ {
+ Lisp_Object face
+ = Fget_text_property (make_fixnum (line_starts[li]),
+ Qface, str);
+ if (ns_face_name_matches_selected_p (face))
+ {
+ Lisp_Object line
+ = Fsubstring_no_properties (
+ str,
+ make_fixnum (line_starts[li]),
+ make_fixnum (line_ends[li]));
+ NSString *text = [NSString stringWithLispString:line];
+ text = [text stringByTrimmingCharactersInSet:
+ [NSCharacterSet
+ whitespaceAndNewlineCharacterSet]];
+ if ([text length] > 0)
+ {
+ *out_line_index = candidate_idx;
+ return text;
+ }
+ }
+
+ /* Count non-trivial lines as candidates for Zoom. */
+ if (line_ends[li] - line_starts[li] > 1)
+ candidate_idx++;
+ }
+ }
+ }
+
+ return nil;
+}
/* Build accessibility text for window W, skipping invisible text.
Populates *OUT_START with the buffer start charpos.
Populates *OUT_RUNS with an array of visible runs and *OUT_NRUNS
with the count. Caller must free *OUT_RUNS with xfree(). */
-
static NSString *
ns_ax_buffer_text (struct window *w, ptrdiff_t *out_start,
ns_ax_visible_run **out_runs, NSUInteger *out_nruns)
@@ -7917,6 +8032,7 @@ @implementation EmacsAccessibilityBuffer
@synthesize cachedOverlayModiff;
@synthesize cachedTextStart;
@synthesize cachedModiff;
+@synthesize cachedCharsModiff;
@synthesize cachedPoint;
@synthesize cachedMarkActive;
@synthesize cachedCompletionAnnouncement;
@@ -8027,20 +8143,33 @@ - (void)ensureTextCache
return;
/* Use BUF_MODIFF, not BUF_CHARS_MODIFF, for cache validity.
+
Fold/unfold commands (org-mode, outline-mode, hideshow-mode) change
text visibility by modifying the 'invisible text property via
- put-text-property or add-text-properties. These bump BUF_MODIFF
- but not BUF_CHARS_MODIFF. Using BUF_CHARS_MODIFF would serve stale
- AX text across fold/unfold, causing VoiceOver to read the wrong
- content after an org-cycle or similar command.
- ensureTextCache is called exclusively from AX getters at human
- interaction speed (never from the redisplay notification path), so
- font-lock passes cause zero rebuild cost via the notification path.
- Do NOT track BUF_OVERLAY_MODIFF here --- overlay text is not
- included in the cached AX text (it is handled separately via
- explicit announcements in postAccessibilityNotificationsForFrame).
- Including overlay_modiff would silently update cachedOverlayModiff
- and prevent the notification dispatch from detecting changes. */
+ `put-text-property' or `add-text-properties'. These bump BUF_MODIFF
+ but NOT BUF_CHARS_MODIFF, because no characters are inserted or
+ deleted. Using only BUF_CHARS_MODIFF would serve stale AX text
+ across fold/unfold: VoiceOver would continue reading hidden content
+ as if it were visible, or miss newly revealed content entirely.
+
+ BUF_MODIFF is bumped by all buffer modifications including
+ text-property changes (e.g. font-lock face assignments). The
+ per-rebuild cost is O(visible-buffer-text), but `ensureTextCache'
+ is called exclusively from AX getters (accessibilityValue,
+ accessibilitySelectedTextRange, etc.) which run at human interaction
+ speed --- not from the redisplay notification path. Font-lock
+ passes do not call this method, so the rebuild cost per font-lock
+ cycle is zero. The redisplay notification path (postAccessibility-
+ NotificationsForFrame:) uses cachedText directly without calling
+ ensureTextCache; granularity detection falls back gracefully when
+ the cache is absent.
+
+ Do NOT use BUF_OVERLAY_MODIFF alone: org-mode >= 29 (org-fold-core)
+ uses text properties, not overlays, for folding, so
+ BUF_OVERLAY_MODIFF would miss those changes. Additionally, modes
+ like hl-line-mode bump BUF_OVERLAY_MODIFF on every
+ post-command-hook, yielding the same per-keystroke rebuild cost as
+ BUF_MODIFF, with none of its correctness guarantee. */
ptrdiff_t modiff = BUF_MODIFF (b);
ptrdiff_t pt = BUF_PT (b);
NSUInteger textLen = cachedText ? [cachedText length] : 0;
@@ -8072,9 +8201,8 @@ included in the cached AX text (it is handled separately via
Walk the cached text once, recording the start offset of each
line. Uses NSString lineRangeForRange: --- O(N) in the total
text --- but this loop runs only on cache rebuild, which is
- gated on BUF_MODIFF. Font-lock passes trigger a rebuild only
- when called from AX getters (human interaction speed), never
- from the notification path. */
+ gated on BUF_MODIFF changes, ensuring the line index always
+ matches the currently visible text (including after fold/unfold). */
if (lineStartOffsets)
xfree (lineStartOffsets);
lineStartOffsets = NULL;
@@ -8590,26 +8718,26 @@ - (NSInteger)accessibilityInsertionPointLineNumber
return [self lineForAXIndex:point_idx];
}
-- (NSRange)accessibilityRangeForLine:(NSInteger)line
+- (NSString *)accessibilityStringForRange:(NSRange)range
{
if (![NSThread isMainThread])
{
- __block NSRange result;
+ __block NSString *result;
dispatch_sync (dispatch_get_main_queue (), ^{
- result = [self accessibilityRangeForLine:line];
+ result = [self accessibilityStringForRange:range];
});
return result;
}
[self ensureTextCache];
- if (!cachedText || line < 0)
- return NSMakeRange (NSNotFound, 0);
-
- NSUInteger len = [cachedText length];
- if (len == 0)
- return (line == 0) ? NSMakeRange (0, 0)
- : NSMakeRange (NSNotFound, 0);
+ if (!cachedText || range.location + range.length > [cachedText length])
+ return @"";
+ return [cachedText substringWithRange:range];
+}
- return [self rangeForLine:(NSUInteger)line textLength:len];
+- (NSAttributedString *)accessibilityAttributedStringForRange:(NSRange)range
+{
+ NSString *str = [self accessibilityStringForRange:range];
+ return [[[NSAttributedString alloc] initWithString:str] autorelease];
}
- (NSInteger)accessibilityLineForIndex:(NSInteger)index
@@ -8631,6 +8759,29 @@ - (NSInteger)accessibilityLineForIndex:(NSInteger)index
idx = [cachedText length];
return [self lineForAXIndex:idx];
+
+}
+
+- (NSRange)accessibilityRangeForLine:(NSInteger)line
+{
+ if (![NSThread isMainThread])
+ {
+ __block NSRange result;
+ dispatch_sync (dispatch_get_main_queue (), ^{
+ result = [self accessibilityRangeForLine:line];
+ });
+ return result;
+ }
+ [self ensureTextCache];
+ if (!cachedText || line < 0)
+ return NSMakeRange (NSNotFound, 0);
+
+ NSUInteger len = [cachedText length];
+ if (len == 0)
+ return (line == 0) ? NSMakeRange (0, 0)
+ : NSMakeRange (NSNotFound, 0);
+
+ return [self rangeForLine:(NSUInteger)line textLength:len];
}
- (NSRange)accessibilityRangeForIndex:(NSInteger)index
@@ -8833,7 +8984,7 @@ - (NSRect)accessibilityFrame
/* ===================================================================
- EmacsAccessibilityBuffer (Notifications) — AX event dispatch
+ EmacsAccessibilityBuffer (Notifications) --- AX event dispatch
These methods notify VoiceOver of text and selection changes.
Called from the redisplay cycle (postAccessibilityUpdates).
@@ -8848,7 +8999,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
if (point > self.cachedPoint
&& point - self.cachedPoint == 1)
{
- /* Single char inserted — refresh cache and grab it. */
+ /* Single char inserted --- refresh cache and grab it. */
[self invalidateTextCache];
[self ensureTextCache];
if (cachedText)
@@ -8867,7 +9018,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
/* Update cachedPoint here so the selection-move branch does NOT
fire for point changes caused by edits. WebKit and Chromium
never send both ValueChanged and SelectedTextChanged for the
- same user action — they are mutually exclusive. */
+ same user action --- they are mutually exclusive. */
self.cachedPoint = point;
NSDictionary *change = @{
@@ -9261,16 +9412,80 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
BOOL markActive = !NILP (BVAR (b, mark_active));
/* --- Text changed (edit) --- */
+ ptrdiff_t chars_modiff = BUF_CHARS_MODIFF (b);
if (modiff != self.cachedModiff)
{
self.cachedModiff = modiff;
- [self postTextChangedNotification:point];
+ /* Only post ValueChanged when actual characters changed.
+ Text property changes (e.g. face updates from
+ vertico--prompt-selection) bump BUF_MODIFF but not
+ BUF_CHARS_MODIFF. Posting ValueChanged for property-only
+ changes causes VoiceOver to say "new line" when the diff
+ is non-empty due to overlay content changes. */
+ if (chars_modiff != self.cachedCharsModiff)
+ {
+ self.cachedCharsModiff = chars_modiff;
+ [self postTextChangedNotification:point];
+ }
+ }
+
+
+ /* --- Overlay content changed (e.g. Vertico/Ivy candidate switch) ---
+ Check independently of the modiff branch above, because
+ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
+ changes in vertico--prompt-selection) and BUF_OVERLAY_MODIFF
+ (via overlay-put) in the same command cycle. If this were an
+ else-if, the modiff branch would always win and overlay
+ announcements would never fire.
+ Do NOT invalidate the text cache --- the buffer text has not
+ changed, and cache invalidation causes VoiceOver to diff old
+ vs new AX text and announce spurious "new line". */
+ if (BUF_OVERLAY_MODIFF (b) != self.cachedOverlayModiff)
+ {
+ self.cachedOverlayModiff = BUF_OVERLAY_MODIFF (b);
+
+ /* Overlay completion candidates (Vertico, Icomplete, Ivy) are
+ displayed in the minibuffer. In normal editing buffers,
+ font-lock and other modes change BUF_OVERLAY_MODIFF on
+ every redisplay, triggering O(overlays) work per keystroke.
+ Restrict the scan to minibuffer windows. */
+ if (MINI_WINDOW_P (w))
+ {
+ int selected_line = -1;
+ NSString *candidate
+ = ns_ax_selected_overlay_text (b, BUF_BEGV (b), BUF_ZV (b),
+ &selected_line);
+ if (candidate)
+ {
+ /* Deduplicate: only announce when the candidate changed. */
+ if (![candidate isEqualToString:
+ self.cachedCompletionAnnouncement])
+ {
+ self.cachedCompletionAnnouncement = candidate;
+
+ /* Announce the candidate text directly via NSApp.
+ Do NOT post SelectedTextChanged --- that would cause
+ VoiceOver to read the AX text at the cursor position
+ (the minibuffer input line), not the overlay candidate.
+ AnnouncementRequested with High priority interrupts
+ any current speech and announces our text. */
+ NSDictionary *annInfo = @{
+ NSAccessibilityAnnouncementKey: candidate,
+ NSAccessibilityPriorityKey:
+ @(NSAccessibilityPriorityHigh)
+ };
+ ns_ax_post_notification_with_info (
+ NSApp,
+ NSAccessibilityAnnouncementRequestedNotification,
+ annInfo);
+ }
+ }
+ }
}
/* --- Cursor moved or selection changed ---
- Use 'else if' — edits and selection moves are mutually exclusive
- per the WebKit/Chromium pattern. */
- else if (point != self.cachedPoint || markActive != self.cachedMarkActive)
+ Independent check from the overlay branch above. */
+ if (point != self.cachedPoint || markActive != self.cachedMarkActive)
{
ptrdiff_t oldPoint = self.cachedPoint;
BOOL oldMarkActive = self.cachedMarkActive;
@@ -9288,8 +9503,14 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
bool isCtrlNP = ns_ax_event_is_line_nav_key (&ctrlNP);
/* --- Granularity detection --- */
+ /* Use cached text as-is; do NOT call ensureTextCache here.
+ ensureTextCache is O(visible-buffer-text) and must not run on
+ every redisplay cycle. Using stale cached text for granularity
+ classification is safe: the worst case is an incorrect
+ granularity hint (defaulting to unknown), which causes VoiceOver
+ to make its own determination. Fresh text is always available
+ to VoiceOver via the AX getter path (accessibilityValue etc.). */
NSInteger granularity = ns_ax_text_selection_granularity_unknown;
- [self ensureTextCache];
if (cachedText && oldPoint > 0)
{
NSUInteger tlen = [cachedText length];
@@ -12449,7 +12670,7 @@ - (int) fullscreenState
if (WINDOW_LEAF_P (w))
{
- /* Buffer element — reuse existing if available. */
+ /* Buffer element --- reuse existing if available. */
EmacsAccessibilityBuffer *elem
= [existing objectForKey:[NSValue valueWithPointer:w]];
if (!elem)
@@ -12483,7 +12704,7 @@ - (int) fullscreenState
}
else
{
- /* Internal (combination) window — recurse into children. */
+ /* Internal (combination) window --- recurse into children. */
Lisp_Object child = w->contents;
while (!NILP (child))
{
@@ -12595,7 +12816,7 @@ - (void)postAccessibilityUpdates
accessibilityUpdating = YES;
/* Detect window tree change (split, delete, new buffer). Compare
- FRAME_ROOT_WINDOW — if it changed, the tree structure changed. */
+ FRAME_ROOT_WINDOW --- if it changed, the tree structure changed. */
Lisp_Object curRoot = FRAME_ROOT_WINDOW (emacsframe);
if (!EQ (curRoot, lastRootWindow))
{
@@ -12604,12 +12825,12 @@ - (void)postAccessibilityUpdates
}
/* If tree is stale, rebuild FIRST so we don't iterate freed
- window pointers. Skip notifications for this cycle — the
+ window pointers. Skip notifications for this cycle --- the
freshly-built elements have no previous state to diff against. */
if (!accessibilityTreeValid)
{
[self rebuildAccessibilityTree];
- /* Invalidate span cache — window layout changed. */
+ /* Invalidate span cache --- window layout changed. */
for (EmacsAccessibilityElement *elem in accessibilityElements)
if ([elem isKindOfClass: [EmacsAccessibilityBuffer class]])
[(EmacsAccessibilityBuffer *) elem invalidateInteractiveSpans];
--
2.43.0