From da6b478fbe0565c6b780267a51c4198efd3693bc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 14 Apr 2025 12:34:24 -0700 Subject: [PATCH 1/5] terminal: clear correct row on index operation in certain edge cases Fixes #7066 This fixes an issue where under certain conditions (expanded below), we would not clear the correct row, leading to the screen having duplicate data. This was triggered by a page state of the following: ``` +----------+ = PAGE 0 ... : : 4305 |1ABCD00000| 4306 |2EFGH00000| :^ : = PIN 0 +-------------+ ACTIVE 4307 |3IJKL00000| | 0 +----------+ : +----------+ : = PAGE 1 0 | | | 1 1 | | | 2 +----------+ : +-------------+ ``` Namely, the cursor had to NOT be on the last row of the first page, but somewhere on the first page. Then, when an `index` (LF) operation was performed the result would look like this: ``` +----------+ = PAGE 0 ... : : 4305 |1ABCD00000| 4306 |2EFGH00000| +-------------+ ACTIVE 4307 |3IJKL00000| | 0 :^ : : = PIN 0 +----------+ : +----------+ : = PAGE 1 0 |3IJKL00000| | 1 1 | | | 2 +----------+ : +-------------+ ``` The `3IJKL` line was duplicated. What was happening here is that we performed the index operation correctly but failed to clear the cursor line as expected. This is because we were always clearing the first row in the page instead of the row of the cursor. Test added. --- src/terminal/Screen.zig | 92 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 0772bfa75..9ab4b23e2 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -924,8 +924,8 @@ fn cursorScrollAboveRotate(self: *Screen) !void { fastmem.rotateOnceR(Row, cur_rows[self.cursor.page_pin.y..cur_page.size.rows]); self.clearCells( cur_page, - &cur_rows[0], - cur_page.getCells(&cur_rows[0]), + &cur_rows[self.cursor.page_pin.y], + cur_page.getCells(&cur_rows[self.cursor.page_pin.y]), ); // Set all the rows we rotated and cleared dirty @@ -1256,6 +1256,17 @@ pub fn clearCells( self.assertIntegrity(); } + if (comptime std.debug.runtime_safety) { + // Our row and cells should be within the page. + const page_rows = page.rows.ptr(page.memory.ptr); + assert(@intFromPtr(row) >= @intFromPtr(&page_rows[0])); + assert(@intFromPtr(row) <= @intFromPtr(&page_rows[page.size.rows - 1])); + + const row_cells = page.getCells(row); + assert(@intFromPtr(&cells[0]) >= @intFromPtr(&row_cells[0])); + assert(@intFromPtr(&cells[cells.len - 1]) <= @intFromPtr(&row_cells[row_cells.len - 1])); + } + // If this row has graphemes, then we need go through a slow path // and delete the cell graphemes. if (row.grapheme) { @@ -4818,6 +4829,83 @@ test "Screen: scroll above creates new page" { try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } +test "Screen: scroll above with cursor on non-final row" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 4, 10); + defer s.deinit(); + + // Get the cursor to be 2 rows above a new page + const first_page_size = s.pages.pages.first.?.data.capacity.rows; + s.pages.pages.first.?.data.pauseIntegrityChecks(true); + for (0..first_page_size - 3) |_| try s.testWriteString("\n"); + s.pages.pages.first.?.data.pauseIntegrityChecks(false); + + // Write 3 lines of text, forcing the last line into the first + // row of a new page. Move our cursor onto the previous page. + try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } }); + try s.testWriteString("1AB\n2BC\n3DE\n4FG"); + s.cursorAbsolute(0, 1); + s.pages.clearDirty(); + + // Ensure we're still on the first page. So our cursor is on the first + // page but we have two pages of data. + try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + + // +----------+ = PAGE 0 + // ... : : + // +-------------+ ACTIVE + // 4305 |1AB0000000| | 0 + // 4306 |2BC0000000| | 1 + // :^ : : = PIN 0 + // 4307 |3DE0000000| | 2 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |4FG0000000| | 3 + // +----------+ : + // +-------------+ + try s.cursorScrollAbove(); + + // +----------+ = PAGE 0 + // ... : : + // 4305 |1AB0000000| + // +-------------+ ACTIVE + // 4306 |2BC0000000| | 0 + // 4307 | | | 1 + // :^ : : = PIN 0 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |3DE0000000| | 2 + // 1 |4FG0000000| | 3 + // +----------+ : + // +-------------+ + // try s.pages.diagram(std.io.getStdErr().writer()); + + { + const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); + defer alloc.free(contents); + try testing.expectEqualStrings("2BC\n\n3DE\n4FG", contents); + } + { + const list_cell = s.pages.getCell(.{ .active = .{ .x = 0, .y = 1 } }).?; + const cell = list_cell.cell; + try testing.expect(cell.content_tag == .bg_color_rgb); + try testing.expectEqual(Cell.RGB{ + .r = 155, + .g = 0, + .b = 0, + }, cell.content.color_rgb); + } + + // Page 0's penultimate row is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0's final row is dirty because it was cleared. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + // Page 1's row is dirty because it's new. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); +} + test "Screen: scroll above no scrollback bottom of page" { const testing = std.testing; const alloc = testing.allocator; From b77c5634f0ad4e43128ba9c614a743cbd1dcdd37 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 15 Apr 2025 08:52:00 -0700 Subject: [PATCH 2/5] macos: quick terminal uses padded notch mode if notch is visible Fixes #6612 --- .../QuickTerminalController.swift | 20 ++++++++++++------- .../Sources/Helpers/NSScreen+Extension.swift | 7 +++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift index 6e5607c6f..1abe30da1 100644 --- a/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift +++ b/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift @@ -495,14 +495,20 @@ class QuickTerminalController: BaseTerminalController { private func onToggleFullscreen() { // We ignore the configured fullscreen style and always use non-native // because the way the quick terminal works doesn't support native. - // - // An additional detail is that if the is NOT frontmost, then our - // NSApp.presentationOptions will not take effect so we must always - // do the visible menu mode since we can't get rid of the menu. - let mode: FullscreenMode = if (NSApp.isFrontmost) { - .nonNative + let mode: FullscreenMode + if (NSApp.isFrontmost) { + // If we're frontmost and we have a notch then we keep padding + // so all lines of the terminal are visible. + if (window?.screen?.hasNotch ?? false) { + mode = .nonNativePaddedNotch + } else { + mode = .nonNative + } } else { - .nonNativeVisibleMenu + // An additional detail is that if the is NOT frontmost, then our + // NSApp.presentationOptions will not take effect so we must always + // do the visible menu mode since we can't get rid of the menu. + mode = .nonNativeVisibleMenu } toggleFullscreen(mode: mode) diff --git a/macos/Sources/Helpers/NSScreen+Extension.swift b/macos/Sources/Helpers/NSScreen+Extension.swift index ef2c02908..675e0b2ec 100644 --- a/macos/Sources/Helpers/NSScreen+Extension.swift +++ b/macos/Sources/Helpers/NSScreen+Extension.swift @@ -34,4 +34,11 @@ extension NSScreen { return visibleFrame.height < (frame.height - max(menuHeight, notchInset) - boundaryAreaPadding) } + + /// Returns true if the screen has a visible notch (i.e., a non-zero safe area inset at the top). + var hasNotch: Bool { + // We assume that a top safe area means notch, since we don't currently + // know any other situation this is true. + return safeAreaInsets.top > 0 + } } From cc690eddb54126b16961e2d5d21782d61e0fa74a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 15 Apr 2025 09:47:52 -0700 Subject: [PATCH 3/5] macOS: Implement basic bell features (no sound) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #7099 This adds basic bell features to macOS to conceptually match the GTK implementation. When a bell is triggered, macOS will do the following: 1. Bounce the dock icon once, if the app isn't already in focus. 2. Add a bell emoji (🔔) to the title of the surface that triggered the bell. This emoji will be removed after the surface is focused or a keyboard event if the surface is already focused. This behavior matches iTerm2. This doesn't add an icon badge because macOS's dockTitle.badgeLabel API wasn't doing anything for me and I wasn't able to fully figure out why... --- macos/Sources/App/macOS/AppDelegate.swift | 11 ++++++++ macos/Sources/Ghostty/Ghostty.App.swift | 27 +++++++++++++++++++ macos/Sources/Ghostty/Ghostty.Config.swift | 14 ++++++++++ macos/Sources/Ghostty/Package.swift | 3 +++ macos/Sources/Ghostty/SurfaceView.swift | 11 +++++++- .../Sources/Ghostty/SurfaceView_AppKit.swift | 21 ++++++++++++++- macos/Sources/Ghostty/SurfaceView_UIKit.swift | 3 +++ src/config/Config.zig | 8 +++++- 8 files changed, 95 insertions(+), 3 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 7d5e7cd25..9d866d734 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -186,6 +186,12 @@ class AppDelegate: NSObject, name: .ghosttyConfigDidChange, object: nil ) + NotificationCenter.default.addObserver( + self, + selector: #selector(ghosttyBellDidRing(_:)), + name: .ghosttyBellDidRing, + object: nil + ) // Configure user notifications let actions = [ @@ -502,6 +508,11 @@ class AppDelegate: NSObject, ghosttyConfigDidChange(config: config) } + @objc private func ghosttyBellDidRing(_ notification: Notification) { + // Bounce the dock icon if we're not focused. + NSApp.requestUserAttention(.informationalRequest) + } + private func ghosttyConfigDidChange(config: Ghostty.Config) { // Update the config we need to store self.derivedConfig = DerivedConfig(config) diff --git a/macos/Sources/Ghostty/Ghostty.App.swift b/macos/Sources/Ghostty/Ghostty.App.swift index ddb954e04..dfd066870 100644 --- a/macos/Sources/Ghostty/Ghostty.App.swift +++ b/macos/Sources/Ghostty/Ghostty.App.swift @@ -538,6 +538,9 @@ extension Ghostty { case GHOSTTY_ACTION_COLOR_CHANGE: colorChange(app, target: target, change: action.action.color_change) + case GHOSTTY_ACTION_RING_BELL: + ringBell(app, target: target) + case GHOSTTY_ACTION_CLOSE_ALL_WINDOWS: fallthrough case GHOSTTY_ACTION_TOGGLE_TAB_OVERVIEW: @@ -747,6 +750,30 @@ extension Ghostty { appDelegate.toggleVisibility(self) } + private static func ringBell( + _ app: ghostty_app_t, + target: ghostty_target_s) { + switch (target.tag) { + case GHOSTTY_TARGET_APP: + // Technically we could still request app attention here but there + // are no known cases where the bell is rang with an app target so + // I think its better to warn. + Ghostty.logger.warning("ring bell does nothing with an app target") + return + + case GHOSTTY_TARGET_SURFACE: + guard let surface = target.target.surface else { return } + guard let surfaceView = self.surfaceView(from: surface) else { return } + NotificationCenter.default.post( + name: .ghosttyBellDidRing, + object: surfaceView + ) + + default: + assertionFailure() + } + } + private static func moveTab( _ app: ghostty_app_t, target: ghostty_target_s, diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index 20a43aa2b..d146477dc 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -116,6 +116,14 @@ extension Ghostty { /// details on what each means. We only add documentation if there is a strange conversion /// due to the embedded library and Swift. + var bellFeatures: BellFeatures { + guard let config = self.config else { return .init() } + var v: CUnsignedInt = 0 + let key = "bell-features" + guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return .init() } + return .init(rawValue: v) + } + var initialWindow: Bool { guard let config = self.config else { return true } var v = true; @@ -543,6 +551,12 @@ extension Ghostty.Config { case download } + struct BellFeatures: OptionSet { + let rawValue: CUnsignedInt + + static let system = BellFeatures(rawValue: 1 << 0) + } + enum MacHidden : String { case never case always diff --git a/macos/Sources/Ghostty/Package.swift b/macos/Sources/Ghostty/Package.swift index cda4b557e..3afca56aa 100644 --- a/macos/Sources/Ghostty/Package.swift +++ b/macos/Sources/Ghostty/Package.swift @@ -253,6 +253,9 @@ extension Notification.Name { /// Resize the window to a default size. static let ghosttyResetWindowSize = Notification.Name("com.mitchellh.ghostty.resetWindowSize") + + /// Ring the bell + static let ghosttyBellDidRing = Notification.Name("com.mitchellh.ghostty.ghosttyBellDidRing") } // NOTE: I am moving all of these to Notification.Name extensions over time. This diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift index beae50331..7eebd3ef1 100644 --- a/macos/Sources/Ghostty/SurfaceView.swift +++ b/macos/Sources/Ghostty/SurfaceView.swift @@ -59,6 +59,15 @@ extension Ghostty { @EnvironmentObject private var ghostty: Ghostty.App + var title: String { + var result = surfaceView.title + if (surfaceView.bell) { + result = "🔔 \(result)" + } + + return result + } + var body: some View { let center = NotificationCenter.default @@ -74,7 +83,7 @@ extension Ghostty { Surface(view: surfaceView, size: geo.size) .focused($surfaceFocus) - .focusedValue(\.ghosttySurfaceTitle, surfaceView.title) + .focusedValue(\.ghosttySurfaceTitle, title) .focusedValue(\.ghosttySurfacePwd, surfaceView.pwd) .focusedValue(\.ghosttySurfaceView, surfaceView) .focusedValue(\.ghosttySurfaceCellSize, surfaceView.cellSize) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 230d3a9e2..1269f2314 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -63,6 +63,9 @@ extension Ghostty { /// dynamically updated. Otherwise, the background color is the default background color. @Published private(set) var backgroundColor: Color? = nil + /// True when the bell is active. This is set inactive on focus or event. + @Published private(set) var bell: Bool = false + // An initial size to request for a window. This will only affect // then the view is moved to a new window. var initialSize: NSSize? = nil @@ -190,6 +193,11 @@ extension Ghostty { selector: #selector(ghosttyColorDidChange(_:)), name: .ghosttyColorDidChange, object: self) + center.addObserver( + self, + selector: #selector(ghosttyBellDidRing(_:)), + name: .ghosttyBellDidRing, + object: self) center.addObserver( self, selector: #selector(windowDidChangeScreen), @@ -300,9 +308,12 @@ extension Ghostty { SecureInput.shared.setScoped(ObjectIdentifier(self), focused: focused) } - // On macOS 13+ we can store our continuous clock... if (focused) { + // On macOS 13+ we can store our continuous clock... focusInstant = ContinuousClock.now + + // We unset our bell state if we gained focus + bell = false } } @@ -556,6 +567,11 @@ extension Ghostty { } } + @objc private func ghosttyBellDidRing(_ notification: SwiftUI.Notification) { + // Bell state goes to true + bell = true + } + @objc private func windowDidChangeScreen(notification: SwiftUI.Notification) { guard let window = self.window else { return } guard let object = notification.object as? NSWindow, window == object else { return } @@ -855,6 +871,9 @@ extension Ghostty { return } + // On any keyDown event we unset our bell state + bell = false + // We need to translate the mods (maybe) to handle configs such as option-as-alt let translationModsGhostty = Ghostty.eventModifierFlags( mods: ghostty_surface_key_translation_mods( diff --git a/macos/Sources/Ghostty/SurfaceView_UIKit.swift b/macos/Sources/Ghostty/SurfaceView_UIKit.swift index 8ac08d0bd..8d5b3038f 100644 --- a/macos/Sources/Ghostty/SurfaceView_UIKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_UIKit.swift @@ -35,6 +35,9 @@ extension Ghostty { // on supported platforms. @Published var focusInstant: ContinuousClock.Instant? = nil + /// True when the bell is active. This is set inactive on focus or event. + @Published var bell: Bool = false + // Returns sizing information for the surface. This is the raw C // structure because I'm lazy. var surfaceSize: ghostty_surface_size_s? { diff --git a/src/config/Config.zig b/src/config/Config.zig index f648e8a28..d57ed161b 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1874,7 +1874,13 @@ keybind: Keybinds = .{}, /// for instance under the "Sound > Alert Sound" setting in GNOME, /// or the "Accessibility > System Bell" settings in KDE Plasma. /// -/// Currently only implemented on Linux. +/// On macOS this has no affect. +/// +/// On macOS, if the app is unfocused, it will bounce the app icon in the dock +/// once. Additionally, the title of the window with the alerted terminal +/// surface will contain a bell emoji (🔔) until the terminal is focused +/// or a key is pressed. These are not currently configurable since they're +/// considered unobtrusive. @"bell-features": BellFeatures = .{}, /// Control the in-app notifications that Ghostty shows. From bef0d5d88e2289a48bef29bf0aa8017b52ca9e68 Mon Sep 17 00:00:00 2001 From: ekusiadadus Date: Fri, 18 Apr 2025 07:22:40 +0900 Subject: [PATCH 4/5] fix: spelling inconsistencies for Japanese Translations --- po/ja_JP.UTF-8.po | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/po/ja_JP.UTF-8.po b/po/ja_JP.UTF-8.po index 7a4ee6929..e06ec2fbc 100644 --- a/po/ja_JP.UTF-8.po +++ b/po/ja_JP.UTF-8.po @@ -1,5 +1,5 @@ # Japanese translations for com.mitchellh.ghostty package -# com.mitchellh.ghostty パッケージに対する英訳. +# com.mitchellh.ghostty パッケージに対する和訳. # Copyright (C) 2025 Mitchell Hashimoto # This file is distributed under the same license as the com.mitchellh.ghostty package. # Lon Sagisawa , 2025. @@ -37,7 +37,7 @@ msgstr "OK" #: src/apprt/gtk/ui/1.5/config-errors-dialog.blp:5 msgid "Configuration Errors" -msgstr "設定エラー" +msgstr "設定ファイルにエラーがあります" #: src/apprt/gtk/ui/1.5/config-errors-dialog.blp:6 msgid "" @@ -192,7 +192,7 @@ msgstr "" #: src/apprt/gtk/ui/1.5/ccw-paste.blp:6 msgid "Warning: Potentially Unsafe Paste" -msgstr "警告: 危険な可能性のあるペースト" +msgstr "警告: 危険な可能性のある貼り付け" #: src/apprt/gtk/ui/1.5/ccw-paste.blp:7 msgid "" @@ -232,19 +232,19 @@ msgstr "分割ウィンドウを閉じますか?" #: src/apprt/gtk/CloseDialog.zig:96 msgid "All terminal sessions will be terminated." -msgstr "すべてのターミナルセッションが終了されます。" +msgstr "すべてのターミナルセッションが終了します。" #: src/apprt/gtk/CloseDialog.zig:97 msgid "All terminal sessions in this window will be terminated." -msgstr "ウィンドウ内のすべてのターミナルセッションが終了されます。" +msgstr "ウィンドウ内のすべてのターミナルセッションが終了します。" #: src/apprt/gtk/CloseDialog.zig:98 msgid "All terminal sessions in this tab will be terminated." -msgstr "タブ内のすべてのターミナルセッションが終了されます。" +msgstr "タブ内のすべてのターミナルセッションが終了します。" #: src/apprt/gtk/CloseDialog.zig:99 msgid "The currently running process in this split will be terminated." -msgstr "分割ウィンドウ内のすべてのターミナルセッションが終了されます。" +msgstr "分割ウィンドウ内のすべてのプロセスが終了します。" #: src/apprt/gtk/Window.zig:200 msgid "Main Menu" From 9bab900c753ae75b85bde4a9615a3e0f60492f0b Mon Sep 17 00:00:00 2001 From: phanium <91544758+phanen@users.noreply.github.com> Date: Fri, 18 Apr 2025 02:16:27 +0800 Subject: [PATCH 5/5] vim: fix syntax highlight on scratch buffer --- src/config/vim.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config/vim.zig b/src/config/vim.zig index 6084bd248..17ab0bc2e 100644 --- a/src/config/vim.zig +++ b/src/config/vim.zig @@ -88,6 +88,7 @@ fn writeSyntax(writer: anytype) !void { \\let s:cpo_save = &cpo \\set cpo&vim \\ + \\syn iskeyword @,48-57,- \\syn keyword ghosttyConfigKeyword );