Skip to content

Commit 588af97

Browse files
authored
Merge pull request #126 from bugzmanov/pre039
Bugs: -c overrites history, djvu filtering, pdf/djvu support in non g…
2 parents 2548dd4 + a70009a commit 588af97

File tree

4 files changed

+157
-11
lines changed

4 files changed

+157
-11
lines changed

src/book_manager.rs

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ pub struct BookManager {
1818
pub books: Vec<BookInfo>,
1919
scan_directory: String,
2020
pub library_mode: LibraryMode,
21+
#[cfg(feature = "pdf")]
22+
pub supports_graphics: bool,
2123
}
2224

2325
/// Format of a book file
@@ -71,6 +73,8 @@ impl BookManager {
7173
books,
7274
scan_directory,
7375
library_mode,
76+
#[cfg(feature = "pdf")]
77+
supports_graphics: false,
7478
}
7579
}
7680

@@ -593,11 +597,13 @@ impl BookManager {
593597
let mut books: Vec<BookInfo>;
594598
#[cfg(feature = "pdf")]
595599
{
596-
if !is_pdf_enabled() {
600+
if !is_pdf_enabled() || !self.supports_graphics {
597601
books = self
598602
.books
599603
.iter()
600-
.filter(|book| book.format != BookFormat::Pdf)
604+
.filter(|book| {
605+
book.format != BookFormat::Pdf && book.format != BookFormat::Djvu
606+
})
601607
.cloned()
602608
.collect();
603609
} else {
@@ -693,6 +699,75 @@ mod tests {
693699
fs::write(path, contents.as_bytes()).unwrap();
694700
}
695701

702+
#[cfg(feature = "pdf")]
703+
#[test]
704+
fn get_books_filters_pdf_and_djvu_when_pdf_disabled() {
705+
use crate::settings::{is_pdf_enabled, set_pdf_enabled};
706+
707+
let temp_dir = TempDir::new().unwrap();
708+
// Create dummy files so discover_books_in_dir picks them up
709+
fs::write(temp_dir.path().join("novel.epub"), b"fake").unwrap();
710+
fs::write(temp_dir.path().join("paper.pdf"), b"fake").unwrap();
711+
fs::write(temp_dir.path().join("scan.djvu"), b"fake").unwrap();
712+
713+
let manager = BookManager::new_with_directory(temp_dir.path().to_str().unwrap());
714+
assert_eq!(manager.books.len(), 3, "all 3 files should be discovered");
715+
716+
let prev = is_pdf_enabled();
717+
set_pdf_enabled(false);
718+
719+
let filtered = manager.get_books();
720+
let formats: Vec<_> = filtered.iter().map(|b| b.format).collect();
721+
722+
set_pdf_enabled(prev);
723+
724+
assert!(
725+
!formats.contains(&BookFormat::Pdf),
726+
"PDF books should be filtered out when pdf is disabled"
727+
);
728+
assert!(
729+
!formats.contains(&BookFormat::Djvu),
730+
"DJVU books should be filtered out when pdf is disabled"
731+
);
732+
assert_eq!(filtered.len(), 1, "only the epub should remain");
733+
}
734+
735+
#[cfg(feature = "pdf")]
736+
#[test]
737+
fn get_books_filters_pdf_and_djvu_when_no_graphics_support() {
738+
use crate::settings::{is_pdf_enabled, set_pdf_enabled};
739+
740+
let temp_dir = TempDir::new().unwrap();
741+
fs::write(temp_dir.path().join("novel.epub"), b"fake").unwrap();
742+
fs::write(temp_dir.path().join("paper.pdf"), b"fake").unwrap();
743+
fs::write(temp_dir.path().join("scan.djvu"), b"fake").unwrap();
744+
745+
let manager = BookManager::new_with_directory(temp_dir.path().to_str().unwrap());
746+
assert_eq!(manager.books.len(), 3, "all 3 files should be discovered");
747+
748+
// pdf_enabled is true (default) — simulating a user who has never toggled the setting.
749+
// But the terminal doesn't support graphics, so PDFs/DJVUs should still be hidden.
750+
let prev = is_pdf_enabled();
751+
set_pdf_enabled(true);
752+
753+
let filtered = manager.get_books();
754+
let formats: Vec<_> = filtered.iter().map(|b| b.format).collect();
755+
756+
set_pdf_enabled(prev);
757+
758+
// This must hold regardless of the pdf_enabled setting:
759+
// a terminal without graphics cannot render PDFs/DJVUs.
760+
assert!(
761+
!formats.contains(&BookFormat::Pdf),
762+
"PDF books should be filtered out when terminal has no graphics support"
763+
);
764+
assert!(
765+
!formats.contains(&BookFormat::Djvu),
766+
"DJVU books should be filtered out when terminal has no graphics support"
767+
);
768+
assert_eq!(filtered.len(), 1, "only the epub should remain");
769+
}
770+
696771
#[test]
697772
fn load_exploded_epub_directory() {
698773
let temp_dir = TempDir::new().unwrap();

src/bookmarks.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ impl Bookmarks {
330330
bookmark.book_title = book_title;
331331
bookmark.book_author = book_author;
332332
bookmark.absolute_path = absolute_path;
333+
bookmark.last_read = chrono::Utc::now();
333334
if total_chapters.is_some() {
334335
bookmark.total_chapters = total_chapters;
335336
}
@@ -492,6 +493,49 @@ mod tests {
492493
assert_eq!(bookmark.absolute_path.as_deref(), Some("/abs/book.epub"));
493494
}
494495

496+
#[test]
497+
fn save_initial_bookmark_refreshes_last_read_for_existing_entry() {
498+
let mut bookmarks = Bookmarks::ephemeral();
499+
let path = "./book.epub";
500+
501+
bookmarks.save_initial_bookmark(
502+
path,
503+
"ch1".to_string(),
504+
Some(0),
505+
Some(10),
506+
None,
507+
Some("Title".to_string()),
508+
None,
509+
Some("/abs/book.epub".to_string()),
510+
);
511+
512+
let first_last_read = bookmarks.get_bookmark(path).unwrap().last_read;
513+
let refreshed_last_read = loop {
514+
bookmarks.save_initial_bookmark(
515+
path,
516+
"ch2".to_string(),
517+
Some(1),
518+
Some(10),
519+
None,
520+
Some("Updated Title".to_string()),
521+
None,
522+
Some("/abs/book.epub".to_string()),
523+
);
524+
525+
let bookmark = bookmarks.get_bookmark(path).unwrap();
526+
if bookmark.last_read > first_last_read {
527+
break bookmark.last_read;
528+
}
529+
530+
std::thread::sleep(std::time::Duration::from_millis(1));
531+
};
532+
533+
let bookmark = bookmarks.get_bookmark(path).unwrap();
534+
assert!(refreshed_last_read > first_last_read);
535+
assert_eq!(bookmark.book_title.as_deref(), Some("Updated Title"));
536+
assert_eq!(bookmark.chapter_index, Some(0));
537+
}
538+
495539
/// Opening a book from "All Libraries" must NOT create a bookmark entry
496540
/// in the current library. This simulates the exact main_app.rs flow:
497541
/// open_book_for_reading_by_path calls set_metadata which creates an entry.

src/main.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use bookokrat::event_source::KeyboardEventSource;
1616
#[cfg(feature = "pdf")]
1717
use bookokrat::inputs::UnifiedEventSource;
1818
use bookokrat::library;
19-
use bookokrat::main_app::{App, run_app_with_event_source};
19+
use bookokrat::main_app::{App, run_app_with_event_source, should_auto_load_recent};
2020
#[cfg(feature = "pdf")]
2121
use bookokrat::main_app::{
2222
set_kitty_delete_range_support_override, set_kitty_shm_support_override,
@@ -204,7 +204,11 @@ fn main() -> Result<()> {
204204
}
205205
});
206206
// In test mode: no auto-load, ephemeral bookmarks
207-
let auto_load_recent = args.file_path.is_none() && !args.test_mode;
207+
let auto_load_recent = should_auto_load_recent(
208+
args.file_path.as_deref(),
209+
args.test_mode,
210+
args.continue_reading,
211+
);
208212
let (bookmark_file, comments_dir) = if args.test_mode {
209213
(None, None)
210214
} else if let Ok(ref paths) = lib_paths {

src/main_app.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,14 @@ pub enum AppAction {
297297
Quit,
298298
}
299299

300+
pub fn should_auto_load_recent(
301+
file_path: Option<&str>,
302+
test_mode: bool,
303+
continue_reading: bool,
304+
) -> bool {
305+
file_path.is_none() && !test_mode && !continue_reading
306+
}
307+
300308
#[cfg(feature = "pdf")]
301309
struct PdfEventResult {
302310
handled: bool,
@@ -595,11 +603,18 @@ impl App {
595603
comments_dir: Option<&Path>,
596604
image_cache_dir: Option<PathBuf>,
597605
) -> Self {
598-
let book_manager = match book_directory {
606+
let mut book_manager = match book_directory {
599607
Some(dir) => BookManager::new_with_directory(dir),
600608
None => BookManager::new(),
601609
};
602610

611+
#[cfg(feature = "pdf")]
612+
let startup_caps = crate::terminal::detect_terminal_with_probe();
613+
#[cfg(feature = "pdf")]
614+
{
615+
book_manager.supports_graphics = startup_caps.supports_graphics;
616+
}
617+
603618
let navigation_panel = NavigationPanel::new(&book_manager);
604619
#[cfg(any(test, feature = "test-utils"))]
605620
let mut text_reader = MarkdownTextReader::new_without_image_support();
@@ -644,9 +659,6 @@ impl App {
644659
Rect::new(0, 0, 80, 24)
645660
};
646661

647-
#[cfg(feature = "pdf")]
648-
let startup_caps = crate::terminal::detect_terminal_with_probe();
649-
650662
let mut app = Self {
651663
book_manager,
652664
navigation_panel,
@@ -1234,6 +1246,7 @@ impl App {
12341246
None => crate::terminal::detect_terminal_with_probe(),
12351247
};
12361248
self.pdf_supports_graphics = caps.supports_graphics;
1249+
self.book_manager.supports_graphics = caps.supports_graphics;
12371250
self.pdf_supports_scroll_mode = caps.pdf.supports_scroll_mode;
12381251

12391252
if let Some(reason) = caps.pdf.blocked_reason.as_ref() {
@@ -7039,22 +7052,32 @@ mod tests {
70397052
app.save_bookmark_with_throttle(true);
70407053
}
70417054

7042-
// --- Session 2: `bookokrat -c` ---
7055+
// --- Session 2: simulate `bookokrat -c` startup from main.rs ---
70437056
{
7057+
let auto_load_recent = should_auto_load_recent(None, false, true);
7058+
assert!(
7059+
!auto_load_recent,
7060+
"`bookokrat -c` must skip the home-library auto-open path"
7061+
);
7062+
70447063
let mut app = App::new_with_config(
70457064
Some(dir.path().to_str().unwrap()),
70467065
Some(home_bm_path.to_str().unwrap()),
7047-
false,
7066+
auto_load_recent,
70487067
None,
70497068
Some(dir.path().join("img_cache2")),
70507069
);
70517070

70527071
let recent = crate::library::find_most_recent_book_in(&libraries_dir)
70537072
.expect("should find book_b");
70547073

7055-
// The fix: open with source_bookmarks so context_override is set
70567074
app.open_book_for_reading_with_source_bookmarks(&recent.path, &recent.source_bookmarks)
70577075
.unwrap();
7076+
assert_eq!(
7077+
app.navigation_panel.current_book_path.as_deref(),
7078+
Some(book_b_abs.to_str().unwrap()),
7079+
"`bookokrat -c` should reopen the cross-library book"
7080+
);
70587081
app.save_bookmark_with_throttle(true);
70597082
}
70607083

0 commit comments

Comments
 (0)