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

patches: address remaining medium/low review findings

Browse Source 
M-REM-1 - Stack array bound comment:
ns_ax_selected_child_frame_text uses line_starts/line_ends[128].
Added comment documenting the 128-line bound and the 10,000-character
upstream guard that makes silent truncation safe in practice.

M-REM-2 - Remove dead BUFFER_LIVE_P check:
The second BUFFER_LIVE_P check in announceChildFrameCompletion (after
updating childFrameLastBuffer/Modiff) is unreachable: no code between
the entry check and this point can kill the buffer.  Replaced with
eassert so DEBUG builds catch violations while releasing dead code.

M-REM-3 - @interface declaration for postEchoAreaAnnouncementIfNeeded:
Added declaration to the #ifdef NS_IMPL_COCOA accessibility block in
nsterm.h alongside existing postAccessibilityUpdates and
announceChildFrameCompletion declarations.  postAccessibilityUpdates
is called from ns_update_end; all three methods should be declared.

L-REM-4 - Commit message class name:
Changed (EmacsAXBuffer) to (EmacsAccessibilityBuffer) in the subject
line.  EmacsAXBuffer does not exist; EmacsAccessibilityBuffer is the
actual class that received the voiceoverSetPoint ivar.

NEWS - Remove third-party package names:
Replaced 'Vertico, Icomplete, Ivy for overlay candidates; Corfu,
Company-box for child frame popups' with generic descriptions per GNU
Emacs NEWS convention (third-party package names avoided in NEWS).
This commit is contained in:
Daneel
2026-03-03 10:21:41 +01:00
parent 3bb6c989c9
commit 9bee2a1987
9 changed files with 63 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch
View File

@@ -1,4 +1,4 @@
From 7cb9bca680a56848fe4bd4ddb51f643e8946e7b8 Mon Sep 17 00:00:00 2001
From 1e3d3919fd41e4480a02190fb89bee1ef8107d62 Mon Sep 17 00:00:00 2001
From: Daneel <daneel@sukany.cz>
Date: Mon, 2 Mar 2026 18:49:13 +0100
Subject: [PATCH 8/8] ns: announce child frame completion candidates for
@@ -14,7 +14,7 @@ childFrameLastCandidate, childFrameCompletionActive, lastEchoCharsModiff
ivars; remove cachedOverlayModiffForText (unused after BUF_OVERLAY_MODIFF
removed from ensureTextCache to prevent hl-line-mode O(N) rebuilds).
Initialize voiceoverSetPoint, childFrameLastBuffer in initFrameFromEmacs:.
(EmacsAXBuffer): Add voiceoverSetPoint ivar.
(EmacsAccessibilityBuffer): Add voiceoverSetPoint ivar.
* src/nsterm.m (ns_ax_buffer_text): Add block_input protection for
Lisp calls; use record_unwind_protect_void to guarantee unblock_input.
(ensureTextCache): Remove BUF_OVERLAY_MODIFF tracking; keep only
@@ -31,10 +31,10 @@ area announcements.
* doc/emacs/macos.texi: Fix dangling semicolon in GNUstep paragraph.
---
doc/emacs/macos.texi | 14 +-
etc/NEWS | 18 +-
src/nsterm.h | 20 ++
src/nsterm.m | 507 +++++++++++++++++++++++++++++++++++++++----
4 files changed, 499 insertions(+), 60 deletions(-)
etc/NEWS | 25 ++-
src/nsterm.h | 21 ++
src/nsterm.m | 514 +++++++++++++++++++++++++++++++++++++++----
4 files changed, 510 insertions(+), 64 deletions(-)
diff --git a/doc/emacs/macos.texi b/doc/emacs/macos.texi
index 8d4a7825d8..03a657f970 100644
@@ -69,10 +69,24 @@ index 8d4a7825d8..03a657f970 100644
@vindex ns-accessibility-enabled
To disable the accessibility interface entirely (for instance, to
diff --git a/etc/NEWS b/etc/NEWS
index 7f917f93b2..d7631fa6c7 100644
index 7f917f93b2..bbec21b635 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -4385,16 +4385,20 @@ allowing Emacs users access to speech recognition utilities.
@@ -88,10 +88,9 @@ When macOS Zoom is enabled (System Settings, Accessibility, Zoom,
Follow keyboard focus), Emacs informs Zoom of the text cursor position
after every cursor redraw via 'UAZoomChangeFocus'. The zoomed viewport
automatically tracks the insertion point across window splits and
-switches. Completion frameworks (Vertico, Icomplete, Ivy for overlay
-candidates; Corfu, Company-box for child frame popups) are also
-tracked: Zoom follows the selected candidate rather than the text
-cursor during completion.
+switches. Overlay-based completion frameworks and child-frame popup completions
+are also tracked: Zoom follows the selected candidate rather than the
+text cursor during completion.
+++
** 'line-spacing' now supports specifying spacing above the line.
@@ -4385,16 +4384,20 @@ allowing Emacs users access to speech recognition utilities.
Note: Accepting this permission allows the use of system APIs, which may
send user data to Apple's speech recognition servers.
@@ -95,13 +109,13 @@ index 7f917f93b2..d7631fa6c7 100644
+ jumps (]], M-<, xref, imenu, etc.).
+- Tab-navigable interactive spans (buttons, links, completion
+ candidates) within a buffer.
+- Completion announcements for the *Completions* buffer and overlay
+ and child-frame completion UIs (Vertico, Corfu, Company-box).
+- Completion announcements for the *Completions* buffer, overlay
+ completion UIs, and child-frame completion popup UIs.
Set 'ns-accessibility-enabled' to nil to disable the accessibility
interface and eliminate the associated overhead.
diff --git a/src/nsterm.h b/src/nsterm.h
index 21a93bc799..bdd40b8eb7 100644
index 21a93bc799..b5c9f84499 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -504,9 +504,20 @@ typedef struct ns_ax_visible_run
@@ -140,16 +154,17 @@ index 21a93bc799..bdd40b8eb7 100644
#endif
BOOL font_panel_active;
NSFont *font_panel_result;
@@ -665,6 +684,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
@@ -665,6 +684,8 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
- (void)rebuildAccessibilityTree;
- (void)invalidateAccessibilityTree;
- (void)postAccessibilityUpdates;
+- (void)postEchoAreaAnnouncementIfNeeded;
+- (void)announceChildFrameCompletion;
#endif
@end
diff --git a/src/nsterm.m b/src/nsterm.m
index 8ef344d9fe..5038f9830d 100644
index 8ef344d9fe..1acb64630a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1275,7 +1275,13 @@ If a completion candidate is selected (overlay or child frame),
@@ -185,7 +200,7 @@ index 8ef344d9fe..5038f9830d 100644
{
NSRect windowRect = [view convertRect:r toView:nil];
NSRect screenRect
@@ -7407,6 +7413,112 @@ visual line index for Zoom (skip whitespace-only lines
@@ -7407,6 +7413,117 @@ visual line index for Zoom (skip whitespace-only lines
return nil;
}
@@ -232,6 +247,11 @@ index 8ef344d9fe..5038f9830d 100644
+ The data pointer is used only in this loop, before Lisp calls. */
+ const unsigned char *data = SDATA (str);
+ ptrdiff_t byte_len = SBYTES (str);
+ /* 128 lines is a safe upper bound for a completion child frame.
+ The caller rejects buffers larger than 10,000 characters
+ (BUF_ZV(b) - BUF_BEGV(b) > 10000 guard in announceChildFrameCompletion),
+ so the worst case is ~10 KB / 1 byte per line < 128. If a future
+ caller removes that guard, lines beyond 128 are silently skipped; */
+ ptrdiff_t line_starts[128];
+ ptrdiff_t line_ends[128];
+ int nlines = 0;
@@ -298,7 +318,7 @@ index 8ef344d9fe..5038f9830d 100644
/* 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
@@ -7440,9 +7552,13 @@ visual line index for Zoom (skip whitespace-only lines
@@ -7440,9 +7557,13 @@ visual line index for Zoom (skip whitespace-only lines
return @"";
specpdl_ref count = SPECPDL_INDEX ();
@@ -313,7 +333,7 @@ index 8ef344d9fe..5038f9830d 100644
if (b != current_buffer)
set_buffer_internal_1 (b);
@@ -8605,6 +8721,11 @@ - (void)setAccessibilitySelectedTextRange:(NSRange)range
@@ -8605,6 +8726,11 @@ - (void)setAccessibilitySelectedTextRange:(NSRange)range
[self ensureTextCache];
@@ -325,7 +345,7 @@ index 8ef344d9fe..5038f9830d 100644
specpdl_ref count = SPECPDL_INDEX ();
record_unwind_current_buffer ();
/* Ensure block_input is always matched by unblock_input even if
@@ -9053,20 +9174,38 @@ - (void)postFocusedCursorNotification:(ptrdiff_t)point
@@ -9053,20 +9179,38 @@ - (void)postFocusedCursorNotification:(ptrdiff_t)point
&& granularity
== ns_ax_text_selection_granularity_character);
@@ -374,7 +394,7 @@ index 8ef344d9fe..5038f9830d 100644
ns_ax_post_notification_with_info (
self,
NSAccessibilitySelectedTextChangedNotification,
@@ -9166,12 +9305,17 @@ user expectation ("w" jumps to next word and reads it). */
@@ -9166,12 +9310,17 @@ user expectation ("w" jumps to next word and reads it). */
}
}
@@ -397,7 +417,7 @@ index 8ef344d9fe..5038f9830d 100644
if (cachedText
&& granularity == ns_ax_text_selection_granularity_line)
{
@@ -9236,6 +9380,11 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b
@@ -9236,6 +9385,11 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b
block_input ();
specpdl_ref count2 = SPECPDL_INDEX ();
@@ -409,7 +429,7 @@ index 8ef344d9fe..5038f9830d 100644
record_unwind_protect_void (unblock_input);
record_unwind_current_buffer ();
if (b != current_buffer)
@@ -9412,12 +9561,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
@@ -9412,12 +9566,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
if (!b)
return;
@@ -439,7 +459,7 @@ index 8ef344d9fe..5038f9830d 100644
if (modiff != self.cachedModiff)
{
self.cachedModiff = modiff;
@@ -9431,6 +9597,7 @@ Text property changes (e.g. face updates from
@@ -9431,6 +9602,7 @@ Text property changes (e.g. face updates from
{
self.cachedCharsModiff = chars_modiff;
[self postTextChangedNotification:point];
@@ -447,7 +467,7 @@ index 8ef344d9fe..5038f9830d 100644
}
}
@@ -9453,8 +9620,15 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9453,8 +9625,15 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
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.
@@ -465,7 +485,7 @@ index 8ef344d9fe..5038f9830d 100644
goto skip_overlay_scan;
int selected_line = -1;
@@ -9500,7 +9674,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9500,7 +9679,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
self.cachedPoint = point;
self.cachedMarkActive = markActive;
@@ -485,7 +505,7 @@ index 8ef344d9fe..5038f9830d 100644
NSInteger direction = ns_ax_text_selection_direction_discontiguous;
if (point > oldPoint)
direction = ns_ax_text_selection_direction_next;
@@ -9512,6 +9697,7 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9512,6 +9702,7 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
/* --- Granularity detection --- */
NSInteger granularity = ns_ax_text_selection_granularity_unknown;
@@ -493,7 +513,7 @@ index 8ef344d9fe..5038f9830d 100644
[self ensureTextCache];
if (cachedText && oldPoint > 0)
{
@@ -9526,7 +9712,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9526,7 +9717,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
NSRange newLine = [cachedText lineRangeForRange:
NSMakeRange (newIdx, 0)];
if (oldLine.location != newLine.location)
@@ -513,7 +533,7 @@ index 8ef344d9fe..5038f9830d 100644
else
{
NSUInteger dist = (newIdx > oldIdx
@@ -9548,34 +9745,23 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9548,34 +9750,23 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
granularity = ns_ax_text_selection_granularity_line;
}
@@ -561,7 +581,7 @@ index 8ef344d9fe..5038f9830d 100644
{
NSWindow *win = [self.emacsView window];
if (win)
@@ -9734,6 +9920,13 @@ - (NSRect)accessibilityFrame
@@ -9734,6 +9925,13 @@ - (NSRect)accessibilityFrame
if (vis_start >= vis_end)
return @[];
@@ -575,7 +595,7 @@ index 8ef344d9fe..5038f9830d 100644
block_input ();
specpdl_ref blk_count = SPECPDL_INDEX ();
record_unwind_protect_void (unblock_input);
@@ -10040,6 +10233,10 @@ - (void)dealloc
@@ -10040,6 +10238,10 @@ - (void)dealloc
#endif
[accessibilityElements release];
@@ -586,7 +606,7 @@ index 8ef344d9fe..5038f9830d 100644
[[self menu] release];
[super dealloc];
}
@@ -11489,6 +11686,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
@@ -11489,6 +11691,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
windowClosing = NO;
processingCompose = NO;
@@ -596,7 +616,7 @@ index 8ef344d9fe..5038f9830d 100644
scrollbarsNeedingUpdate = 0;
fs_state = FULLSCREEN_NONE;
fs_before_fs = next_maximized = -1;
@@ -12797,6 +12997,159 @@ - (id)accessibilityFocusedUIElement
@@ -12797,6 +13002,161 @@ - (id)accessibilityFocusedUIElement
The existing elements carry cached state (modiff, point) from the
previous redisplay cycle. Rebuilding first would create fresh
elements with current values, making change detection impossible. */
@@ -697,8 +717,10 @@ index 8ef344d9fe..5038f9830d 100644
+ childFrameLastBuffer = BVAR (b, name);
+ childFrameLastModiff = modiff;
+
+ if (!BUFFER_LIVE_P (b))
+ return;
+ /* BUFFER_LIVE_P(b) is already checked at entry (line above the
+ EQ comparison). No code between that check and here can kill
+ the buffer, so this second check is redundant. */
+ eassert (BUFFER_LIVE_P (b));
+
+ /* Skip buffers larger than a typical completion popup.
+ This avoids scanning eldoc, which-key, or other child
@@ -756,7 +778,7 @@ index 8ef344d9fe..5038f9830d 100644
- (void)postAccessibilityUpdates
{
NSTRACE ("[EmacsView postAccessibilityUpdates]");
@@ -12807,11 +13160,69 @@ - (void)postAccessibilityUpdates
@@ -12807,11 +13167,69 @@ - (void)postAccessibilityUpdates
/* Re-entrance guard: VoiceOver callbacks during notification posting
can trigger redisplay, which calls ns_update_end, which calls us