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

Debug compiling #334

Merged
merged 9 commits into from Jun 14, 2011
Merged

Debug compiling #334

merged 9 commits into from Jun 14, 2011

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented May 27, 2011

See http://groups.google.com/group/quicksilver---development/browse_thread/thread/b9fa430792166170 for a full discussion.

I've basically made any of the 'debug' code only be compiled if XCode is set to build a debug target.
The old way in which this worked was:
The code for the debug and release target were the same. QS would then, at runtime, decide whether to spit out the debug code (mostly NSLog's) depending on a flag/environment setting.

What I've done here is made the compiler decide if the debug code should be compiled or not - depending on the target.
The idea is that in a release build of QS, there will be no checks for if(DEBUG), hopefully speeding it up -- somewhat.

Tests that should be done before this is merged:
Read over my commits (sorry, they're pretty messy). They're mostly just changing if(DEBUG) to #ifdef DEBUG -- this is set in the target's build settings (see it in XCode by going 'Project' -> 'Edit project settings'
Test the debug build (you can get full verbose mode by holding ⌥ when QS is launching. Pretty neat.
Test the release build. File size should be (slightly) smaller than the current HEAD's release build, as there's less code in it

@@ -22,9 +22,9 @@ BOOL QSApplicationCompletedLaunch = NO;

@implementation QSApp
+(void)load {
if (DEBUG)
#ifdef DEBUG
setenv("verbose", "1", YES);
Copy link
Member

@skurfer skurfer May 27, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line still needed? It looks like previously, you could get verbose mode by either the environment variable or by holding down Option. Now it looks like the Option key trick should only be available in Debug builds, which is fine, but in that case, verbose will get set on line 29.

Copy link
Member Author

@pjrobertson pjrobertson May 27, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's similar to how it used to be, but I agree - it is a bit funny.
If you're not holding down the ⌥ key then the verbose env gets set then removed. I guess lines 26 and 33 - 35 could be removed.

P.S. did you delete a line comment about #ifndef? That's supposed to be #ifndef since it's replacing an if(!DEBUG) - i.e. only include the code if we're doing a release build

Copy link
Member

@skurfer skurfer May 27, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you delete a line comment about #ifndef?

Yes. Just my ignorance of C syntax. Ignore. :)

@skurfer
Copy link
Member

skurfer commented May 27, 2011

OK, I’ve looked it over with my eyeballs and all looks good except the one comment. I’ll do some testing over the next couple of days. For the Option trick, does that only work when launching from Finder or can you hold ⌥ during a “Build & Debug” too?

@pjrobertson
Copy link
Member Author

pjrobertson commented May 27, 2011

Nice work on checking it all over. Exactly what was needed :)

does that only work when launching from Finder or can you hold ⌥ during a
“Build & Debug” too?

Works whenever you launch QS - so from XCode as well.
If you're going to run the verbose mode, I'll just point out that there's a
bug in the gdbformatter Andre Berg (@andreberg) wrote - line 310 of
QSObject.m that seems to make QS crash when you have a search URL in the 1st
pane.
I looked into it a little, but couldn't figure it out - I think it was to do
with the icon.

I'll change that thing you mentioned tomorrow.

On 27 May 2011 23:48, skurfer <
reply@reply.github.com>wrote:

OK, I’ve looked it over with my eyeballs and all looks good except the one
comment. I’ll do some testing over the next couple of days. For the Option
trick, does that only work when launching from Finder or can you hold ⌥
during a “Build & Debug” too?

Reply to this email directly or view it on GitHub:
#334 (comment)

@pjrobertson
Copy link
Member Author

pjrobertson commented May 28, 2011

Made the change you suggested

@pjrobertson
Copy link
Member Author

pjrobertson commented May 29, 2011

I noticed some methods are debug methods - called from the debug menu.
I added an #ifdef DEBUG so they're not included in a release dist.

@skurfer
Copy link
Member

skurfer commented Jun 14, 2011

Why can’t this be merged automatically any more? I get no conflicts when I do it locally. Does it conflict with another pull request? I was going to merge it.

@pjrobertson
Copy link
Member Author

pjrobertson commented Jun 14, 2011

Strange. I don't know... Probably just GitHub's auto merge thing being a bit buggy. If you get no merge conflicts using git, I'd just go ahead and use that :)

@skurfer skurfer merged commit a034ba6 into quicksilver:master Jun 14, 2011
@skurfer
Copy link
Member

skurfer commented Jun 14, 2011

I’ve been using this for weeks with no issues (and I want it merged so I can work on something).

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

2 participants