gitx icon indicating copy to clipboard operation
gitx copied to clipboard

Git history view does not remember selected commit if put in background

Open bpsm opened this issue 13 years ago • 9 comments

Maybe related to #91.

As far as I can tell, PBGitHistoryController:

  1. refreshes the history view whenever GitX gains focus
  2. tries to restore the selected commit when doing this, but fails.

I poked around in the debugger and found:

- (NSArray *) selectedObjectsForSHA:(NSString *)commitSHA
{
    NSPredicate *selection = [NSPredicate predicateWithFormat:@"sha == %@", commitSHA];
    NSArray *selectedCommits = [[commitController content] filteredArrayUsingPredicate:selection];

    if (([selectedCommits count] == 0) && [self firstCommit])
            selectedCommits = [NSArray arrayWithObject:[self firstCommit]];

    return selectedCommits;
}

We come in with commitSHA equal to some commit e.g. "c2ec9ea..." which exists in the repository and is, in fact, the commit that was selected when the gitx window lost focus.

The NSPredicate looks plausible (I'm a total Cocoa nooob), but [commitController content] is delivering an empty NSArray, so selectedCommits ends up as an empty NSArray. I have no theories on why that could be.

The result of all this is that the list of selected commits comes back empty and PBGitHistoryController improvises by selecting the HEAD of the current branch instead of restoring the last selected commit.

This is absolutely maddening as I've grown accustomed to using gitx to do code reviews, which means a lot of command-tabbing back and forth between gitx and the IDE/Editor.

bpsm avatar Jul 26 '11 11:07 bpsm

I too find this issue very frustrating. Quite a few times now I've done extensive searching and browsing in the history view only to have everything reset as soon as I activate my browser or editor.

I'll try to take what bpsm has found to see if I can get any further, but it'd be awesome if laullon knew about a quick fix for this problem.

netnichols avatar Aug 23 '11 11:08 netnichols

From a brief inspection I couldn't figure out what is going wrong in selectedObjectsForSHA:, but I did trace the cause of the refresh back to [PBGitWindowController windowDidBecomeKey] which triggers the refresh if "Refresh Automatically" is checked in the preferences.

So, the easy workaround is to uncheck "Refresh Automatically". But it seems the proper way to fix this would be to only actually refresh the commit history if the repository or working directory has changed since the window resigned it's key status.

netnichols avatar Aug 26 '11 11:08 netnichols

Thanks for the workaround. I agree -- this is awful!

n8gray avatar Sep 12 '11 19:09 n8gray

I would argue that the expected behavior is that the previously-selected commit would remain selected even if additional commits have been added to the repository. If you've got HEAD selected, ⌘-tab to Terminal and commit another change, then ⌘-tab back to Gitx(L), the commit that was HEAD is now HEAD@{1} and should still be selected while the new HEAD should appear above (or below, I guess, depending on sorting) in the commit list. I believe this is how the original GitX works…

I believe this is a duplicate of both #91 and #87.

blalor avatar Sep 13 '11 01:09 blalor

As bpsm writes selectedObjectsForSHA is the likely culprit. Or at least partiallly.

I might be barking up the wrong tree since I'm not familiar enough with the code to tell. I don't know if the following is intended behavior or if the bug lurks in here somewhere.

When switching to GitX, with autorefresh turned on, selectedObjectsForSHA is called at least twice.

The first time [commitController content] is empty, which automatically selects the HEAD (which is, we all agree, terrible). The subsequent times the content array is populated with the commits from the history.

These invocations seems to be the result of key/value observing. The first time the cause is [PBGitHistoryList updateProjectHistoryForRev:].

When looking at [PBGitHistoryList updateProjectHistoryForRev:] it seems to me that the content array is set to a new empty array in order to refresh the history of the commit. This in turn causes the head to be selected.


Hopefully this bug gets killed, and soon.

asheidan avatar Sep 15 '11 13:09 asheidan

+1 This is my one major complaint.

zippy avatar Nov 14 '11 14:11 zippy

+1 I've stopped using GitX(L) because of this, and switched back to 0.7.4

knowtheory avatar May 25 '12 00:05 knowtheory

The workaround (disabling auto-refresh in preferences) works for me. I don't think 0.7.4 had auto-refresh anyway, so nothing really lost from my perspective; although would be great if this bug was fixed.

dissolved avatar Jun 04 '12 04:06 dissolved

This was fixed in 3e045a4

jphalip avatar Jun 06 '12 22:06 jphalip