fix: address reviewer feedback — rename scroll field to bool, relocate static, drop env var
- Rename `ui_scroll_tick_count: u64` → `should_text_scroll: bool` across all 33 call sites; `on_ui_scroll_tick` now assigns true/false instead of 0/1 - Remove two now-redundant comments from `test_on_ui_scroll_tick` - Move `WHITESPACE_RE` static declaration below module imports in network/mod.rs - Remove `MANAGARR_TICK_RATE_MS` env var and its parsing helpers; `recv()`-based blocking already brings idle CPU to near-0, and tuning tick rate silently stretches the poll cadence — keep `DEFAULT_TICK_RATE_MS` const only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3FpbniZme9mJqgFkMVowz
This commit is contained in:
@@ -83,7 +83,7 @@ mod tests {
|
||||
assert_eq!(app.tick_until_poll, 400);
|
||||
assert_eq!(app.scroll_interval, Duration::from_millis(100));
|
||||
assert_eq!(app.tick_count, 0);
|
||||
assert_eq!(app.ui_scroll_tick_count, 0);
|
||||
assert!(!app.should_text_scroll);
|
||||
assert!(!app.is_loading);
|
||||
assert!(!app.is_routing);
|
||||
assert!(!app.should_refresh);
|
||||
@@ -252,19 +252,16 @@ mod tests {
|
||||
..App::default()
|
||||
};
|
||||
|
||||
// Zero interval: elapsed() >= scroll_interval is always true, so every
|
||||
// call should signal "scroll now" (ui_scroll_tick_count == 0)
|
||||
app.on_ui_scroll_tick();
|
||||
assert_eq!(app.ui_scroll_tick_count, 0);
|
||||
assert!(app.should_text_scroll);
|
||||
|
||||
app.on_ui_scroll_tick();
|
||||
assert_eq!(app.ui_scroll_tick_count, 0);
|
||||
assert!(app.should_text_scroll);
|
||||
|
||||
// Long interval with a fresh timestamp: should signal "not yet" (count == 1)
|
||||
app.scroll_interval = Duration::from_secs(60);
|
||||
app.last_scroll = Instant::now();
|
||||
app.on_ui_scroll_tick();
|
||||
assert_eq!(app.ui_scroll_tick_count, 1);
|
||||
assert!(!app.should_text_scroll);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
+4
-4
@@ -45,7 +45,7 @@ pub struct App<'a> {
|
||||
pub scroll_interval: Duration,
|
||||
pub last_scroll: Instant,
|
||||
pub tick_count: u64,
|
||||
pub ui_scroll_tick_count: u64,
|
||||
pub should_text_scroll: bool,
|
||||
pub is_routing: bool,
|
||||
pub is_loading: bool,
|
||||
pub should_refresh: bool,
|
||||
@@ -174,10 +174,10 @@ impl App<'_> {
|
||||
|
||||
pub fn on_ui_scroll_tick(&mut self) {
|
||||
if self.last_scroll.elapsed() >= self.scroll_interval {
|
||||
self.ui_scroll_tick_count = 0;
|
||||
self.should_text_scroll = true;
|
||||
self.last_scroll = Instant::now();
|
||||
} else {
|
||||
self.ui_scroll_tick_count = 1;
|
||||
self.should_text_scroll = false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -266,7 +266,7 @@ impl Default for App<'_> {
|
||||
scroll_interval: Duration::from_millis(100),
|
||||
last_scroll: Instant::now(),
|
||||
tick_count: 0,
|
||||
ui_scroll_tick_count: 0,
|
||||
should_text_scroll: false,
|
||||
is_loading: false,
|
||||
is_routing: false,
|
||||
should_refresh: false,
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use anyhow::Result;
|
||||
use std::env;
|
||||
use std::sync::mpsc;
|
||||
use std::sync::mpsc::Receiver;
|
||||
use std::thread;
|
||||
@@ -21,22 +20,10 @@ pub struct Events {
|
||||
|
||||
const DEFAULT_TICK_RATE_MS: u64 = 50;
|
||||
|
||||
fn configured_tick_rate_ms_from(raw: Option<&str>) -> u64 {
|
||||
raw
|
||||
.and_then(|value| value.parse::<u64>().ok())
|
||||
.filter(|ms| *ms > 0)
|
||||
.unwrap_or(DEFAULT_TICK_RATE_MS)
|
||||
}
|
||||
|
||||
fn configured_tick_rate_ms() -> u64 {
|
||||
let raw = env::var("MANAGARR_TICK_RATE_MS").ok();
|
||||
configured_tick_rate_ms_from(raw.as_deref())
|
||||
}
|
||||
|
||||
impl Events {
|
||||
pub fn new() -> Self {
|
||||
let (tx, rx) = mpsc::channel();
|
||||
let tick_rate: Duration = Duration::from_millis(configured_tick_rate_ms());
|
||||
let tick_rate: Duration = Duration::from_millis(DEFAULT_TICK_RATE_MS);
|
||||
|
||||
thread::spawn(move || {
|
||||
let mut last_tick = Instant::now();
|
||||
@@ -80,16 +67,5 @@ mod tests {
|
||||
#[test]
|
||||
fn defaults_to_original_tick_rate() {
|
||||
assert_eq!(DEFAULT_TICK_RATE_MS, 50);
|
||||
assert_eq!(configured_tick_rate_ms_from(None), 50);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_positive_tick_rates_and_rejects_invalid_values() {
|
||||
assert_eq!(configured_tick_rate_ms_from(Some("250")), 250);
|
||||
assert_eq!(configured_tick_rate_ms_from(Some("0")), DEFAULT_TICK_RATE_MS);
|
||||
assert_eq!(
|
||||
configured_tick_rate_ms_from(Some("abc")),
|
||||
DEFAULT_TICK_RATE_MS
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -6,8 +6,6 @@ use async_trait::async_trait;
|
||||
use lidarr_network::LidarrEvent;
|
||||
use log::{debug, error, warn};
|
||||
use regex::Regex;
|
||||
|
||||
static WHITESPACE_RE: OnceLock<Regex> = OnceLock::new();
|
||||
use reqwest::{Client, RequestBuilder};
|
||||
use serde::Serialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
@@ -35,6 +33,8 @@ mod network_tests;
|
||||
#[cfg(test)]
|
||||
pub mod servarr_test_utils;
|
||||
|
||||
static WHITESPACE_RE: OnceLock<Regex> = OnceLock::new();
|
||||
|
||||
#[cfg_attr(test, automock)]
|
||||
#[async_trait]
|
||||
pub trait NetworkTrait {
|
||||
|
||||
@@ -88,7 +88,7 @@ fn draw_downloads(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
output_path.as_ref().unwrap().scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 18),
|
||||
current_selection == *download_record,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ fn draw_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 50),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -46,7 +46,7 @@ fn draw_test_all_indexers_test_results(f: &mut Frame<'_>, app: &mut App<'_>, are
|
||||
result.validation_failures.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 86),
|
||||
*result == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let pass_fail = if result.is_valid { "+" } else { "x" };
|
||||
let row = Row::new(vec![
|
||||
|
||||
@@ -112,7 +112,7 @@ fn draw_add_artist_search(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
artist.artist_name.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 27),
|
||||
*artist == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -250,7 +250,7 @@ fn draw_album_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
@@ -345,7 +345,7 @@ fn draw_album_releases(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
get_width_from_percentage(area, 35),
|
||||
current_selection == *release
|
||||
&& active_lidarr_block != ActiveLidarrBlock::ManualAlbumSearchConfirmPrompt,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let size = convert_to_gb(*size);
|
||||
let rejected_str = if *rejected { "⛔" } else { "" };
|
||||
|
||||
@@ -271,7 +271,7 @@ fn draw_albums_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
album.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 33),
|
||||
*album == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let monitored = if album.monitored { "🏷" } else { "" };
|
||||
let album_type = album.album_type.clone().unwrap_or_default();
|
||||
@@ -376,7 +376,7 @@ fn draw_artist_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
@@ -490,7 +490,7 @@ fn draw_artist_releases(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
get_width_from_percentage(area, 35),
|
||||
current_selection == *release
|
||||
&& active_lidarr_block != ActiveLidarrBlock::ManualArtistSearchConfirmPrompt,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let size = convert_to_gb(*size);
|
||||
let rejected_str = if *rejected { "⛔" } else { "" };
|
||||
|
||||
@@ -96,7 +96,7 @@ fn draw_library(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
artist.artist_name.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 25),
|
||||
*artist == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let monitored = if artist.monitored { "🏷" } else { "" };
|
||||
let artist_type = artist.artist_type.clone().unwrap_or_default();
|
||||
|
||||
@@ -158,7 +158,7 @@ fn draw_track_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
+1
-1
@@ -139,7 +139,7 @@ fn draw_error(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
|
||||
app
|
||||
.error
|
||||
.scroll_left_or_reset(area.width as usize, true, app.ui_scroll_tick_count == 0);
|
||||
.scroll_left_or_reset(area.width as usize, true, app.should_text_scroll);
|
||||
|
||||
let paragraph = Paragraph::new(Text::from(app.error.to_string().failure()))
|
||||
.block(block)
|
||||
|
||||
@@ -96,7 +96,7 @@ fn draw_blocklist_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
movie.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 20),
|
||||
current_selection == *blocklist_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
let languages_string = languages
|
||||
|
||||
@@ -90,7 +90,7 @@ pub fn draw_collection_details(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect)
|
||||
movie.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(table_area, 20),
|
||||
current_selection == *movie,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let (hours, minutes) = convert_runtime(movie.runtime);
|
||||
let imdb_rating = movie
|
||||
|
||||
@@ -70,7 +70,7 @@ pub(super) fn draw_collections(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect)
|
||||
collection.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 25),
|
||||
*collection == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let monitored = if collection.monitored { "🏷" } else { "" };
|
||||
let search_on_add = if collection.search_on_add {
|
||||
|
||||
@@ -87,7 +87,7 @@ fn draw_downloads(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
output_path.as_ref().unwrap().scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 18),
|
||||
current_selection == *download_record,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -61,7 +61,7 @@ fn draw_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -47,7 +47,7 @@ fn draw_test_all_indexers_test_results(f: &mut Frame<'_>, app: &mut App<'_>, are
|
||||
result.validation_failures.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 86),
|
||||
*result == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let pass_fail = if result.is_valid { "✔" } else { "❌" };
|
||||
let row = Row::new(vec![
|
||||
|
||||
@@ -141,7 +141,7 @@ fn draw_add_movie_search(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
movie.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 27),
|
||||
*movie == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -90,7 +90,7 @@ fn draw_library(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
movie.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 27),
|
||||
*movie == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let monitored = if movie.monitored { "🏷" } else { "" };
|
||||
let studio = movie.studio.clone().unwrap_or_default();
|
||||
|
||||
@@ -250,7 +250,7 @@ fn draw_movie_history(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
movie_history_item.source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 34),
|
||||
current_selection == *movie_history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
@@ -402,7 +402,7 @@ fn draw_movie_releases(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
get_width_from_percentage(area, 30),
|
||||
current_selection == *release
|
||||
&& current_route != ActiveRadarrBlock::ManualSearchConfirmPrompt.into(),
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let size = convert_to_gb(*size);
|
||||
let rejected_str = if *rejected { "⛔" } else { "" };
|
||||
|
||||
@@ -88,7 +88,7 @@ fn draw_downloads(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
output_path.as_ref().unwrap().scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 18),
|
||||
current_selection == *download_record,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -61,7 +61,7 @@ fn draw_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -46,7 +46,7 @@ fn draw_test_all_indexers_test_results(f: &mut Frame<'_>, app: &mut App<'_>, are
|
||||
result.validation_failures.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 86),
|
||||
*result == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let pass_fail = if result.is_valid { "✔" } else { "❌" };
|
||||
let row = Row::new(vec![
|
||||
|
||||
@@ -121,7 +121,7 @@ fn draw_add_series_search(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
series.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 27),
|
||||
*series == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
@@ -279,7 +279,7 @@ fn draw_episode_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect)
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
@@ -414,7 +414,7 @@ fn draw_episode_releases(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
get_width_from_percentage(area, 30),
|
||||
current_selection == *release
|
||||
&& active_sonarr_block != ActiveSonarrBlock::ManualEpisodeSearchConfirmPrompt,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let size = convert_to_gb(*size);
|
||||
let rejected_str = if *rejected { "⛔" } else { "" };
|
||||
|
||||
@@ -95,7 +95,7 @@ fn draw_library(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
series.title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 23),
|
||||
*series == current_selection,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let monitored = if series.monitored { "🏷" } else { "" };
|
||||
let certification = series.certification.clone().unwrap_or_default();
|
||||
|
||||
@@ -266,7 +266,7 @@ fn draw_season_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
@@ -377,7 +377,7 @@ fn draw_season_releases(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
get_width_from_percentage(area, 30),
|
||||
current_selection == *release
|
||||
&& active_sonarr_block != ActiveSonarrBlock::ManualSeasonSearchConfirmPrompt,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
let size = convert_to_gb(*size);
|
||||
let rejected_str = if *rejected { "⛔" } else { "" };
|
||||
|
||||
@@ -314,7 +314,7 @@ fn draw_series_history_table(f: &mut Frame<'_>, app: &mut App<'_>, area: Rect) {
|
||||
source_title.scroll_left_or_reset(
|
||||
get_width_from_percentage(area, 40),
|
||||
current_selection == *history_item,
|
||||
app.ui_scroll_tick_count == 0,
|
||||
app.should_text_scroll,
|
||||
);
|
||||
|
||||
Row::new(vec![
|
||||
|
||||
Reference in New Issue
Block a user