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

Addons: Delay updating of tasks list until document has been colorised #1257

Merged
merged 4 commits into from
Sep 17, 2023

Conversation

eht16
Copy link
Member

@eht16 eht16 commented Jun 4, 2023

Closes #1254.

This is not completely perfect but will do the trick to delay the parsing of tasks in the current document until the main loop has triggered the also delayed colourising of the current document which is necessary for parsing the tasks.

Copy link
Member

@elextr elextr left a comment

Choose a reason for hiding this comment

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

Perhaps get the doc pointer first and check valid, then link the tasks object lifetime to the doc (if it isn't already).

Otherwise get current doc in the callback, since tasks doesn't write to the doc it won't do any harm if it scans tasks for the visible doc, not the one initially requested so long as its re-scanned when it becomes visible.

The chances of the doc being deleted between the idle being scheduled and the callback is unlikely, but who knows what them plugins will do. Better not to use dangling pointers.

{
AoTasksUpdateTasksForDocArguments *arguments = data;
AoTasks *t = arguments->t;
GeanyDocument *doc = arguments->doc;
Copy link
Member

Choose a reason for hiding this comment

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

Needs to check valid since its an unknown period and unknown actions before it happens (isn't asynchronous fun!)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if there is more than checking data for NULL. BUt at least I've added this.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of DOCVALID()

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, done and also in a few more places in the Addons plugin.

{
AoTasksUpdateTasksForDocArguments *arguments = data;
AoTasks *t = arguments->t;
Copy link
Member

Choose a reason for hiding this comment

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

Will the lifetime of AoTasks object guarantee its still valid (see below)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so. The AoTasks object exists as long as the Addons plugin is loaded, independently of opened documents and independent of whether the Tasks addon is enabled or not.

While in theory possible I think it's highly unlikely that a user will manage to unload the Addons plugin or even only to quit Geany so that the AoTasks object will be destroyed while an idle callback is still waiting to be executed.

@eht16
Copy link
Member Author

eht16 commented Jul 9, 2023

@elextr fine by you now?

@eht16
Copy link
Member Author

eht16 commented Aug 5, 2023

I'd like to merge this in a few days if there are no objections or further remarks.

@elextr
Copy link
Member

elextr commented Aug 5, 2023

Oops, missed the changes being added, fine by me.

@eht16 eht16 merged commit 45a6d0b into geany:master Sep 17, 2023
2 checks passed
@eht16 eht16 deleted the issue1254_delay_task_update_for_colorising branch September 17, 2023 09:28
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.

TODO tasks need the autorefresh button when opening a file to be visible 1.38 kubuntu 22.04, plasma 5.24
2 participants