Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Less clones & consistent search result indexes #441

Merged
merged 17 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9a507e9
refactor(lib::podcast::PodcastFeed::new): reduce amount of clones nec…
hasezoey Mar 4, 2025
5c18631
refactor(tui::ui::model::update::update_podcast): take value instead …
hasezoey Mar 4, 2025
6606e88
refactor(lib::types::PCMsg::Error): remove duplicate url-string
hasezoey Mar 5, 2025
f1a348c
refactor(lib::podcast::import_from_opml): reduce clones and error cases
hasezoey Mar 5, 2025
dfb6860
refactor(lib::library_db::get_record_by_path): only get the first entry
hasezoey Mar 5, 2025
21d2e5c
refactor(tui::ui::components::lyric::lyric_reload): reduce a clone
hasezoey Mar 5, 2025
8c42fcd
refactor(tui::ui::components::music_library::library_reload_tree): re…
hasezoey Mar 5, 2025
408184c
refactor(tui::ui::components::podcast): remove some unnecessary clones
hasezoey Mar 5, 2025
0a3951e
refactor(tui::ui::components::config_editor::key_combo::render_value_…
hasezoey Mar 5, 2025
51c3cb9
refactor(tui::ui::components::config_editor::key_combo): reduce cloni…
hasezoey Mar 5, 2025
c76391e
refactor(tui::ui::components::popups::message::umount_message): reduc…
hasezoey Mar 5, 2025
73ef4ce
refactor(tui::ui::model::youtube_options::youtube_dl): reduce arc clo…
hasezoey Mar 5, 2025
eab1ce5
refactor(tui::ui::components::config_editor::key_combo::perform): lig…
hasezoey Mar 5, 2025
59bcc95
style(tui::ui::*): remove some unnecessary "to_string"s
hasezoey Mar 5, 2025
160d5f4
style(lib::playlist::file_url_to_path): only do "to_string" in err case
hasezoey Mar 5, 2025
07fcd37
fix(tui::ui::components::*): consistent numbering of search results
hasezoey Mar 5, 2025
bda754a
Merge branch 'master' into lessClone
tramhao Mar 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Fix(tui): not having the current theme selected when entering Theme preview tab.
- Fix(tui): actually report any errors when adding to the playlist. (Like "invalid file type")
- Fix(tui): sort Music-Library content Alphanumerically.
- Fix(tui): consistent numbering of results in a search popup.
- Fix(tui): let `ueberzug` command inherit `stdout` to display chafa.

### [V0.9.1]
Expand Down
8 changes: 4 additions & 4 deletions lib/src/library_db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,13 +325,13 @@ impl DataBase {
let conn = self.conn.lock();
let mut stmt = conn.prepare(search_str)?;

let vec_records: Vec<TrackDB> = stmt
let maybe_record: Option<TrackDB> = stmt
.query_map([file_path], TrackDB::try_from_row_named)?
.flatten()
.collect();
.next();

if let Some(record) = vec_records.first() {
return Ok(record.clone());
if let Some(record) = maybe_record {
return Ok(record);
}

Err(Error::QueryReturnedNoRows)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/playlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl PlaylistValue {
let as_path = url
.to_file_path()
.map_err(|()| anyhow!("Failed to convert URL to Path!"))
.context(url.to_string())?;
.with_context(|| url.to_string())?;
*self = Self::Path(as_path);
}

Expand Down
26 changes: 9 additions & 17 deletions lib/src/podcast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,8 @@ pub struct PodcastFeed {

impl PodcastFeed {
#[must_use]
pub fn new(id: Option<i64>, url: &str, title: Option<String>) -> Self {
Self {
id,
url: url.to_string(),
title,
}
pub const fn new(id: Option<i64>, url: String, title: Option<String>) -> Self {
Self { id, url, title }
}
}

Expand All @@ -97,7 +93,7 @@ pub fn check_feed(feed: PodcastFeed, max_retries: usize, tp: &TaskPool, tx_to_ma
},
Err(err) => {
error!("get_feed_data had a Error: {:#?}", err);
let _ = tx_to_main.send(Msg::Podcast(PCMsg::Error(feed.url.to_string(), feed)));
let _ = tx_to_main.send(Msg::Podcast(PCMsg::Error(feed)));
}
}
});
Expand Down Expand Up @@ -315,27 +311,23 @@ pub fn import_from_opml(db_path: &Path, config: &PodcastSettings, file: &Path) -
match message {
Msg::Podcast(PCMsg::NewData(pod)) => {
msg_counter += 1;
let title = pod.title.clone();
let title = &pod.title;
let db_result = db_inst.insert_podcast(&pod);
match db_result {
Ok(_) => {
println!("Added {title}");
}
Err(_err) => {
Err(err) => {
failure = true;
error!("Error adding {title}");
error!("Error adding {title}, err: {err}");
}
}
}

Msg::Podcast(PCMsg::Error(_, feed)) => {
Msg::Podcast(PCMsg::Error(feed)) => {
msg_counter += 1;
failure = true;
if let Some(t) = feed.title {
error!("Error retrieving RSS feed: {t}");
} else {
error!("Error retrieving RSS feed");
}
error!("Error retrieving RSS feed: {}", feed.url);
}

Msg::Podcast(PCMsg::SyncData((_id, _pod))) => {
Expand Down Expand Up @@ -394,7 +386,7 @@ fn import_opml_feeds(xml: &str) -> Result<Vec<PodcastFeed>> {
Some(pod.text)
}
});
feeds.push(PodcastFeed::new(None, &pod.xml_url.unwrap(), title));
feeds.push(PodcastFeed::new(None, pod.xml_url.unwrap(), title));
}
}
Ok(feeds)
Expand Down
2 changes: 1 addition & 1 deletion lib/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ pub enum PCMsg {
PodcastAddPopupCloseCancel,
SyncData((i64, PodcastNoId)),
NewData(PodcastNoId),
Error(String, PodcastFeed),
Error(PodcastFeed),
PodcastSelected(usize),
DescriptionUpdate,
EpisodeAdd(usize),
Expand Down
32 changes: 16 additions & 16 deletions tui/src/ui/components/config_editor/key_combo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,9 @@ impl InputStates {

/// ### `render_value_chars`
///
/// Render value as a vec of chars
pub fn render_value_chars(&self) -> Vec<char> {
self.input.clone()
/// Get the current input as a slice
pub fn render_value_chars(&self) -> &[char] {
&self.input
}

/// ### `get_value`
Expand Down Expand Up @@ -469,7 +469,7 @@ impl KeyCombo {
.states
.choices
.iter()
.map(|x| ListItem::new(Span::from(x.clone())))
.map(|x| ListItem::new(Span::from(x)))
.collect();
let mut foreground = self
.props
Expand All @@ -490,9 +490,9 @@ impl KeyCombo {
.constraints([Constraint::Length(2), Constraint::Min(1)].as_ref())
.split(area);
// Render like "closed" tab in chunk 0
let selected_text: String = match self.states.choices.get(self.states.selected) {
None => String::default(),
Some(s) => s.clone(),
let selected_text = match self.states.choices.get(self.states.selected) {
None => "",
Some(s) => s.as_str(),
};
let borders = self
.props
Expand Down Expand Up @@ -656,9 +656,9 @@ impl KeyCombo {
style = style_invalid;
}
}
let selected_text: String = match self.states.choices.get(self.states.selected) {
None => String::default(),
Some(s) => s.clone(),
let selected_text = match self.states.choices.get(self.states.selected) {
None => "",
Some(s) => s.as_str(),
};
let p: Paragraph<'_> = Paragraph::new(selected_text).style(style).block(block);

Expand Down Expand Up @@ -845,9 +845,9 @@ impl MockComponent for KeyCombo {
}
Cmd::Cancel => {
self.states.cancel_tab();
let prev_input = self.states_input.input.clone();
let prev_len = self.states_input.input.len();
self.states_input.delete();
if prev_input == self.states_input.input {
if prev_len == self.states_input.input.len() {
CmdResult::None
} else {
CmdResult::Changed(self.state())
Expand All @@ -866,9 +866,9 @@ impl MockComponent for KeyCombo {

Cmd::Delete => {
// Backspace and None
let prev_input = self.states_input.input.clone();
let prev_len = self.states_input.input.len();
self.states_input.backspace();
if prev_input == self.states_input.input {
if prev_len == self.states_input.input.len() {
CmdResult::None
} else {
CmdResult::Changed(self.state())
Expand All @@ -892,10 +892,10 @@ impl MockComponent for KeyCombo {
}
Cmd::Type(ch) => {
// Push char to input
let prev_input = self.states_input.input.clone();
let prev_len = self.states_input.input.len();
self.states_input.append(ch, self.get_input_len());
// Message on change
if prev_input == self.states_input.input {
if prev_len == self.states_input.input.len() {
CmdResult::None
} else {
CmdResult::Changed(self.state())
Expand Down
9 changes: 3 additions & 6 deletions tui/src/ui/components/config_editor/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,13 @@ impl Model {

match id {
IdConfigEditor::LibraryHighlightSymbol => {
self.config_editor.theme.style.library.highlight_symbol =
symbol.to_string();
self.config_editor.theme.style.library.highlight_symbol = symbol;
}
IdConfigEditor::PlaylistHighlightSymbol => {
self.config_editor.theme.style.playlist.highlight_symbol =
symbol.to_string();
self.config_editor.theme.style.playlist.highlight_symbol = symbol;
}
IdConfigEditor::CurrentlyPlayingTrackSymbol => {
self.config_editor.theme.style.playlist.current_track_symbol =
symbol.to_string();
self.config_editor.theme.style.playlist.current_track_symbol = symbol;
}
_ => {}
};
Expand Down
3 changes: 1 addition & 2 deletions tui/src/ui/components/lyric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ impl Model {
)
.is_ok());
self.lyric_update_title();
let lyric_line = self.lyric_line.clone();
self.lyric_set_lyric(&lyric_line);
self.lyric_set_lyric(self.lyric_line.clone());
}

pub fn lyric_update_for_podcast_by_current_track(&mut self) {
Expand Down
6 changes: 3 additions & 3 deletions tui/src/ui/components/music_library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ impl Model {
.mount(
Id::Library,
Box::new(MusicLibrary::new(
&self.library.tree.clone(),
&self.library.tree,
current_node,
self.config_tui.clone(),
),),
Expand All @@ -358,7 +358,7 @@ impl Model {
.remount(
Id::Library,
Box::new(MusicLibrary::new(
&self.library.tree.clone(),
&self.library.tree,
current_node,
self.config_tui.clone(),
),),
Expand Down Expand Up @@ -473,7 +473,7 @@ impl Model {
let root = self.library.tree.root();
let p: &Path = Path::new(root.id());
let all_items = walkdir::WalkDir::new(p).follow_links(true);
let mut idx = 0;
let mut idx: usize = 0;
let search = format!("*{}*", input.to_lowercase());
let search = wildmatch::WildMatch::new(&search);
for record in all_items.into_iter().filter_map(std::result::Result::ok) {
Expand Down
23 changes: 11 additions & 12 deletions tui/src/ui/components/podcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl Model {
});
}

pub fn podcast_add(&mut self, url: &str) {
pub fn podcast_add(&mut self, url: String) {
let feed = PodcastFeed::new(None, url, None);

crate::podcast::check_feed(
Expand Down Expand Up @@ -627,7 +627,7 @@ impl Model {
.ok_or_else(|| anyhow!("get podcast selected failed."))?;
let pcf = PodcastFeed::new(
Some(pod_selected.id),
&pod_selected.url.clone(),
pod_selected.url.clone(),
Some(pod_selected.title.clone()),
);
pod_data.push(pcf);
Expand All @@ -640,7 +640,7 @@ impl Model {
.podcasts
.iter()
.map(|pod| {
PodcastFeed::new(Some(pod.id), &pod.url.clone(), Some(pod.title.clone()))
PodcastFeed::new(Some(pod.id), pod.url.clone(), Some(pod.title.clone()))
})
.collect();
}
Expand Down Expand Up @@ -843,7 +843,7 @@ impl Model {

for ep in &mut podcast_selected.episodes {
if ep.path.is_some() {
match std::fs::remove_file(ep.path.clone().unwrap()) {
match std::fs::remove_file(ep.path.as_ref().unwrap()) {
Ok(()) => {
eps_to_remove.push(ep.id);
ep.path = None;
Expand Down Expand Up @@ -977,12 +977,11 @@ impl Model {

pub fn podcast_update_search_episode(&mut self, input: &str) {
let mut table: TableBuilder = TableBuilder::default();
let mut idx = 0;
let mut idx: usize = 0;
let search = format!("*{}*", input.to_lowercase());
let mut db_tracks = vec![];
// Get all episodes
let podcasts = self.podcast.podcasts.clone();
for podcast in podcasts {
for podcast in &self.podcast.podcasts {
if let Ok(episodes) = self.podcast.db_podcast.get_episodes(podcast.id, true) {
db_tracks.extend(episodes);
}
Expand All @@ -998,11 +997,11 @@ impl Model {
if idx > 0 {
table.add_row();
}
idx += 1;
table
.add_col(TextSpan::new(idx.to_string()))
.add_col(TextSpan::new(record.title).bold())
.add_col(TextSpan::new(format!("{}", record.id)));
idx += 1;
}
}
}
Expand All @@ -1013,10 +1012,10 @@ impl Model {

pub fn podcast_update_search_podcast(&mut self, input: &str) {
let mut table: TableBuilder = TableBuilder::default();
let mut idx = 0;
let mut idx: usize = 0;
let search = format!("*{}*", input.to_lowercase());
// Get all episodes
let db_tracks = self.podcast.podcasts.clone();
let db_tracks = &self.podcast.podcasts;

if db_tracks.is_empty() {
table.add_col(TextSpan::from("0"));
Expand All @@ -1028,11 +1027,11 @@ impl Model {
if idx > 0 {
table.add_row();
}
idx += 1;
table
.add_col(TextSpan::new(idx.to_string()))
.add_col(TextSpan::new(record.title).bold())
.add_col(TextSpan::new(&record.title).bold())
.add_col(TextSpan::new(format!("{}", record.id)));
idx += 1;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tui/src/ui/components/popups/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ impl Model {
if let Ok(Some(AttrValue::Payload(PropPayload::Vec(spans)))) =
self.app.query(&Id::MessagePopup, Attribute::Text)
{
if let Some(display_text) = spans.first() {
let d = display_text.clone().unwrap_text_span().content;
if let Some(display_text) = spans.into_iter().next() {
let d = display_text.unwrap_text_span().content;
if text.eq(&d) {
self.app.umount(&Id::MessagePopup).ok();
}
Expand Down
5 changes: 3 additions & 2 deletions tui/src/ui/components/popups/podcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,15 @@ impl Model {

pub fn update_podcast_search_table(&mut self) {
let mut table: TableBuilder = TableBuilder::default();
let mut idx = 0;
let mut idx: usize = 0;
if let Some(vec) = &self.podcast.search_results {
for record in vec {
if idx > 0 {
table.add_row();
}

idx += 1;

let title = record
.title
.clone()
Expand All @@ -293,7 +295,6 @@ impl Model {
.add_col(TextSpan::new(title).bold())
.add_col(TextSpan::new(record.url.clone()));
// .add_col(TextSpan::new(record.album().unwrap_or("Unknown Album")));
idx += 1;
}
// if self.player.playlist.is_empty() {
// table.add_col(TextSpan::from("0"));
Expand Down
4 changes: 2 additions & 2 deletions tui/src/ui/model/download_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ impl Default for DownloadTracker {

#[allow(dead_code)]
impl DownloadTracker {
pub fn increase_one(&mut self, url: &str) {
self.items.insert(url.to_string());
pub fn increase_one<U: Into<String>>(&mut self, url: U) {
self.items.insert(url.into());
}

pub fn decrease_one(&mut self, url: &str) {
Expand Down
Loading