Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

CPString stringByDeletingLastPathComponent implementation fix.

#126

stringByDeletingLastPathComponent was ignored from docs because of a bug that gave the input of /a/a/ the output of /a/a/.

Implementation was fixed, and A small summary of the function was added to the comment instead of @ignore.

Old:

return substr(0, lastIndexOf('/') + 1);

New:

return substr(0, lastIndexOf('/', length - 2) + 1);

Sorry for the badly named file...

Reported by Sam McDonald · October 31st, 2008 @ 09:36 PM

State: resolved
Milestone: 0.6
Assigned to: Francisco Tolmasky Francisco Tolmasky

Activity

  1. Ross Boucher
    Ross Boucher

    This fix is definitely appreciated. Unfortunately, there is a problem with the patch you've attached.

    By default, git will just diff the last commit with your working copy. Since you've made your change in what seems to be 2+ commits, we've only gotten the last one.

    Of course, I could copy out the implementation myself, but we also don't have the documentation you wrote at all.

    The solution to this is to pass a branch name (or commit ID, or tag...) to git diff. For example, on my system, I have a branch "master" that tracks the origin. If I have my commits in another branch that is currently checked out, I can type:

    git diff master

    to get a patch that will contain everything new to the branch. If you can post a new patch, it would be very helpful. Hopefully this knowledge will come in handy in the future as well.

    November 1st, 2008 @ 09:55 PM

  2. Sam McDonald
    Sam McDonald
    • Tag changed from patch to cpstring, patch

    Here is a diff using the method you described. Please let me know if doesn't work. This diff contains test cases

    Anyway, I ran into some additional trouble. Basically I changed stringByDeletingLastPathComponent to work like NSString's version, but current code was wanting it to work differently. Basically they disagreed on whether a trailing '/' should exist.

    CPBundle's bundlePath method used the method in question to return the bundle path. The new version of stringByDeletingLastPathComponent would not add a trailing '/', but the old version did. I then decided to check Apple's bundlePath method and noticed that it did not have a trailing '/', so I decided to let the happen and changed resourcePath to not have a trailing '/'. This makes CPBundle work much more like NSBundle.

    However, bundlePath is used by NSImage's pathForResource to get an image. I made a change there to add the trailing '/'.

    NSImage's pathForResource was the only current dependency of bundlePath, and since I changed the output of pathForResource to be exactly like the older version, there shouldn't be any issued with this diff.

    The end result here is a fix in CPString, and more Cocoa-like behavior for CPBundle. But if you don't want to go in this direction just let me know and I will implement it differently. Also, let me know if you have any problems with my implementation of stringByDeletingLastPathComponent

    November 4th, 2008 @ 11:05 AM

  3. Sam McDonald
    Sam McDonald

    Notes From IRC: - Remove Curlies - Use == instead of ===

    November 5th, 2008 @ 05:37 PM

  4. Sam McDonald
    Sam McDonald

    Another note from IRC, use charAt()

    November 5th, 2008 @ 05:39 PM

  5. Sam McDonald
    Sam McDonald

    Here is a new diff with the changes we talked about in IRC.

    November 8th, 2008 @ 10:38 AM

  6. Sam McDonald
    Sam McDonald

    Update to fix the unneeded copy of self.

    November 8th, 2008 @ 10:52 AM

  7. Francisco Tolmasky
    Francisco Tolmasky
    • Milestone set to 0.6
    • State changed from new to resolved
    • Assigned user set to Francisco Tolmasky

    November 8th, 2008 @ 12:14 PM

  8. admin (at 280north)
    admin (at 280north)

    (from [9f9cc4c22c7c0e7b8fe90071c93de41765f6969c]) Fix for stringByDeletingLastPathComponent.

    [#126 CPString stringByDeletingLastPathComponent implementation fix. state:resolved]

    Reviewed by tolmasky http://github.com/280north/cappu...

    November 8th, 2008 @ 01:13 PM

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.