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

Fixes #1371

Merged
merged 5 commits into from
Feb 21, 2013
Merged

Fixes #1371

merged 5 commits into from
Feb 21, 2013

Conversation

pjrobertson
Copy link
Member

Another fixes branch. This one I came across going Catalog Prefs → Quicksilver → internal commands → drawer → contents.

Seems like internal message/commands aren't correctly being created, that's the real problem. But this'll stop a crash if there's ever a nil string again.

@pjrobertson
Copy link
Member Author

The 2nd commit will stop what the 1st commit fixed from ever happening (in this case).

Seems like QSInternalObjects and QSInternalMessages are completely messed up.
Anybody able to explain what they are? @skurfer? @tiennou ? :(

@tiennou
Copy link
Member

tiennou commented Feb 3, 2013

The only default Internal Object I know is the Trash in the Finder plugin. And it doesn't work correctly too. Internal objects are supposed to be like proxies for things related to the plugin (like Trash), and I think Internal Messages are just simple wrappers around plain Obj-C calls. But yeah, I seem to remember they behave strangely.

@skurfer
Copy link
Member

skurfer commented Feb 4, 2013

What if identifier is nil too? It's not required (though I think it should be).

@pjrobertson
Copy link
Member Author

Yeah, that actually happened once. I was going to just change it to add a
string if nothing was found. Any recommendations?

@"Unknown" ?

On 4 February 2013 18:35, Rob McBroom notifications@github.com wrote:

What if identifier is nil too? It's not required (though I think it
should be).


Reply to this email directly or view it on GitHubhttps://github.com//pull/1371#issuecomment-13091734.

@skurfer
Copy link
Member

skurfer commented Feb 4, 2013

Sure.

@skurfer
Copy link
Member

skurfer commented Feb 21, 2013

Should we also use nameString (instead of [drawObject displayName]) on lines 492 and 493?

@pjrobertson
Copy link
Member Author

nameString and displayName should be treated separately. If we did as you're suggesting, then the string in the UI at this point would be:

nameString ⟷ nameString which is silly of course :)
You're right that we should probably check to make sure [drawObject displayName] != nil

@skurfer skurfer merged commit 686be5f into master Feb 21, 2013
skurfer added a commit that referenced this pull request Feb 21, 2013
@pjrobertson pjrobertson deleted the crashfx branch February 22, 2013 18:52
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.

None yet

3 participants