Skip to content

[DocSettingTweak] Use absolute path for document settings#14418

Merged
Frenzie merged 8 commits into
koreader:masterfrom
DemonicSavage:dst_relative
Oct 16, 2025
Merged

[DocSettingTweak] Use absolute path for document settings#14418
Frenzie merged 8 commits into
koreader:masterfrom
DemonicSavage:dst_relative

Conversation

@DemonicSavage

@DemonicSavage DemonicSavage commented Oct 5, 2025

Copy link
Copy Markdown
Contributor

External plugins that load files may load them using relative paths instead of absolute paths. This PR enables these plugins to make use of the per-directory document settings feature.


This change is Reviewable

External plugins that load files may load them using relative paths instead of absolute paths. This enables these plugins to make use of the per-directory document settings feature.
@DemonicSavage

Copy link
Copy Markdown
Contributor Author

This fixes this issue with Rakuyomi. I made a PR there as well, but a) the main dev seems quite sporadically active and b) I think this is a better solution, as it potentially fixes other plugins that may do the same thing.

@Frenzie

Frenzie commented Oct 6, 2025

Copy link
Copy Markdown
Member

@yparitcher Do you know if there could be any side effects?

@yparitcher

Copy link
Copy Markdown
Member

Yes,

The plugin is now agnostic to relative or absolute paths, both should work, we just follow whatever we are given, we also set the doc_path to doc.file the same as Docsettings does on init, anything expecting doc_path to match doc.file will break with this change, this shouldnt be changed here first, if this change is needed, it should probably be changed in ReaderUi where doc.file is set and then the filepath will be consistent everywhere, both for a file changed with directory defaults and for a unchanged file.

This should be changed in the plugin to match the rest of koreader, as i am sure that this is not the only place breaking, and this is for sure not the root cause.

@DemonicSavage

Copy link
Copy Markdown
Contributor Author

That makes sense. I have changed it to normalize the path in ReaderUI instead.

Comment thread frontend/apps/reader/readerui.lua Outdated
Comment thread frontend/apps/reader/readerui.lua Outdated

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me.

@Frenzie Frenzie added this to the 2025.10 milestone Oct 6, 2025
@DemonicSavage

Copy link
Copy Markdown
Contributor Author

What is the status on this? It's slated for 2025.10, does that mean it will be merged before that? Or is it missing something?

I'm sorry for such a basic question as this is my first time contributing to open source stuff that's not just me.

@Frenzie Frenzie merged commit 9a7b362 into koreader:master Oct 16, 2025
4 checks passed
@Frenzie

Frenzie commented Oct 16, 2025

Copy link
Copy Markdown
Member

In this project, the approved and 2025.10 are notes to myself and others. We used to just do that in writing to roughly the same effect before GitHub added the feature, but this way you see it nicely in the overview and you can sort and filter on it in the list. The approved so that I know who looked at it already and found it fine, the 2025.10 so that I can quickly find it when looking for things that should still be merged. But only after there have been at least a few days for other people to comment if they want, unless it's some small or very obvious thing. (This PR doesn't qualify as obvious.)

But it's not typically a 2025.10 in the sense that it has to be finished or merged before a release happens, which may be the case for other projects. It's an indication that I'd like it to be in or that I expect it'll be ready in time.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
…4418)

External plugins that load files may load them using relative paths instead of absolute paths. This enables these plugins to make use of the per-directory document settings feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants