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

Crash in string value for multiple objects #2242

Closed
pjrobertson opened this issue Jul 27, 2016 · 3 comments
Closed

Crash in string value for multiple objects #2242

pjrobertson opened this issue Jul 27, 2016 · 3 comments
Assignees

Comments

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jul 27, 2016

It seems @skurfer's latest changes cause a crash in stringValue. To reproduce:

  • Create a trigger for a 'combined' object for a nonexistent file path. E.g. in text mode type ~/nowhere, pres ESC, then comma, then enter text mode again.
  • You'll be left with a dObject of 2 text objects (to nonexistent files). The action is irrelevant
  • Now add a keyboard shortcut to the trigger
  • Try and activate the trigger (whilst debugging Xcode). You'll see there's an infinite loop - caused by calling [object splitObjects] in the stringValue method.

This was broken by 9e84daa

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Jul 27, 2016

Basically calling [obj splitObject] calls [obj children] which calls [obj loadChildren] which calls [stringHandler loadChildrenForObject:obj] which calls [obj stringValue] which calls... [obj splitObject]

I'm sure there are other ways of reproducing this, but this is how I just came across it

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Jul 27, 2016

Although, 8f902d7 could also be the bug. Checking for a real object using the stringValue as the identifier seems very hacky, although I know what you were trying to fix there

@skurfer skurfer self-assigned this Jul 27, 2016
@skurfer
Copy link
Member

@skurfer skurfer commented Jul 27, 2016

Checking for a real object using the stringValue as the identifier seems very hacky, although I know what you were trying to fix there

In general, text objects don’t have identifiers, but they did for a brief period as you probably remember. I think that code was there so any triggers (or other stored QSCommands) created during that period would still work. So most of the time, it doesn’t do anything.

But I too question using stringValue now. Since we know exactly what we’re after, it seems like objectForType: would work just as well.

Anyway, I’ll get this fixed.

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

No branches or pull requests

2 participants