square / spacecommander

Commit fully-formatted Objective-C as a team without even trying.
Other
1.13k stars 177 forks source link

Fix #42, where a trailing parameter is indented on a newline after a … #44

Closed alanf closed 8 years ago

alanf commented 8 years ago

…block literal.

@msanders this is based on the fix for #41, let me know how that looks

msanders commented 8 years ago

Thanks! This fixes all of the problems surrounding this in our codebase except for this edge case:

- (void)fetchWithSuccess:(nullable dispatch_block_t)success failure:(nullable INSHTTPFailure)failure
{
    [self GET:@"data" parameters:nil success:INSAPIClientModelArraySuccessChain([INSModel class], nil, ^(INSModel *model, id responseObject) {
        if (success) {
            success();
        }
    }, failure) failure:failure];
}

Which becomes:

- (void)fetchWithSuccess:(nullable dispatch_block_t)success failure:(nullable INSHTTPFailure)failure
{
    [self GET:@"data" parameters:nil success:INSAPIClientModelArraySuccessChain([INSModel class], nil, ^(INSModel *model, id responseObject) {
        if (success) {
            success();
        }
    },
                                                                                failure)
           failure:failure];
}

Here's another diff.

alanf commented 8 years ago

Thanks for the additional example, I expanded the script to match what you showed me there as well. I'm cautiously trying to minimize what will positively match to prevent unknown side effects, so please let me know if there are more instances that we should cover @msanders

alanf commented 8 years ago

@justinseanmartin can you take a look at this one too please

justinseanmartin commented 8 years ago

I think there may be a case for explicitly wanting params to be on the next line if you have a lot of parameters, but I think this works for now. I think we need an actual example of something that doesn't work in order to make further improvements.

Out of curiosity, how much time does it take per added python script? I'm assuming it is a trivial amount, but worth noting, because in general the linter needs to be super fast.

justinseanmartin commented 8 years ago

LGTM

msanders commented 8 years ago

Found one more edge case:

- (void)postWithSuccess:(nullable INSHTTPSuccess)success failure:(nullable INSHTTPFailure)failure
{
    id imageData = nil;
    [self POST:@"endpoint" parameters:nil constructingBodyWithBlock:^(id<AFMultipartFormData> formData) {
        if (imageData) {
            [formData appendPartWithFileData:imageData name:@"image" fileName:@"photo.jpg" mimeType:@"image/jpeg"];
        }
    } success:INSAPIClientEmptySuccessHandler(success) failure:failure];
}

Becomes:

- (void)postWithSuccess:(nullable INSHTTPSuccess)success failure:(nullable INSHTTPFailure)failure
{
    id imageData = nil;
    [self POST:@"endpoint" parameters:nil constructingBodyWithBlock:^(id<AFMultipartFormData> formData) {
        if (imageData) {
            [formData appendPartWithFileData:imageData name:@"image" fileName:@"photo.jpg" mimeType:@"image/jpeg"];
        }
    } success:INSAPIClientEmptySuccessHandler(success)
                          failure:failure];
}

Here's the diff.

Thanks again for looking into this so quickly.

alanf commented 8 years ago

@msanders want to take this for another spin? I have a potential fix for the latest edge case you pointed out, let me know if anything unexpected happens please

orta commented 8 years ago

This PR is awesome.

msanders commented 8 years ago

@alanf That seems to catch everything for us. Thanks!!

alanf commented 8 years ago

Thanks @msanders @justinseanmartin and @orta !