Fix a whackton of warnings and clippy lints

robbert-vdh · robbert-vdh · commit 43860aba84a7 · 2022-02-07T23:10:53.000+01:00
The things remaining are all of the cursor things in the X11
implementation (there's _a lot_ of it, so there's probably a reason why
it's all still there but unused), and the super unsound immutable
reference to mutable reference cast in the macOS implementation that
Clippy automatically errors out on.
diff --git a/src/gl/mod.rs b/src/gl/mod.rs
@@ -1,8 +1,10 @@
-use raw_window_handle::HasRawWindowHandle;
-
 use std::ffi::c_void;
 use std::marker::PhantomData;
 
+// On X11 creating the context is a two step process
+#[cfg(not(target_os = "linux"))]
+use raw_window_handle::HasRawWindowHandle;
+
 #[cfg(target_os = "windows")]
 mod win;
 #[cfg(target_os = "windows")]
diff --git a/src/gl/win.rs b/src/gl/win.rs
@@ -255,7 +255,7 @@ impl GlContext {
 
         let hglrc =
             wglCreateContextAttribsARB.unwrap()(hdc, std::ptr::null_mut(), ctx_attribs.as_ptr());
-        if hglrc == std::ptr::null_mut() {
+        if hglrc.is_null() {
             return Err(GlError::CreationFailed(()));
         }
 
diff --git a/src/keyboard.rs b/src/keyboard.rs
@@ -17,6 +17,7 @@
 
 //! Keyboard types.
 
+#[cfg(any(target_os = "linux", target_os = "macos"))]
 use keyboard_types::{Code, Location};
 
 #[cfg(any(target_os = "linux", target_os = "macos"))]
diff --git a/src/macos/window.rs b/src/macos/window.rs
@@ -44,7 +44,7 @@ pub struct WindowHandle {
 
 impl WindowHandle {
     pub fn close(&mut self) {
-        if let Some(_) = self.raw_window_handle.take() {
+        if self.raw_window_handle.take().is_some() {
             self.close_requested.store(true, Ordering::Relaxed);
         }
     }
diff --git a/src/win/keyboard.rs b/src/win/keyboard.rs
@@ -19,7 +19,6 @@
 
 use std::cmp::Ordering;
 use std::collections::{HashMap, HashSet};
-use std::convert::TryInto;
 use std::mem;
 use std::ops::RangeInclusive;
 
@@ -29,15 +28,15 @@ use winapi::shared::minwindef::{HKL, INT, LPARAM, UINT, WPARAM};
 use winapi::shared::ntdef::SHORT;
 use winapi::shared::windef::HWND;
 use winapi::um::winuser::{
-    GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, VkKeyScanW,
-    MAPVK_VK_TO_CHAR, MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN,
-    VK_BACK, VK_BROWSER_BACK, VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME,
-    VK_BROWSER_REFRESH, VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR,
-    VK_CONTROL, VK_CONVERT, VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF,
-    VK_ESCAPE, VK_EXECUTE, VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5,
-    VK_F6, VK_F7, VK_F8, VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI,
-    VK_LAUNCH_APP1, VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT,
-    VK_LMENU, VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK,
+    GetKeyState, GetKeyboardLayout, MapVirtualKeyExW, PeekMessageW, ToUnicodeEx, MAPVK_VK_TO_CHAR,
+    MAPVK_VSC_TO_VK_EX, PM_NOREMOVE, VK_ACCEPT, VK_ADD, VK_APPS, VK_ATTN, VK_BACK, VK_BROWSER_BACK,
+    VK_BROWSER_FAVORITES, VK_BROWSER_FORWARD, VK_BROWSER_HOME, VK_BROWSER_REFRESH,
+    VK_BROWSER_SEARCH, VK_BROWSER_STOP, VK_CANCEL, VK_CAPITAL, VK_CLEAR, VK_CONTROL, VK_CONVERT,
+    VK_CRSEL, VK_DECIMAL, VK_DELETE, VK_DIVIDE, VK_DOWN, VK_END, VK_EREOF, VK_ESCAPE, VK_EXECUTE,
+    VK_EXSEL, VK_F1, VK_F10, VK_F11, VK_F12, VK_F2, VK_F3, VK_F4, VK_F5, VK_F6, VK_F7, VK_F8,
+    VK_F9, VK_FINAL, VK_HELP, VK_HOME, VK_INSERT, VK_JUNJA, VK_KANA, VK_KANJI, VK_LAUNCH_APP1,
+    VK_LAUNCH_APP2, VK_LAUNCH_MAIL, VK_LAUNCH_MEDIA_SELECT, VK_LCONTROL, VK_LEFT, VK_LMENU,
+    VK_LSHIFT, VK_LWIN, VK_MEDIA_NEXT_TRACK, VK_MEDIA_PLAY_PAUSE, VK_MEDIA_PREV_TRACK,
     VK_MEDIA_STOP, VK_MENU, VK_MODECHANGE, VK_MULTIPLY, VK_NEXT, VK_NONCONVERT, VK_NUMLOCK,
     VK_NUMPAD0, VK_NUMPAD1, VK_NUMPAD2, VK_NUMPAD3, VK_NUMPAD4, VK_NUMPAD5, VK_NUMPAD6, VK_NUMPAD7,
     VK_NUMPAD8, VK_NUMPAD9, VK_OEM_ATTN, VK_OEM_CLEAR, VK_PAUSE, VK_PLAY, VK_PRINT, VK_PRIOR,
@@ -344,110 +343,6 @@ fn vk_to_key(vk: VkCode) -> Option<Key> {
     })
 }
 
-/// Convert a key to a virtual key code.
-///
-/// The virtual key code is needed in various winapi interfaces, including
-/// accelerators. This provides the virtual key code in the current keyboard
-/// map.
-///
-/// The virtual key code can have modifiers in the higher order byte when the
-/// argument is a `Character` variant. See:
-/// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-vkkeyscanw
-pub(crate) fn key_to_vk(key: &Key) -> Option<i32> {
-    Some(match key {
-        Key::Character(s) => {
-            if let Some(code_point) = s.chars().next() {
-                if let Ok(wchar) = (code_point as u32).try_into() {
-                    unsafe { VkKeyScanW(wchar) as i32 }
-                } else {
-                    return None;
-                }
-            } else {
-                return None;
-            }
-        }
-        Key::Cancel => VK_CANCEL,
-        Key::Backspace => VK_BACK,
-        Key::Tab => VK_TAB,
-        Key::Clear => VK_CLEAR,
-        Key::Enter => VK_RETURN,
-        Key::Shift => VK_SHIFT,
-        Key::Control => VK_CONTROL,
-        Key::Alt => VK_MENU,
-        Key::Pause => VK_PAUSE,
-        Key::CapsLock => VK_CAPITAL,
-        // TODO: disambiguate kana and hangul? same vk
-        Key::KanaMode => VK_KANA,
-        Key::JunjaMode => VK_JUNJA,
-        Key::FinalMode => VK_FINAL,
-        Key::KanjiMode => VK_KANJI,
-        Key::Escape => VK_ESCAPE,
-        Key::NonConvert => VK_NONCONVERT,
-        Key::Accept => VK_ACCEPT,
-        Key::PageUp => VK_PRIOR,
-        Key::PageDown => VK_NEXT,
-        Key::End => VK_END,
-        Key::Home => VK_HOME,
-        Key::ArrowLeft => VK_LEFT,
-        Key::ArrowUp => VK_UP,
-        Key::ArrowRight => VK_RIGHT,
-        Key::ArrowDown => VK_DOWN,
-        Key::Select => VK_SELECT,
-        Key::Print => VK_PRINT,
-        Key::Execute => VK_EXECUTE,
-        Key::PrintScreen => VK_SNAPSHOT,
-        Key::Insert => VK_INSERT,
-        Key::Delete => VK_DELETE,
-        Key::Help => VK_HELP,
-        Key::Meta => VK_LWIN,
-        Key::ContextMenu => VK_APPS,
-        Key::Standby => VK_SLEEP,
-        Key::F1 => VK_F1,
-        Key::F2 => VK_F2,
-        Key::F3 => VK_F3,
-        Key::F4 => VK_F4,
-        Key::F5 => VK_F5,
-        Key::F6 => VK_F6,
-        Key::F7 => VK_F7,
-        Key::F8 => VK_F8,
-        Key::F9 => VK_F9,
-        Key::F10 => VK_F10,
-        Key::F11 => VK_F11,
-        Key::F12 => VK_F12,
-        Key::NumLock => VK_NUMLOCK,
-        Key::ScrollLock => VK_SCROLL,
-        Key::BrowserBack => VK_BROWSER_BACK,
-        Key::BrowserForward => VK_BROWSER_FORWARD,
-        Key::BrowserRefresh => VK_BROWSER_REFRESH,
-        Key::BrowserStop => VK_BROWSER_STOP,
-        Key::BrowserSearch => VK_BROWSER_SEARCH,
-        Key::BrowserFavorites => VK_BROWSER_FAVORITES,
-        Key::BrowserHome => VK_BROWSER_HOME,
-        Key::AudioVolumeMute => VK_VOLUME_MUTE,
-        Key::AudioVolumeDown => VK_VOLUME_DOWN,
-        Key::AudioVolumeUp => VK_VOLUME_UP,
-        Key::MediaTrackNext => VK_MEDIA_NEXT_TRACK,
-        Key::MediaTrackPrevious => VK_MEDIA_PREV_TRACK,
-        Key::MediaStop => VK_MEDIA_STOP,
-        Key::MediaPlayPause => VK_MEDIA_PLAY_PAUSE,
-        Key::LaunchMail => VK_LAUNCH_MAIL,
-        Key::LaunchMediaPlayer => VK_LAUNCH_MEDIA_SELECT,
-        Key::LaunchApplication1 => VK_LAUNCH_APP1,
-        Key::LaunchApplication2 => VK_LAUNCH_APP2,
-        Key::Alphanumeric => VK_OEM_ATTN,
-        Key::Convert => VK_CONVERT,
-        Key::ModeChange => VK_MODECHANGE,
-        Key::Process => VK_PROCESSKEY,
-        Key::Attn => VK_ATTN,
-        Key::CrSel => VK_CRSEL,
-        Key::ExSel => VK_EXSEL,
-        Key::EraseEof => VK_EREOF,
-        Key::Play => VK_PLAY,
-        Key::ZoomToggle => VK_ZOOM,
-        _ => return None,
-    })
-}
-
 fn code_unit_to_key(code_unit: u32) -> Key {
     match code_unit {
         0x8 | 0x7F => Key::Backspace,
@@ -692,6 +587,7 @@ impl KeyboardState {
                 key_state[VK_LCONTROL as usize] = if has_altgr { 0x80 } else { 0 };
                 key_state[VK_MENU as usize] = if has_altgr { 0x80 } else { 0 };
                 key_state[VK_RMENU as usize] = if has_altgr { 0x80 } else { 0 };
+                #[allow(clippy::iter_overeager_cloned)]
                 for vk in PRINTABLE_VKS.iter().cloned().flatten() {
                     let ret = ToUnicodeEx(
                         vk as UINT,
diff --git a/src/win/window.rs b/src/win/window.rs
@@ -205,11 +205,8 @@ unsafe extern "system" fn wnd_proc(
                 }
             }
             WM_TIMER => {
-                match wparam {
-                    WIN_FRAME_TIMER => {
-                        window_state.handler.on_frame(&mut window);
-                    }
-                    _ => (),
+                if wparam == WIN_FRAME_TIMER {
+                    window_state.handler.on_frame(&mut window);
                 }
                 return 0;
             }
@@ -414,8 +411,8 @@ impl Window {
                     break;
                 }
 
-                TranslateMessage(&mut msg);
-                DispatchMessageW(&mut msg);
+                TranslateMessage(&msg);
+                DispatchMessageW(&msg);
             }
         }
     }
diff --git a/src/x11/cursor.rs b/src/x11/cursor.rs
@@ -101,5 +101,5 @@ pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor)
         MouseCursor::RowResize => loadn(&[b"split_v\0", b"v_double_arrow\0"]),
     };
 
-    cursor.or(load(b"left_ptr\0")).unwrap_or(0)
+    cursor.or_else(|| load(b"left_ptr\0")).unwrap_or(0)
 }
diff --git a/src/x11/window.rs b/src/x11/window.rs
@@ -35,7 +35,7 @@ pub struct WindowHandle {
 
 impl WindowHandle {
     pub fn close(&mut self) {
-        if let Some(_) = self.raw_window_handle.take() {
+        if self.raw_window_handle.take().is_some() {
             // FIXME: This will need to be changed from just setting an atomic to somehow
             // synchronizing with the window being closed (using a synchronous channel, or
             // by joining on the event loop thread).
@@ -96,6 +96,7 @@ pub struct Window {
     xcb_connection: XcbConnection,
     window_id: u32,
     window_info: WindowInfo,
+    // FIXME: There's all this mouse cursor logic but it's never actually used, is this correct?
     mouse_cursor: MouseCursor,
 
     frame_interval: Duration,
@@ -135,7 +136,7 @@ impl Window {
 
         let (parent_handle, mut window_handle) = ParentHandle::new();
 
-        let thread = thread::spawn(move || {
+        thread::spawn(move || {
             Self::window_thread(Some(parent_id), options, build, tx.clone(), Some(parent_handle));
         });
 
@@ -155,7 +156,7 @@ impl Window {
 
         let (parent_handle, mut window_handle) = ParentHandle::new();
 
-        let thread = thread::spawn(move || {
+        thread::spawn(move || {
             Self::window_thread(None, options, build, tx.clone(), Some(parent_handle));
         });
 
@@ -174,12 +175,14 @@ impl Window {
         let (tx, rx) = mpsc::sync_channel::<WindowOpenResult>(1);
 
         let thread = thread::spawn(move || {
-            Self::window_thread(None, options, build, tx.clone(), None);
+            Self::window_thread(None, options, build, tx, None);
         });
 
         let _ = rx.recv().unwrap().unwrap();
 
-        thread.join();
+        thread.join().unwrap_or_else(|err| {
+            eprintln!("Window thread panicked: {:#?}", err);
+        });
     }
 
     fn window_thread<H, B>(
@@ -302,16 +305,16 @@ impl Window {
             title.as_bytes(),
         );
 
-        xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window).map(
-            |(wm_protocols, wm_delete_window)| {
-                xcb_util::icccm::set_wm_protocols(
-                    &xcb_connection.conn,
-                    window_id,
-                    wm_protocols,
-                    &[wm_delete_window],
-                );
-            },
-        );
+        if let Some((wm_protocols, wm_delete_window)) =
+            xcb_connection.atoms.wm_protocols.zip(xcb_connection.atoms.wm_delete_window)
+        {
+            xcb_util::icccm::set_wm_protocols(
+                &xcb_connection.conn,
+                window_id,
+                wm_protocols,
+                &[wm_delete_window],
+            );
+        }
 
         xcb_connection.conn.flush();
 
@@ -636,7 +639,7 @@ impl Window {
 
                 handler.on_event(
                     &mut crate::Window::new(self),
-                    Event::Keyboard(convert_key_press_event(&event)),
+                    Event::Keyboard(convert_key_press_event(event)),
                 );
             }
 
@@ -645,7 +648,7 @@ impl Window {
 
                 handler.on_event(
                     &mut crate::Window::new(self),
-                    Event::Keyboard(convert_key_release_event(&event)),
+                    Event::Keyboard(convert_key_release_event(event)),
                 );
             }
 
diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs
@@ -11,7 +11,6 @@ use super::cursor;
 pub(crate) struct Atoms {
     pub wm_protocols: Option<u32>,
     pub wm_delete_window: Option<u32>,
-    pub wm_normal_hints: Option<u32>,
 }
 
 pub struct XcbConnection {
@@ -20,6 +19,7 @@ pub struct XcbConnection {
 
     pub(crate) atoms: Atoms,
 
+    // FIXME: Same here, there's a ton of unused cursor machinery in here
     pub(super) cursor_cache: HashMap<MouseCursor, u32>,
 }
 
@@ -46,14 +46,13 @@ impl XcbConnection {
 
         conn.set_event_queue_owner(xcb::base::EventQueueOwner::Xcb);
 
-        let (wm_protocols, wm_delete_window, wm_normal_hints) =
-            intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW, WM_NORMAL_HINTS);
+        let (wm_protocols, wm_delete_window) = intern_atoms!(&conn, WM_PROTOCOLS, WM_DELETE_WINDOW);
 
         Ok(Self {
             conn,
             xlib_display,
 
-            atoms: Atoms { wm_protocols, wm_delete_window, wm_normal_hints },
+            atoms: Atoms { wm_protocols, wm_delete_window },
 
             cursor_cache: HashMap::new(),
         })
@@ -136,7 +135,7 @@ impl XcbConnection {
 
     #[inline]
     pub fn get_scaling(&self) -> Option<f64> {
-        self.get_scaling_xft().or(self.get_scaling_screen_dimensions())
+        self.get_scaling_xft().or_else(|| self.get_scaling_screen_dimensions())
     }
 
     #[inline]

Original file line number	Diff line number	Diff line change
`@@ -255,7 +255,7 @@ impl GlContext {`
`255`	`255`
`256`	`256`	`let hglrc =`
`257`	`257`	`wglCreateContextAttribsARB.unwrap()(hdc, std::ptr::null_mut(), ctx_attribs.as_ptr());`
`258`		`- if hglrc == std::ptr::null_mut() {`
	`258`	`+ if hglrc.is_null() {`
`259`	`259`	`return Err(GlError::CreationFailed(()));`
`260`	`260`	`}`
`261`	`261`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ pub struct WindowHandle {`
`44`	`44`
`45`	`45`	`impl WindowHandle {`
`46`	`46`	`pub fn close(&mut self) {`
`47`		`- if let Some(_) = self.raw_window_handle.take() {`
	`47`	`+ if self.raw_window_handle.take().is_some() {`
`48`	`48`	`self.close_requested.store(true, Ordering::Relaxed);`
`49`	`49`	`}`
`50`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -205,11 +205,8 @@ unsafe extern "system" fn wnd_proc(`
`205`	`205`	`}`
`206`	`206`	`}`
`207`	`207`	`WM_TIMER => {`
`208`		`- match wparam {`
`209`		`- WIN_FRAME_TIMER => {`
`210`		`- window_state.handler.on_frame(&mut window);`
`211`		`- }`
`212`		`- _ => (),`
	`208`	`+ if wparam == WIN_FRAME_TIMER {`
	`209`	`+ window_state.handler.on_frame(&mut window);`
`213`	`210`	`}`
`214`	`211`	`return 0;`
`215`	`212`	`}`
`@@ -414,8 +411,8 @@ impl Window {`
`414`	`411`	`break;`
`415`	`412`	`}`
`416`	`413`
`417`		`- TranslateMessage(&mut msg);`
`418`		`- DispatchMessageW(&mut msg);`
	`414`	`+ TranslateMessage(&msg);`
	`415`	`+ DispatchMessageW(&msg);`
`419`	`416`	`}`
`420`	`417`	`}`
`421`	`418`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,5 +101,5 @@ pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor)`
`101`	`101`	`MouseCursor::RowResize => loadn(&[b"split_v\0", b"v_double_arrow\0"]),`
`102`	`102`	`};`
`103`	`103`
`104`		`- cursor.or(load(b"left_ptr\0")).unwrap_or(0)`
	`104`	`+ cursor.or_else(\|\| load(b"left_ptr\0")).unwrap_or(0)`
`105`	`105`	`}`