From 0f9cd85fcc2a66730a524946fdf4154c2ae71c54 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 25 Aug 2024 21:54:56 +0200 Subject: [PATCH] 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 --- .github/workflows/rust.yml | 46 +++++++++++++++++--------------------- Cargo.toml | 6 +---- build.rs | 11 ++++----- src/app/central_panel.rs | 18 +++++++-------- src/app/dir_handling.rs | 19 ++++++---------- src/app/mod.rs | 43 +++++++++++++---------------------- src/app/top_bottom.rs | 3 +-- 7 files changed, 59 insertions(+), 87 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 96b2747..c3b78c3 100644 --- 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 index 78b4699..258dac1 100644 --- 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 index adf7248..2b96603 100644 --- 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 index 22dbde3..39defda 100644 --- 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 index 928fad2..8ba87fc 100644 --- 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 index 228f51a..6512342 100644 --- 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, Vec)> { +fn get_fonts() -> anyhow::Result<(Vec, Vec)> { 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, Vec)> { - let Ok(app_data) = std::env::var("APPDATA") else { - return None; - }; +fn get_fonts() -> anyhow::Result<(Vec, Vec)> { + 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 index 4d2fec0..d736da3 100644 --- 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")