fix some todos (#2)

* fix some todos * remove unused build script * fix lints * fixes * fix workflow * get_fonts use anyhow * test ?better? workflow * fix build script to set icon on windows * Expects * fix Cargo.toml --------- Co-authored-by: MevLyshkin <[email protected]>
Leinnan · Aug 25, 2024 · 0f9cd85 · 0f9cd85
1 parent 304a302
commit 0f9cd85
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 87 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -1,32 +1,24 @@
 name: CI Build
 
 on:
-  workflow_call:
   push:
     branches:
-      - "main"
-      - "master"
+      - main
   pull_request:
     branches:
-      - "main"
-      - "master"
-
-env:
-  CARGO_TERM_COLOR: always
+      - main
 
 jobs:
-  check-and-build:
+  ci:
+    name: ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ubuntu-latest, windows-2022]
-    runs-on: ${{ matrix.os }}
-    name: Build-${{ matrix.os }}
-    concurrency:
-      group: ${{ matrix.os }}-build-${{ github.head_ref }}
-      cancel-in-progress: true
-    timeout-minutes: 60
+        os: [ubuntu-latest, windows-latest, macos-latest]
+
     steps:
-      - uses: actions/checkout@v4
+      - name: Checkout code
+        uses: actions/checkout@v4
       - uses: actions/cache@v4
         continue-on-error: false
         with:
@@ -38,11 +30,15 @@ jobs:
             target/
           key: ${{ runner.os }}-build-${{ hashFiles('**/Cargo.lock') }}
           restore-keys: ${{ runner.os }}-build-
-      - uses: dtolnay/rust-toolchain@stable
-        with:
-          toolchain: stable
-          components: clippy,rustfmt
-      - name: check
-        run: |
-          cargo fmt --all -- --check
-          cargo clippy -- -D warnings
+
+      - name: Build
+        run: cargo build
+
+      - name: Test
+        run: cargo test
+
+      - name: Check formatting
+        run: cargo fmt --all -- --check
+
+      - name: Lint
+        run: cargo clippy -- -D warnings
diff --git a/Cargo.toml b/Cargo.toml
@@ -41,12 +41,9 @@ windows = { version = "0.58.0", features = [
     "Win32_UI_WindowsAndMessaging",
 ] }
 
-[target.'cfg(windows)'.build-dependencies]
-winres = "0.1"
-
 [build-dependencies]
-embed-resource = "2.4"
 anyhow = "1.0"
+winresource = "0.1.17"
 
 [lints.rust]
 unsafe_code = "deny"
@@ -64,4 +61,3 @@ perf = { level = "warn", priority = 8 }
 style = { level = "warn", priority = 9 }
 # this should be active but for that anyhow should be used everywhere
 unwrap_used = { level = "deny", priority = 10 }
-#expect_used = { level = "deny", priority = 11 }
diff --git a/build.rs b/build.rs
@@ -1,5 +1,3 @@
-extern crate embed_resource;
-
 fn main() -> anyhow::Result<()> {
     let output = std::process::Command::new("git")
         .args(["rev-parse", "HEAD"])
@@ -8,9 +6,12 @@ fn main() -> anyhow::Result<()> {
     let git_hash = String::from_utf8(output.stdout).expect("Failed to get output from git command");
     println!("cargo:rustc-env=GIT_HASH={}", &git_hash[..7]);
 
-    let target = std::env::var("TARGET")?;
-    if target.contains("windows") {
-        embed_resource::compile("static/icon.rc", embed_resource::NONE);
+    let target = std::env::var("CARGO_CFG_TARGET_OS")?;
+    if target == "windows" {
+        let mut res = winresource::WindowsResource::new();
+        res.set_icon("./static/icon.ico");
+        res.compile()?;
     }
+
     Ok(())
 }
diff --git a/src/app/central_panel.rs b/src/app/central_panel.rs
@@ -90,11 +90,10 @@ impl App {
                                 if is_dir {
                                     #[cfg(windows)]
                                     if ui.button("Open in explorer").clicked() {
-                                        // todo use result of that function
-                                        let _ = crate::windows_tools::open_in_explorer(
-                                            val.path(),
-                                            true,
-                                        );
+                                        crate::windows_tools::open_in_explorer(val.path(), true)
+                                            .unwrap_or_else(|_| {
+                                                toast!(Error, "Could not open in explorer");
+                                            });
                                         ui.close_menu();
                                         return;
                                     }
@@ -150,11 +149,10 @@ impl App {
                                 } else {
                                     #[cfg(windows)]
                                     if ui.button("Show in explorer").clicked() {
-                                        // todo use result of that function
-                                        let _ = crate::windows_tools::open_in_explorer(
-                                            val.path(),
-                                            false,
-                                        );
+                                        crate::windows_tools::open_in_explorer(val.path(), false)
+                                            .unwrap_or_else(|_| {
+                                                toast!(Error, "Could not open in explorer");
+                                            });
                                         ui.close_menu();
                                     }
                                 }

diff --git a/src/app/dir_handling.rs b/src/app/dir_handling.rs
@@ -79,9 +79,6 @@ impl App {
         dir_entries
     }
 
-    // todo refactor to not use unwraps
-    //#[allow(clippy::unwrap_used)]
-
     fn sort_entries(&self, dir_entries: &mut [DirEntry]) {
         dir_entries.sort_by(|a, b| {
             // Extract metadata for both entries and handle errors
@@ -102,15 +99,13 @@ impl App {
                     let name_b = b.file_name().to_ascii_lowercase();
                     name_a.cmp(&name_b)
                 }
-                Sort::Modified => {
-                    match (metadata_a, metadata_b) {
-                        (Ok(meta_a), Ok(meta_b)) => match (meta_a.modified(), meta_b.modified()) {
-                            (Ok(time_a), Ok(time_b)) => time_a.cmp(&time_b),
-                            (Err(_), _) | (_, Err(_)) => Ordering::Equal, // Handle cases where time can't be retrieved
-                        },
-                        _ => Ordering::Equal, // Handle cases where metadata can't be retrieved
-                    }
-                }
+                Sort::Modified => match (metadata_a, metadata_b) {
+                    (Ok(meta_a), Ok(meta_b)) => match (meta_a.modified(), meta_b.modified()) {
+                        (Ok(time_a), Ok(time_b)) => time_a.cmp(&time_b),
+                        (Err(_), _) | (_, Err(_)) => Ordering::Equal,
+                    },
+                    _ => Ordering::Equal,
+                },
                 Sort::Created => match (metadata_a, metadata_b) {
                     (Ok(meta_a), Ok(meta_b)) => match (meta_a.created(), meta_b.created()) {
                         (Ok(time_a), Ok(time_b)) => time_a.cmp(&time_b),

diff --git a/src/app/mod.rs b/src/app/mod.rs
@@ -1,5 +1,6 @@
 use crate::locations::Locations;
 use egui::ahash::{HashMap, HashMapExt};
+use serde::{Deserialize, Serialize};
 use std::{fs, path::PathBuf};
 
 mod central_panel;
@@ -33,7 +34,7 @@ macro_rules! toast{
         };
     }
 
-#[derive(serde::Deserialize, serde::Serialize)]
+#[derive(Deserialize, Serialize)]
 #[serde(default)] // if we add new fields, give them default values when deserializing old state
 pub struct App {
     show_hidden: bool,
@@ -50,7 +51,7 @@ pub struct App {
     dir_has_cargo: bool,
 }
 
-#[derive(serde::Deserialize, serde::Serialize, Default, PartialEq, Eq, Debug, Clone, Copy)]
+#[derive(Deserialize, Serialize, Default, PartialEq, Eq, Debug, Clone, Copy)]
 pub enum Sort {
     #[default]
     Name,
@@ -60,7 +61,7 @@ pub enum Sort {
     Random,
 }
 
-#[derive(serde::Deserialize, serde::Serialize, Default)]
+#[derive(Deserialize, Serialize, Default)]
 pub struct Search {
     pub visible: bool,
     pub favorites: bool,
@@ -144,10 +145,7 @@ impl eframe::App for App {
     }
 
     /// Called each time the UI needs repainting, which may be many times per second.
-    #[allow(clippy::too_many_lines)] // todo: refactor
     fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
-        // Put your widgets into a `SidePanel`, `TopBottomPanel`, `CentralPanel`, `Window` or `Area`.
-        // For inspiration and more examples, go to https://emilk.github.io/egui
         let mut new_path = None;
         let mut search_changed = false;
         self.top_panel(ctx, &mut new_path);
@@ -171,7 +169,7 @@ impl eframe::App for App {
 fn setup_custom_fonts(ctx: &egui::Context) {
     // Start with the default fonts (we will be adding to them rather than replacing them).
     let mut fonts = egui::FontDefinitions::default();
-    if let Some((regular, semibold)) = get_fonts() {
+    if let Ok((regular, semibold)) = get_fonts() {
         fonts
             .font_data
             .insert("regular".to_owned(), egui::FontData::from_owned(regular));
@@ -201,6 +199,7 @@ fn setup_custom_fonts(ctx: &egui::Context) {
         // Tell egui to use these fonts:
         ctx.set_fonts(fonts);
     }
+
     ctx.style_mut(|style| {
         for font_id in style.text_styles.values_mut() {
             font_id.size *= 1.4;
@@ -209,36 +208,24 @@ fn setup_custom_fonts(ctx: &egui::Context) {
 }
 
 #[cfg(not(windows))]
-fn get_fonts() -> Option<(Vec<u8>, Vec<u8>)> {
+fn get_fonts() -> anyhow::Result<(Vec<u8>, Vec<u8>)> {
     let font_path = std::path::Path::new("/System/Library/Fonts");
 
-    let Ok(regular) = fs::read(font_path.join("SFNSRounded.ttf")) else {
-        return None;
-    };
-    let Ok(semibold) = fs::read(font_path.join("SFCompact.ttf")) else {
-        return None;
-    };
+    let regular = fs::read(font_path.join("SFNSRounded.ttf"))?;
+    let semibold = fs::read(font_path.join("SFCompact.ttf"))?;
 
-    Some((regular, semibold))
+    Ok((regular, semibold))
 }
 
 #[cfg(windows)]
-fn get_fonts() -> Option<(Vec<u8>, Vec<u8>)> {
-    let Ok(app_data) = std::env::var("APPDATA") else {
-        return None;
-    };
+fn get_fonts() -> anyhow::Result<(Vec<u8>, Vec<u8>)> {
+    let app_data = std::env::var("APPDATA")?;
     let font_path = std::path::Path::new(&app_data);
 
-    let Ok(regular) = fs::read(font_path.join("../Local/Microsoft/Windows/Fonts/aptos.ttf")) else {
-        return None;
-    };
-    let Ok(semibold) =
-        fs::read(font_path.join("../Local/Microsoft/Windows/Fonts/aptos-semibold.ttf"))
-    else {
-        return None;
-    };
+    let regular = fs::read(font_path.join("../Local/Microsoft/Windows/Fonts/aptos.ttf"))?;
+    let semibold = fs::read(font_path.join("../Local/Microsoft/Windows/Fonts/aptos-semibold.ttf"))?;
 
-    Some((regular, semibold))
+    Ok((regular, semibold))
 }
 
 fn get_starting_path() -> PathBuf {

diff --git a/src/app/top_bottom.rs b/src/app/top_bottom.rs
@@ -30,7 +30,7 @@ impl App {
                     ui.add_space(TOP_SIDE_MARGIN);
                     let mut path: String = String::new();
 
-                    #[allow(unused_variables)] // todo fix unwraps here
+                    #[allow(unused_variables)] // not used on linux
                     for (i, e) in self.cur_path.iter().enumerate() {
                         #[cfg(windows)]
                         {
@@ -81,7 +81,6 @@ impl App {
                     };
                     let amount = size_left.x - amount;
                     ui.add_space(amount);
-                    #[allow(clippy::collapsible_if)]
                     if self.dir_has_cargo && ui.button(">").on_hover_text("Run project").clicked() {
                         // todo: add possibility to stop it again
                         match Command::new("cargo")