Fallback to canonical path for unstaged images

[Stengo]

Fixes #788

This rolls back to the original behavior of falling back to the canonical path for unstaged image files that do not have a SHA1 value yet.

I AGREE TO THE GITUP CONTRIBUTOR LICENSE AGREEMENT

[lucasderraugh]

Hmm, not sure why I would have changed that logic, I feel like there was another reason I had in mind, perhaps it was the deletion case where I was trying to only see the old image and not have the split view.

I'll reconfirm why I made that change, but I agree I did break the unstaged case. I'll make a quick turn around push of 1.3.1 soon.

[lucasderraugh]

Could we replace the updateCurrentImage call with the following:

- (void)updateCurrentImage {
  NSError* error;
  NSString* newPath;
  if (_delta.newFile.SHA1 != nil) {
    newPath = [GILaunchServicesLocator.diffTemporaryDirectoryPath stringByAppendingPathComponent:_delta.newFile.SHA1];
    NSString* newExtension = _delta.newFile.path.pathExtension;
    if (newExtension.length) {
      newPath = [newPath stringByAppendingPathExtension:newExtension];
    }
    if (![[NSFileManager defaultManager] fileExistsAtPath:newPath]) {
      [self.repository exportBlobWithSHA1:_delta.newFile.SHA1 toPath:newPath error:&error];
    }
  } else if (_delta.canonicalPath && _delta.oldFile.SHA1 == nil) {
    newPath = [self.repository absolutePathForFile:_delta.canonicalPath];
  }
  if (newPath) {
    _currentImageSize = [self imageSizeWithoutLoadingFromPath:newPath];
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
      NSImage* limitedSizeImage = [self generateLimitedSizeImageFromPath:newPath];
      dispatch_async(dispatch_get_main_queue(), ^{
        _currentImageView.image = limitedSizeImage;
        [self setNeedsDisplay:true];
      });
    });
  } else {
    _currentImageView.image = nil;
  }
}

This way we'll show unstaged images and the deleted assets won't show with the diff view (we only want to see the full asset that is being deleted).

[Stengo]

I just tried out your code and I'm still seeing the deleted asset in the diff view, the behavior appears identical to the code in this PR 🤔
But I might be misunderstanding the exact scenario 😅

Oh also: feel free to ignore this PR and solve the issue directly if that's faster or more convenient 🙂 I don't want to hold back the release.

[lucasderraugh]

Sorry, that was an incorrect explanation. The only difference in the code paths would be that in your code we would be generating 2 images in the deleted case (the old and "new" which is the same asset). My code suggestion would be skipping over creating an image for the current image when we only need to be drawing the old image.

I'll push my changes to master and get a build for continuous later tonight. Thank you for bringing this to my attention and I'm sorry I broke your excellent work.

[lucasderraugh]

Going to close this PR out as I've applied the fix directly to master. Thanks again.