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

Don't store IDs and archives for a QSCommand (in the plist) #2426

Merged
merged 1 commit into from Mar 2, 2018
Merged

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Feb 15, 2018

Fixes issue #2419

This should fix #2419 (I think), it seems that in this case:

  1. You create a trigger for an dObject that's not in the catalog
  2. That dObject subsequently gets added to the catalog
  3. You resave the trigger (I guess it happens on every quit of the app)

Then the trigger would have both a directArchive (created originally when the object isn't in the catalog) and then subsequently a directID (when the object gets added to the catalog). This causes problems in QSCommand.m:-(QSObject *)dObject (I think).

Now the ideal solution would be to store both the directID and directArchive, and (for the event that you add a trigger for something that's in the catalog, then it subsequently gets removed from the catalog) but that's another task for another day.

@skurfer
Copy link
Member

@skurfer skurfer commented Feb 22, 2018

I don’t see a problem in - [QSCommand dObject]. If there’s a directID and it can be used to find and object, the archive is ignored. But there’s obviously a problem somewhere. Rather than try to find it, I’ll just merge this locally and test it out.

Were you able to reproduce the problem prior to this change?

@skurfer skurfer merged commit ad453cc into master Mar 2, 2018
2 checks passed
@skurfer skurfer deleted the 2419 branch Mar 2, 2018
@skurfer
Copy link
Member

@skurfer skurfer commented Mar 2, 2018

I haven’t tried to reproduce the problem to know if this fixes it, but it doesn’t seem to break anything, so…

Please look at and adjust the release notes if I explained this incorrectly @pjrobertson. And feel free to change the spelling of “behavior” if you must. 😉

skurfer added a commit that referenced this issue Mar 2, 2018
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Mar 17, 2018

looks good. It doesn't break anything... haha, best kind of code change out there ;-)

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.

2 participants