======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h?a=5 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.h (snapshot 5) ------------------------------------ Line 26: #ifndef GEARS_DESKTOP_DRAG_AND_DROP_UTILS_OSX_H__ On Tue Nov 11 19:58:28 2008 PST, dsymonds wrote: > Header guards shouldn't use double underscores at the end; I believe those are > reserved for compiler usage nowadays. Change this to a single underscore.
All of Gears uses the double-underscore, so I'll leave it as is. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=7 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm (snapshot 7) ------------------------------------ Line 39: static bool g_is_in_a_drag_operation_ = false; On Tue Nov 11 20:02:43 2008 PST, dsymonds wrote: > This doesn't seem particularly thread-safe; can you use NSLock or similar? This code always runs in the (sole) UI thread. ------------------------------------ Line 69: NSStringFromSelector(old_selector), On Tue Nov 11 19:59:09 2008 PST, dsymonds wrote: > You might as well indent these three lines two more spaces so they line up with > the inside of the opening parenthesis. Done. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=5 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm (snapshot 5) ------------------------------------ Line 74: result ? @"succeeded" : @"failed"); On Tue Nov 11 15:26:10 2008 PST, nigeltao wrote: > On Tue Nov 11 09:03:29 2008 PST, playmobil wrote: > > This should be a LOG(), you can NSLog on failure, but it's fine by me if you > > just NSLog() in the calling method and LOG() here. > > As previously mentioned, LOG eventually calls NSLog, so unless I'm confused, > there's no practical difference to what you're suggesting. On second thoughts, I've simply changed this to be guarded by an #if DEBUG, to avoid the monkeywork of converting NSStrings to and from std::string16's, between NSLog and LOG. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=7 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm (snapshot 7) ------------------------------------ Line 77: - (NSDragOperation)swizzledDraggingEntered:(id <NSDraggingInfo>) draggingInfo; On Tue Nov 11 20:00:56 2008 PST, dsymonds wrote: > Ditch the spaces between "(id <NSDraggingInfo>)" and "draggingInfo", as well as > the other instances below. Done. ------------------------------------ Line 83: @implementation WebView (GearsSwizzledMethods) On Tue Nov 11 20:01:34 2008 PST, dsymonds wrote: > Add a blank line after @implementation. Done. ------------------------------------ Line 119: @end On Tue Nov 11 20:01:47 2008 PST, dsymonds wrote: > Add a blank line before @end. Done. ======================================================================== http://mondrian.corp.google.com/file/8931781///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm?a=5 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_osx.mm (snapshot 5) ------------------------------------ Line 124: return On Tue Nov 11 15:27:07 2008 PST, nigeltao wrote: > On Tue Nov 11 09:12:56 2008 PST, playmobil wrote: > > nit: may be a little cleaner to stick these in a vector and iterate it. > > Eh, maybe, but its marginal. Well, I've gone ahead and done this, and it probably looks a little better. ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/8931781
