Skip to content

Fix down arrow key not working after font size change #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
objectiveous wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix down arrow key not working after font size change #364

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b8e8fb
Fix down arrow key not working after font size change
objectiveous Feb 8, 2026
3335aee
Address PR #364 review comments from Copilot
objectiveous Feb 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 58 additions & 1 deletion Sources/CodeEditSourceEditor/Controller/TextViewController+Lifecycle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,17 @@
}
}

func handleEvent(event: NSEvent) -> NSEvent? {

Check failure on line 207 in Sources/CodeEditSourceEditor/Controller/TextViewController+Lifecycle.swift

View workflow job for this annotation

GitHub Actions / SwiftLint / SwiftLint 

Function should have complexity 10 or less; currently complexity is 11 (cyclomatic_complexity)
let modifierFlags = event.modifierFlags.intersection(.deviceIndependentFlagsMask)
switch event.type {
case .keyDown:
let tabKey: UInt16 = 0x30
let downArrow: UInt16 = 125
let upArrow: UInt16 = 126

if event.keyCode == tabKey {
if event.keyCode == downArrow || event.keyCode == upArrow {
return self.handleArrowKey(event: event, modifierFlags: modifierFlags)
} else if event.keyCode == tabKey {
return self.handleTab(event: event, modifierFlags: modifierFlags.rawValue)
} else {
return self.handleCommand(event: event, modifierFlags: modifierFlags)
Expand Down Expand Up @@ -243,7 +247,7 @@
}
}

func handleCommand(event: NSEvent, modifierFlags: NSEvent.ModifierFlags) -> NSEvent? {

Check failure on line 250 in Sources/CodeEditSourceEditor/Controller/TextViewController+Lifecycle.swift

View workflow job for this annotation

GitHub Actions / SwiftLint / SwiftLint 

Function should have complexity 10 or less; currently complexity is 12 (cyclomatic_complexity)
let commandKey = NSEvent.ModifierFlags.command
let controlKey = NSEvent.ModifierFlags.control

Expand Down Expand Up @@ -276,11 +280,64 @@
}
jumpToDefinitionModel.performJump(at: cursor.range)
return nil
case (controlKey, "n"):
self.textView.moveDown(nil)
return nil
case (controlKey, "p"):
self.textView.moveUp(nil)
return nil
Comment on lines +291 to +296
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

The new Ctrl-N / Ctrl-P shortcuts match only when modifierFlags equals exactly .control. Because modifierFlags is built from .deviceIndependentFlagsMask, it can include extra flags (e.g. Caps Lock / Fn / numericPad), which would prevent these bindings from firing. Consider normalizing the flags to only [.shift, .control, .option, .command] before switching, or using .contains(.control) with explicit exclusions for other modifiers as needed.

Copilot uses AI. Check for mistakes.
case (_, _):
return event
}
}

/// Handles up/down arrow key events with all modifier combinations.
/// Dispatches the appropriate movement method on the text view and consumes the event.
///
/// - Returns: `nil` to consume the event after dispatching the movement action.
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

Doc comment says this handles arrow keys "with all modifier combinations", but the implementation only dispatches for Shift/Option/Command combinations and returns the event for others (e.g. Control+Arrow). Please adjust the wording to match what is actually handled so readers don’t assume broader coverage.

Suggested change
/// Handles up/down arrow key events with all modifier combinations.
/// Dispatches the appropriate movement method on the text view and consumes the event.
///
/// - Returns: `nil` to consume the event after dispatching the movement action.
/// Handles up/down arrow key events for plain, Shift, Option, Command,
/// and their combinations among these modifiers.
/// Dispatches the appropriate movement method on the text view and consumes the event.
///
/// - Returns: `nil` to consume the event after dispatching the movement action,
/// or the original event for unsupported modifier combinations.

Copilot uses AI. Check for mistakes.
private func handleArrowKey(event: NSEvent, modifierFlags: NSEvent.ModifierFlags) -> NSEvent? {

Check failure on line 298 in Sources/CodeEditSourceEditor/Controller/TextViewController+Lifecycle.swift

View workflow job for this annotation

GitHub Actions / SwiftLint / SwiftLint 

Function should have complexity 10 or less; currently complexity is 13 (cyclomatic_complexity)
let isDown = event.keyCode == 125
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

handleEvent defines downArrow/upArrow constants, but handleArrowKey re-hardcodes 125 to compute isDown. To reduce magic numbers and keep key-code mapping consistent, reuse the same constants (or centralize them) instead of duplicating the raw value.

Suggested change
let isDown = event.keyCode == 125
let downArrow: UInt16 = 125
let upArrow: UInt16 = 126
let isDown = event.keyCode == downArrow

Copilot uses AI. Check for mistakes.
let shift = modifierFlags.contains(.shift)
let option = modifierFlags.contains(.option)
let command = modifierFlags.contains(.command)

switch (isDown, shift, option, command) {
// Plain arrow
case (true, false, false, false):
self.textView.moveDown(nil)
case (false, false, false, false):
self.textView.moveUp(nil)
// Shift+Arrow (extend selection)
case (true, true, false, false):
self.textView.moveDownAndModifySelection(nil)
case (false, true, false, false):
self.textView.moveUpAndModifySelection(nil)
// Option+Arrow (paragraph)
case (true, false, true, false):
self.textView.moveToEndOfParagraph(nil)
case (false, false, true, false):
self.textView.moveToBeginningOfParagraph(nil)
// Cmd+Arrow (document)
case (true, false, false, true):
self.textView.moveToEndOfDocument(nil)
case (false, false, false, true):
self.textView.moveToBeginningOfDocument(nil)
// Shift+Option+Arrow (extend selection to paragraph)
case (true, true, true, false):
self.textView.moveToEndOfParagraphAndModifySelection(nil)
case (false, true, true, false):
self.textView.moveToBeginningOfParagraphAndModifySelection(nil)
// Shift+Cmd+Arrow (extend selection to document)
case (true, true, false, true):
self.textView.moveToEndOfDocumentAndModifySelection(nil)
case (false, true, false, true):
self.textView.moveToBeginningOfDocumentAndModifySelection(nil)
default:
return event
}
return nil
}

/// Handles the tab key event.
/// If the Shift key is pressed, it handles unindenting. If no modifier key is pressed, it checks if multiple lines
/// are highlighted and handles indenting accordingly.
Expand Down
8 changes: 8 additions & 0 deletions ...CodeEditSourceEditor/SourceEditorConfiguration/SourceEditorConfiguration+Appearance.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ extension SourceEditorConfiguration {
controller.textView.font = font
controller.textView.typingAttributes = controller.attributesFor(nil)
controller.gutterView.font = font.rulerFont
// Force the layout manager to recalculate its cached estimated line height.
// The estimate is cached and not invalidated by font changes, causing vertical
// cursor movement (moveDown:) to use stale values and fail to cross line boundaries.
let renderDelegate = controller.textView.layoutManager.renderDelegate
controller.textView.layoutManager.renderDelegate = renderDelegate
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

The layout-manager cache invalidation is implemented by reassigning renderDelegate to itself. This works but is duplicated (and relies on a non-obvious side effect). Consider extracting this into a small helper (e.g., refreshEstimatedLineHeightCache() on the controller/layout manager) so the intent is clearer and the logic is not repeated.

Copilot uses AI. Check for mistakes.
needsHighlighterInvalidation = true
}

Expand All @@ -103,6 +108,9 @@ extension SourceEditorConfiguration {

if oldConfig?.lineHeightMultiple != lineHeightMultiple {
controller.textView.layoutManager.lineHeightMultiplier = lineHeightMultiple
// Also invalidate the cached estimated line height (same issue as font change above).
let renderDelegate = controller.textView.layoutManager.renderDelegate
controller.textView.layoutManager.renderDelegate = renderDelegate
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

Same as the font-change branch above: the renderDelegate self-reassignment cache invalidation is duplicated. Please centralize this into a helper to avoid repeating the same workaround in multiple places.

Copilot uses AI. Check for mistakes.
}

if oldConfig?.wrapLines != wrapLines {
Expand Down
Loading