On Aug 31, 2009, at 4:38 PM, Pieter de Bie wrote:
...
Now, about the patch: It looks fine, one thing one might argue
about is
the wording of the prefs and the placement, but that's something we
can
worry about when we get more preference-options.
Feel free to re-word the preferences. That was just the first thing
that popped into my head. I'm not fond of it either.
I can't say much about
the XIB, but it probably could be a smaller diff. But it applied
fine on
my bugfixes-branch, so no problem here.
One tiny technical detail I was wondering about was:
id curPath = [[[NSProcessInfo processInfo] environment]
objectForKey:@"PWD"];
This works fine, but shouldn't we use NSString* curPath = ... here?
Using a specific type makes it easier for a reviewer to see what
kind of
object is to be expected, and in this case it's clearly a string,
last
but not least because [NSURL fileURLWithPath:curPath] expects an
NSString* as parameter.
Other than that I have no objections to this patch from the
usability-standpoint, I even might come to use it myself ;)
I agree with Johannes' comment about NSString *, but I haven't looked
into the patch thoroughly yet. I'm still having trouble with my RSI
(:(), but I'll see if I have some time tomorrow to look at it.
- Pieter
I don't do a lot of Objective-C programming, and wasn't sure whether
it was a good idea to take an untyped item from a dictionary and
assign it to a typed pointer.
I agree that it should be typed as accurately as possible. I just
tried it as an NSString*, and it works fine.
Actually, I never use Objective-C, only Objective-C++. I'm unsure of
what's legal in the base language.
- Stoney
--
Stonewall Ballard
[email protected] http://stoney.sb.org/
