johnno1962 / GitDiff

Highlights deltas against git repo in Xcode
MIT License
891 stars 54 forks source link

Revert change #43

Closed julian-weinert closed 8 years ago

julian-weinert commented 9 years ago

When hovering the change indicator in the line number bar you get the original code displayed. It would be amazing to be able to double-click the indicator to revert the change.

Stay awesome – thanks!

sobri909 commented 9 years ago

This is one of AppCode's greatest features. And if this plugin supported this feature, I'd probably be able to happily ditch AppCode (which I would very much like to do).

So take this as a strong +1 from me!

johnno1962 commented 9 years ago

I’ve checked in an implementation of this with a basic interface. Hover the mouse over the line number of modified or deleted code and after a second a button will appear that can be used to revert the change or recover the code. Can you give it a pretty thourough testing for me please?

sobri909 commented 9 years ago

@johnno1962 Thanks! Really looking forward to this one.

First problem is related to colour scheme. On a dark colour scheme the button is very hard to see. I didn't notice at first, so thought I hadn't installed the right build.

Second problem might be Xcode 7 related. Tapping the button reverts the change incorrectly.

Original line

    [self.contentView sendSubviewToBack:self.highlightView];

Replacement line

    [self.contentView sendSubviewToFront:self.highlightView];

(this produces an appropriate Xcode error, because that method doesn't exist)

After reverting the change

Oh... This time it didn't do it. On previous attempts it was reverting the line to this (see the missing colon):

    [self.contentView sendSubviewToBackself.highlightView];

Hm. I'll give it some more testing and see if I can pin down the pattern.

johnno1962 commented 9 years ago

@sobri909 I’ve updated the Icon to black on white rather than transparent so it should be visible now and tidied things up a little. I didn’t find a bug though and it is strange that you had a problem mid line as the differences are all line based. It does seem to break the undo buffer however. See how you fare with the new version.

sobri909 commented 9 years ago

Yeah, that mid line thing is odd. I saw it repeatedly, but then I couldn't reproduce it again. Might as well pretend it didn't happen for now.

I'm seeing another problem:

  1. Make a code change that causes an error (eg change a var name to something that doesn't exist)
  2. Xcode adds red squigglies
  3. Save
  4. Revert the change with GitDiff
  5. Note that the red squigglies and gutter error icon are still present
  6. Save
  7. Note that the red squigglies and gutter error icon are still present

Basically Xcode doesn't notice the revert, even after saving the file. Though it should notice shortly after the revert, in normal cases. Yeah, that mid line thing is odd. I saw it repeatedly, but then I couldn't reproduce it again. Might as well pretend it didn't happen for now.

I'm seeing another problem:

  1. Make a code change that causes an error (eg change a var name to something that doesn't exist)
  2. Xcode adds red squigglies
  3. Save
  4. Revert the change with GitDiff
  5. Note that the red squigglies and gutter error icon are still present
  6. Save
  7. Note that the red squigglies and gutter error icon are still present

Basically Xcode doesn't notice the revert, even after saving the file. Though it should notice shortly after the revert, in normal cases.

johnno1962 commented 9 years ago

Learning on the job here… I’ve managed to wake up the undo manager properly on revert so while the syntax error indication doesn’t go away when you revert (until you do the next build) at least Xcode knows the file needs to be saved and undo is working now so there shouldn't be any spurious edits and if there are you can at least undo them.

julian-weinert commented 9 years ago

In the version currently provided by Alcatraz undoing does not work. Actually it does work somehow, but the code gets messed up totally:

// before change (HEAD)
[self dismissViewControllerAnimated:YES completion:nil];
[viewController presentViewController:selectionViewController animated:YES completion:nil];

// after change (^HEAD)
[self dismissViewControllerAnimated:YES completion:nil];
[viewController presentViewController:[selectionViewController navigationController] animated:YES completion:nil];

// after revert, before undo (HEAD)
[self dismissViewControllerAnimated:YES completion:nil];
[viewController presentViewController:selectionViewController animated:YES completion:nil];

// after undo (^HEAD)
[self dismissViewControllerAnimated:YES completion:nil];
[[viewController navigationControllerr:elf selectionViewController] n - UISearchControllerDelegate
//                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^

The marked UISearchControllerDelegate come from whoever knows I never toughed the class definition (where it must come from) since at least 4 commits and didn't change it, so it must be some kind of failed range or so...

johnno1962 commented 9 years ago

Can you build from the githib project and see if this happens? There was a recent change related to getting undo working.

johnno1962 commented 8 years ago

I’m assuming silence means it seems to work ok. Thanks for the inspiration, this change has been very popular.

sobri909 commented 8 years ago

Everything was working last I tried. Thanks for this one! Unfortunately I ran into a bunch of other missing Xcode features that sent me back to AppCode, so I didn't get a chance to test this more thoroughly.

julian-weinert commented 8 years ago

@johnno1962 sorry for not responding, I'm in a really tight schedule at the moment. Thanks, I confirm it's working now! Thanks.