Error in process execution forces automatic scanning on status update

projectgus commented 11 months ago

This is a pretty minor bug that I noticed but thought was worth reporting.

Steps to reproduce

Customise magit-todos-update to either nil (manual updates) or a number of seconds to cache.
Run manual magit-todos-update to scan for TODOs.
An error occurs while running the process (for example, rg reports permission denied error for a file in the repo working directory).

Afterwards, refreshing magit status will continue to not automatically scan for to-dos (as per the magit-todos-update selection).

Afterwards, refreshing magit status will trigger a new scan each time, behaving as if magit-todos-update was set to t.

Local variable magit-todos-updating is set to t before evaluating magit-todos--async-start-process. It is only set to nil again by the finish-func callback (and this only runs if the process succeeds.)
If there's an error then the magit-todos-updating stays t until/unless the scan process re-runs and succeeds. This causes the magit status callback to be treated as a manual update each time.

alphapapa commented 11 months ago

Hi Angus,

This might be the best-written bug report I've ever received. Should be a simple fix. Thank you!

alphapapa commented 6 months ago

Fixing this has the potential to cause other bugs, so targeting it for 1.8 so it can sit on master for a while.