Restore last opened page on previously opened document #84

liltrendi · 2025-08-07T16:35:42Z

Overview

We need a way to restore the last opened page when reopening a previously opened document.

Addresses #78.

Problem

Often times I found myself having to remember the last page I was on when I reopened a PDF so I could navigate back to it. As you can imagine, on large PDFs this gets annoying pretty quickly.

Solution

I opted to fix this by tracking the path of the last opened document alongside the last opened page number in a key/value fashion. This information is stored under the configuration directory of the current user (cross platform).

Changelog

A DocumentHistoryConfig struct under config.rs tracks the last viewed document and its page
Basic tests for the above surrounding the above logic asserts functionality

Demo

Screen.Recording.2025-08-07.at.19.29.09.mov

itsjunetime · 2025-08-21T22:32:23Z

Sorry I haven't been responsive here yet - this PR just makes some big functional changes (specifically, we don't store any data to disk yet and this changes that), so I want to make sure I have the proper time to look over this and think about the approach. I'm hopeful that I can get to this within the week.

liltrendi · 2025-08-21T22:35:11Z

Sorry I haven't been responsive here yet - this PR just makes some big functional changes (specifically, we don't store any data to disk yet and this changes that), so I want to make sure I have the proper time to look over this and think about the approach. I'm hopeful that I can get to this within the week.

No worries man, love the work you've done so far, eager to see what the roadmap will look like in the long term! 🚀

itsjunetime

Thanks for contributing! There's still some work that needs to be done before this feature could be added, but it's looking good so far.

I have a few questions and comments about how various parts of this work, which I hope you can answer when you have the time :)

I do also think there needs to be some more discussion about what users will expect from a feature like this before we push this through. Specifically:

Do you think users will expect this to save every time they change the page, or only when they exit? Like maybe they'll open the same document in another instance of tdf and expect it to go to the same page so that they can look at it side-by-side or something (I'm not necessarily saying I know which users will expect, I'm just curious about your thoughts)
Should this be default behavior, or do you think it could be confusing for users and should thus be hidden behind some sort of configuration? Personally, I think it would be slightly surprising for most people (and so should maybe be hidden behind a flag), but also it's very easy for people to go back to the beginning of documents while it's hard to go back to the page you were previously at (so maybe it should just be default behavior). Thoughts?

I hope this isn't too daunting, and I'm excited to hear what you think :)

src/config.rs

itsjunetime · 2025-09-06T21:20:35Z

src/config.rs

+use serde::{Deserialize, Serialize};
+
+#[derive(Serialize, Deserialize, Default)]
+pub struct DocumentHistoryConfig {


I think that calling this a 'config' is somewhat misleading - this isn't meant to be edited by the user, right? Just overwritten by tdf when the user switches pages?

I went with the more generic DocumentHistory (nomenclatures are hard work ;) ) that more closely represents internal state rather than configuration that users could fiddle with. What do you think?

src/config.rs

src/main.rs

liltrendi · 2025-10-03T22:53:49Z

Thanks for contributing! There's still some work that needs to be done before this feature could be added, but it's looking good so far.

I have a few questions and comments about how various parts of this work, which I hope you can answer when you have the time :)

I do also think there needs to be some more discussion about what users will expect from a feature like this before we push this through. Specifically:

Do you think users will expect this to save every time they change the page, or only when they exit? Like maybe they'll open the same document in another instance of tdf and expect it to go to the same page so that they can look at it side-by-side or something (I'm not necessarily saying I know which users will expect, I'm just curious about your thoughts)

Should this be default behavior, or do you think it could be confusing for users and should thus be hidden behind some sort of configuration? Personally, I think it would be slightly surprising for most people (and so should maybe be hidden behind a flag), but also it's very easy for people to go back to the beginning of documents while it's hard to go back to the page you were previously at (so maybe it should just be default behavior). Thoughts?

I hope this isn't too daunting, and I'm excited to hear what you think :)

Hey @itsjunetime, my apologies for having left this pending for so long, lots happened life-wise and I'm just getting back. Thanks for taking your time to review my PR! I'll address the other comments shortly, but in response to your main questions:

I think users would expect the save to happen on every page change. Many users are already accustomed to this sort of behavior from other mainstream apps (video players, browsers, e-readers etc), and if they were to open it in another instance they'd likely expect to continue from the same page they were viewing. Performance-wise, I'd imagine the overhead to be negligible since it's just a binary file write. Personally, if the app for some reason crashed while I was using it (say my computer shut down unexpectedly), I'd like to reopen the same page I left my doc at.
I personally think this should be an opt-out feature, not opt-in. As the default behavior, it aligns with a lot of users' expectations as most apps have that "remember where you were" feature, and solves a real problem because it's a lot harder to keep track of what page you were last on than just scrolling to the top. It would be counterintuitive to hide it behind a feature flag because most users would just assume the feature doesn't exist, and I think they'd rather have the "it just works" feel. On the other hand, some users might be confused if they always expect their document to start from page 1. For me, as a user, I imagined this to be a very needed feature because it solves a problem I got kinda annoyed at, which was having to start from the first page and find where I was last. I think we could add a --no-history flag or something that disables this behavior if a user doesn't want it as the default.

Thanks for hearing me out, let me know what you think; always open to feedback and new ideas!

- replaced serde with bitcode for efficient binary serialization - better struct renaming to DocumentHistory to reflect internal state vs user-defined settings - implemented functional-style error handling with WrappedErr - notify renderer/converter threads on page jumps for immediate processing - removed redundant tests

itsjunetime

Ok, I finally got some time to look this over, and yeah I like this a lot better.

I also think, as I've sat with it, that you're right that this would be better opt-out than opt-in. With that said, I do think we should add the --no-history flag like you suggested.

I also think the code you added is pretty solid and looks mostly good besides a few issues, specifically with errors being created but not acted upon. I think once you fix those and answer the other questions I mention, we can merge this in :)

itsjunetime · 2025-11-30T02:45:24Z

src/history.rs

+	fn history_path() -> Result<PathBuf, WrappedErr> {
+		config_dir()
+			.map(|p| p.join("tdf.history.bin"))
+			.ok_or_else(|| WrappedErr("Could not determine history directory".into()))


If I were a user and I received this error, I would be kinda frustrated, since I wouldn't really know what to do with it. But I (as a programmer) am not quite certain how we could add enough information to be useful without completely overloading the user with detail. Maybe linking them to the documentation for the config_dir fn would be good? Or maybe that's too technical for someone who just wants a PDF reader?

Not a blocker, just thinking. Do you have an opinion?

itsjunetime · 2025-11-30T02:46:36Z

src/history.rs

+	}
+}
+
+#[cfg(test)]


I appreciate you adding tests - we don't have much tests in this repo (I'm mostly relying on the benchmarks' ability to run as tests, but that doesn't really verify any correctness - it just verifies that nothing crashes), so this is a good step in the right direction.

itsjunetime · 2025-11-30T02:48:41Z

src/main.rs

+		WrappedErr(format!("Couldn't initialize document history: {e}").into());
+		DocumentHistory::default()


I think you missed something here - is this error supposed to be returned? Or just logged? Or displayed to the user?

itsjunetime · 2025-11-30T02:53:28Z

src/main.rs

+	});
+	let restored_page = document_history
+		.last_pages_opened
+		.get(&path.to_string_lossy().to_string())


mmm I would like to be able to store the paths as PathBufs or OsStrings but it looks like that doesn't really work in bitcode (yet) due to some unresolved questions (?): SoftbearStudios/bitcode#86. Maybe we can just keep that in mind for the future, and try to switch over if they ever support it.

itsjunetime · 2025-11-30T02:59:05Z

src/main.rs

+		to_renderer
+			.send(RenderNotif::JumpToPage(page))
+			.map_err(|e| {
+				WrappedErr(format!("Couldn't tell renderer to jump to restored page: {e}").into())
+			})?;
+		to_converter
+			.send(ConverterMsg::GoToPage(page))
+			.map_err(|e| {
+				WrappedErr(format!("Couldn't tell converter to jump to restored page: {e}").into())
+			})?;


I wonder if passing these 'initial page' parameters in to the renderer and converter threads when starting them up would be faster? It might save them from starting to render an irrelevant page first, but the extra time that the threads would have to wait on decoding the file from disk might negate that?

I doubt it would make much difference at all, but if you can figure out the benchmark system tdf has (don't worry, it's completely undocumented lmao, sorry abt that), you could try out both and see if either of them improve the render_first_page family of benchmarks? If not, though, don't worry, I can check it out later.

itsjunetime · 2025-11-30T02:59:58Z

src/main.rs

+		.insert(path.to_string_lossy().to_string(), tui.page);
+
+	if let Err(e) = document_history.save() {
+		WrappedErr(format!("Failed to save last opened page: {e}").into());


Same thing as up above with just creating the error - just creating the error won't do anything, you've gotta log it or display it to the user or panic or smth.

itsjunetime · 2025-11-30T03:04:50Z

~~Also I'm gonna rebase this rq so we can get CI going again.~~ nvm, looks like I can't push for some reason, feel free to merge main back in or whatever you need to get CI going again.

liltrendi added 3 commits August 7, 2025 18:46

feat: track opened pdf and its last opened page

e35f6f8

chore: format tracked files

0663ddd

test: basic tests for config loading

7519515

liltrendi mentioned this pull request Aug 7, 2025

new feature - can i relocate to last time close position when i open it. #78

Open

Merge branch 'main' into feat/reopen-last-opened-page

dda95bd

liltrendi changed the title ~~Restore Last Opened Page On Previously Opened PDF~~ Restore last opened page on previously opened document Aug 21, 2025

itsjunetime reviewed Sep 6, 2025

View reviewed changes

Merge branch 'main' into feat/reopen-last-opened-page

d90729f

liltrendi added 3 commits October 4, 2025 02:05

fix: remove duplication

8207e60

refactor: simplify DocumentHistory & enforce proper naming

b0d93da

itsjunetime reviewed Nov 30, 2025

View reviewed changes

		WrappedErr(format!("Couldn't initialize document history: {e}").into());
		DocumentHistory::default()

Restore last opened page on previously opened document #84

Are you sure you want to change the base?

Restore last opened page on previously opened document #84

Uh oh!

Conversation

liltrendi commented Aug 7, 2025

Overview

Problem

Solution

Changelog

Demo

Uh oh!

itsjunetime commented Aug 21, 2025

Uh oh!

liltrendi commented Aug 21, 2025

Uh oh!

itsjunetime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liltrendi commented Oct 3, 2025

Uh oh!

itsjunetime left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsjunetime commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

itsjunetime commented Nov 30, 2025 •

edited

Loading