Skip to content

Profiles: auto exec promptly#14133

Merged
hius07 merged 2 commits into
koreader:masterfrom
hius07:profiles-auto-exec-promptly
Aug 5, 2025
Merged

Profiles: auto exec promptly#14133
hius07 merged 2 commits into
koreader:masterfrom
hius07:profiles-auto-exec-promptly

Conversation

@hius07

@hius07 hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member

Profiles are auto-executed in a safe way: within nextTick() or even tickAfterNext(), to avoid interference with other operations/events.
But sometimes (e.g. on closing a document) a user needs to execute it promtly, provided there are no conflicts with other actions.
Closes #14126.

1

This change is Reviewable

@hius07 hius07 added the Plugin label Aug 3, 2025

@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.

I'm not quite sure of the name but I'm currently drawing a blank on other ideas.

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

I thought about "earlier", but earlier than what?
"immediately" conflicts with the above "Ask to execute".

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

Execute promptly
Usually a profile is auto-executed with a slight delay, to avoid conflicts with other operations. If your profile is not executed in a way you want, for example when you export highlights on document closing, enable this feature.

@offset-torque, proposition for the User guide.

(Minor mistake in the guide: Combined with book is being opened for the first time is not applicable to the "on book closing" trigger)

UPDATED HELP TEXT:
Usually a profile is auto-executed in a certain order with other operations, to avoid conflicts. If your profile is not executed in the way you want, for example when you export highlights on document closing, enable this feature to auto-execute the profile earlier.

@Frenzie

Frenzie commented Aug 3, 2025

Copy link
Copy Markdown
Member

Though mind that slight delay can probably be measured in sub-microseconds most of the time, correct?

@poire-z

poire-z commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

Feels a bit quite technical, and I'm not sure a normal (even if power-)user would know when to enable this or not.
Are there other use cases, other than on closing document, that would need it ?
If not, can't you detect it's being run on closing document, and decide that programatically ?

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

I'm afraid some conflicts may appear depending on the profile actions.
So let a user to try and to disable it if something goes wrong.

Generally, I would always try to auto-execute with this feature enabled, and to disable it when execution fails only.

@jonnyl2

jonnyl2 commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

Generally, I would always try to auto-execute with this feature enabled, and to disable it when execution fails only.

Wouldn't it then make more sense to have prompt execution as the default behavior, and have a checkbox for "Execute with a slight delay (to avoid conflicts)" (or similar phrasing) as an option?

@Frenzie

Frenzie commented Aug 3, 2025

Copy link
Copy Markdown
Member

Yes, if it's intended as the default it can be disabled and say something like Avoid conflicts (may not execute on close document).

I wouldn't speak of delay if it can be avoided since next tick isn't a delay in human terms.

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

This may cause an increasing number of questions "My auto-exec fails, what should I do" and our answers "Enable the checkbox".

Writing "I would always..." I meant an expirienced user.

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

UPDATED HELP TEXT:
Usually a profile is auto-executed in a certain order with other operations, to avoid conflicts. If your profile is not executed in the way you want, for example when you export highlights on document closing, enable this feature to auto-execute the profile earlier.

@Frenzie

Frenzie commented Aug 3, 2025

Copy link
Copy Markdown
Member

Let's also as a hold callback.

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

Okay, waiting for @offset-torque 's comments.

@offset-torque

Copy link
Copy Markdown

Just some clarifications:

Usually a profile is auto-executed in a certain order with other operations, to avoid conflicts. If your profile is not executed in the way you want, for example when you export highlights on document closing, enable this feature to auto-execute the profile earlier.

From this text I understand this (from the POV of a normal user):

  1. "Other operations" means KOReader operations like closing a book, not "other operations within the profile".
  2. And when a user enables this, profile action (practically) gets the priority over everything else, so it can work before the closing (or other) action.

Are these correct?


As a problem, first thing that comes to my mind is same as what poire-z said:

Feels a bit quite technical, and I'm not sure a normal (even if power-)user would know when to enable this or not.

So for this option, I guess we (devs) too don't know the exact problematic conditions that might arise. So basically we are telling the users this:
"If your profile fails, try enabling this before opening a issue".

And a question: Currently there are no other troubleshooting steps for profiles except this, right?

@hius07

hius07 commented Aug 3, 2025

Copy link
Copy Markdown
Member Author

"If your profile fails, try enabling this before opening a issue".

Just the opposite. Enabling this new checkbox allows using some actions, like exporting highlights on document closing.
But it also may cause (unknown) issues, the number of action combinations in a profile is almost infinite and yes, we don't know how they may interfere.

So I propose to not change the default state of things, and to have an option to enable (or to try to enable) some combinations of trigger/condition/actions, on the user's own risk.

Finally, "if your profile fails, try to enable this new option. If it fails again (maybe in the other way), your profile cannot be auto-executed, report it to us, and we will think again".

@offset-torque

Copy link
Copy Markdown

Got it. I added this to my task list hius07. When I write the update based on your replies, we will clarify it with your feedback then.

@Commodore64user

Copy link
Copy Markdown
Member

one of my issues with profiles and status bar presets, is that they [presets] don't get applied when closing a book, I wonder if this will do it...

@hius07

hius07 commented Aug 4, 2025

Copy link
Copy Markdown
Member Author

Yes, it will.

Final recommended wording for the User Guide:
"Enable this option to execute the profile before some other operations triggered by the event.
For example, with a trigger "on document closing" the profile will be executed before the document is closed.
When setting an auto-execution of a profile, always enable this option.
If the profile is not executed in the proper way, disable the option".

end,
},
{
text = _("Execute promptly"),

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.

could be something like "Pre-event execution" or "Execute before event action"

@offset-torque

Copy link
Copy Markdown

Final recommended wording for the User Guide:

Noted

@hius07 hius07 merged commit 176d3de into koreader:master Aug 5, 2025
4 checks passed
@hius07 hius07 deleted the profiles-auto-exec-promptly branch August 5, 2025 09:25
@hius07 hius07 added this to the 2025.08 milestone Aug 5, 2025
@Commodore64user

Copy link
Copy Markdown
Member

had this crash while closing a book with "execute promptly"

./luajit: frontend/apps/reader/modules/readerrolling.lua:1754: attempt to index field 'document' (a nil value)
stack traceback:
	frontend/apps/reader/modules/readerrolling.lua:1754: in function '_stepRerenderingAutomation'
	frontend/apps/reader/modules/readerrolling.lua:1789: in function 'action'
	frontend/ui/uimanager.lua:1001: in function '_checkTasks'
	frontend/ui/uimanager.lua:1447: in function 'handleInput'
	frontend/ui/uimanager.lua:1567: in function 'run'
	./reader.lua:280: in main chunk
	[C]: at 0x00013eb1

@hius07

hius07 commented Aug 18, 2025

Copy link
Copy Markdown
Member Author

What is the action?

@hius07

hius07 commented Aug 18, 2025

Copy link
Copy Markdown
Member Author

I see, "Load preset". It doesn't depend on "Execute promptly", but on "Partial rendering" (the document is closed on rerendering). Thanks, I shall dig into it.
UPD: not only "partial rendering". Must be investigated.

@Commodore64user

Copy link
Copy Markdown
Member

What is the action?

it's a a profile, that has one single action, it loads a status bar preset.

@hius07

hius07 commented Aug 18, 2025

Copy link
Copy Markdown
Member Author

"Execute promptly" is enabled
The crash occurs if the profile to be auto-executed causes the document rerendering.
In our case: when the preset to be loaded changes the footer size (eg: enable/disable the progress bar).
What's happening:
-ReaderUI sends "CloseDocument" event
-the profile catches it and loads the footer preset that requires the document rerendering
-ReaderRolling starts rerendering
-but the document closing continues, and rerendering crashes.
(When testing, I got a crash in ReaderAnnotation if there were highlights, or in ReaderBack otherwise. Can be in other modules I suppose).

"Execute promptly" is disabled
The footer preset is loaded in nextTick (ie does nothing, the document is already closed). No crash.

By the way, no crash for PDF with "Execute promptly" enabled.

I'm afraid this cannot be fixed due to the program architecture.
"Execute promptly" has limitations in usage, and it is disabled by default.
(I'm still thinking about it, maybe some ideas will come)

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FR: on book closing, autoexport notes in current book

6 participants