Skip to content

Commit

Permalink
Use runtime knowledge of OS for OS-specific text editing (#3840)
Browse files Browse the repository at this point in the history
How text editing behaves depends on what OS we are running on.

`target_os` is a compile-time check, but `ctx.os()` is a runtime check,
that works also on web.
  • Loading branch information
emilk authored Jan 19, 2024
1 parent b766a48 commit f7ec8f2
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/egui/src/widgets/label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ fn process_selection_key_events(
}

event => {
cursor_range.on_event(event, galley, widget_id);
cursor_range.on_event(ui.ctx().os(), event, galley, widget_id);
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions crates/egui/src/widgets/text_edit/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use std::sync::Arc;

use epaint::text::{cursor::*, Galley, LayoutJob};

use crate::{output::OutputEvent, text_edit::cursor_interaction::cursor_rect, *};
use crate::{
os::OperatingSystem, output::OutputEvent, text_edit::cursor_interaction::cursor_rect, *,
};

use super::{
cursor_interaction::{ccursor_next_word, ccursor_previous_word, find_line_start},
Expand Down Expand Up @@ -773,6 +775,8 @@ fn events(
char_limit: usize,
event_filter: EventFilter,
) -> (bool, CursorRange) {
let os = ui.ctx().os();

let mut cursor_range = state.cursor.range(galley).unwrap_or(default_cursor_range);

// We feed state to the undoer both before and after handling input
Expand All @@ -794,7 +798,7 @@ fn events(
for event in &events {
let did_mutate_text = match event {
// First handle events that only changes the selection cursor, not the text:
event if cursor_range.on_event(event, galley, id) => None,
event if cursor_range.on_event(os, event, galley, id) => None,

Event::Copy => {
if cursor_range.is_empty() {
Expand Down Expand Up @@ -909,7 +913,7 @@ fn events(
key,
pressed: true,
..
} => check_for_mutating_key_press(&mut cursor_range, text, galley, modifiers, *key),
} => check_for_mutating_key_press(os, &mut cursor_range, text, galley, modifiers, *key),

Event::CompositionStart => {
state.has_ime = true;
Expand Down Expand Up @@ -1143,6 +1147,7 @@ fn delete_paragraph_after_cursor(

/// Returns `Some(new_cursor)` if we did mutate `text`.
fn check_for_mutating_key_press(
os: OperatingSystem,
cursor_range: &mut CursorRange,
text: &mut dyn TextBuffer,
galley: &Galley,
Expand All @@ -1166,7 +1171,7 @@ fn check_for_mutating_key_press(
Some(CCursorRange::one(ccursor))
}

Key::Delete if !modifiers.shift || !cfg!(target_os = "windows") => {
Key::Delete if !modifiers.shift || os != OperatingSystem::Windows => {
let ccursor = if modifiers.mac_cmd {
delete_paragraph_after_cursor(text, galley, cursor_range)
} else if let Some(cursor) = cursor_range.single() {
Expand Down
36 changes: 27 additions & 9 deletions crates/egui/src/widgets/text_edit/cursor_range.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use epaint::{text::cursor::*, Galley};

use crate::{Event, Id, Key, Modifiers};
use crate::{os::OperatingSystem, Event, Id, Key, Modifiers};

use super::cursor_interaction::{ccursor_next_word, ccursor_previous_word, slice_char_range};

Expand Down Expand Up @@ -115,7 +115,13 @@ impl CursorRange {
/// Check for key presses that are moving the cursor.
///
/// Returns `true` if we did mutate `self`.
pub fn on_key_press(&mut self, galley: &Galley, modifiers: &Modifiers, key: Key) -> bool {
pub fn on_key_press(
&mut self,
os: OperatingSystem,
galley: &Galley,
modifiers: &Modifiers,
key: Key,
) -> bool {
match key {
Key::A if modifiers.command => {
*self = Self::select_all(galley);
Expand All @@ -137,17 +143,17 @@ impl CursorRange {
| Key::ArrowDown
| Key::Home
| Key::End => {
move_single_cursor(&mut self.primary, galley, key, modifiers);
move_single_cursor(os, &mut self.primary, galley, key, modifiers);
if !modifiers.shift {
self.secondary = self.primary;
}
true
}

Key::P | Key::N | Key::B | Key::F | Key::A | Key::E
if cfg!(target_os = "macos") && modifiers.ctrl && !modifiers.shift =>
if os == OperatingSystem::Mac && modifiers.ctrl && !modifiers.shift =>
{
move_single_cursor(&mut self.primary, galley, key, modifiers);
move_single_cursor(os, &mut self.primary, galley, key, modifiers);
self.secondary = self.primary;
true
}
Expand All @@ -159,14 +165,20 @@ impl CursorRange {
/// Check for events that modify the cursor range.
///
/// Returns `true` if such an event was found and handled.
pub fn on_event(&mut self, event: &Event, galley: &Galley, _widget_id: Id) -> bool {
pub fn on_event(
&mut self,
os: OperatingSystem,
event: &Event,
galley: &Galley,
_widget_id: Id,
) -> bool {
match event {
Event::Key {
modifiers,
key,
pressed: true,
..
} => self.on_key_press(galley, modifiers, *key),
} => self.on_key_press(os, galley, modifiers, *key),

#[cfg(feature = "accesskit")]
Event::AccessKitActionRequest(accesskit::ActionRequest {
Expand Down Expand Up @@ -287,8 +299,14 @@ fn ccursor_from_accesskit_text_position(
// ----------------------------------------------------------------------------

/// Move a text cursor based on keyboard
fn move_single_cursor(cursor: &mut Cursor, galley: &Galley, key: Key, modifiers: &Modifiers) {
if cfg!(target_os = "macos") && modifiers.ctrl && !modifiers.shift {
fn move_single_cursor(
os: OperatingSystem,
cursor: &mut Cursor,
galley: &Galley,
key: Key,
modifiers: &Modifiers,
) {
if os == OperatingSystem::Mac && modifiers.ctrl && !modifiers.shift {
match key {
Key::A => *cursor = galley.cursor_begin_of_row(cursor),
Key::E => *cursor = galley.cursor_end_of_row(cursor),
Expand Down
9 changes: 9 additions & 0 deletions scripts/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ def lint_lines(filepath, lines_in):
)
lines_out.append("#[inline]")

if (
"(target_os" in line
and filepath.startswith("./crates/egui/")
and filepath != "./crates/egui/src/os.rs"
):
errors.append(
f"{filepath}:{line_nr}: Don't use `target_os` - use ctx.os() instead."
)

lines_out.append(line)

prev_line = line
Expand Down

0 comments on commit f7ec8f2

Please sign in to comment.