Repair broken triggers

[skurfer]

Here’s my proposed fix for #1858. “Repairing” the trigger is a pretty inexpensive process, and it only happens once per launch of QS.

The only questionable change is in QSTriggerCenter.m. Is there a reason we were short-circuiting execute, @tiennou? Was it just to speed things up?

[pjrobertson]

How come you say 'it only happens once per launch of QS'? If a trigger's dObject is text, then surely it's run every time.

Also, this doesn't fix the problem where I try and execute a trigger before the catalog is loaded.
You've (possible, in a hacky way... hehe) fixed the problem of a trigger being created before the catalog is loaded, but not necessarily triggered before.

Not that this is the wrong solution, just we know the real solution is not really feasible, so we might as well make the 'not right' solution as good as possible :/

[skurfer]

How come you say 'it only happens once per launch of QS'? If a trigger's dObject is text, then surely it's run every time.

OK, that part happens every time. I just meant the replacement of the objects happens once.

(If you look at the setters, they test to see if the objects are the same, so text shouldn’t get replaced. Well, maybe it will be a different-but-identical string object and get replaced. Do we care?)

Also, this doesn't fix the problem where I try and execute a trigger before the catalog is loaded.

Right. The trigger will still fail if the object in question hasn’t been scanned in yet. Don’t see any way around that, but it’s largely theoretical. See if you can pull it off in practice. I can’t. 😃

[tiennou]

git blame says it's been that way since the beginning of time, so the reason is lost. I added the comment because it smelled fishy, but wasn't sure what would break if I changed it.

[pjrobertson]

@skurfer I apologise for my laziness in moving this forward. The only thing I'm slightly concerned about is bfb5ea5 and any unknown effects it might have. Have you been running with this branch for a while now?

[skurfer]

Yes, I’ve been using it and haven’t seen a single side-effect. I feel better about it than the old way, anyway. executeTrigger and [trigger execute] should probably behave consistently, right?

[pjrobertson]

OK, for now I think this is the best we've got :)

[skurfer]

Sweet! So close to 1.2.0…

[pjrobertson]

Yep…. let’s do this! :D

On 23 Gorff 2014, at 15:41, Rob McBroom notifications@github.com wrote:

Sweet! So close to 1.2.0…

—
Reply to this email directly or view it on GitHub.