matthiasblaesing commented on code in PR #8922:
URL: https://github.com/apache/netbeans/pull/8922#discussion_r2641261471


##########
ide/git/src/org/netbeans/modules/git/ui/fetch/PullBranchesStep.java:
##########
@@ -129,7 +129,11 @@ public void setRemote (GitRemoteConfig remote) {
     }
 
     public void fillRemoteBranches (final Map<String, GitBranch> branches) {
-        fillRemoteBranches(Collections.<String,GitBranch>emptyMap(), 
Collections.<String,GitBranch>emptyMap());
+        fillRemoteBranches(branches, null);
+    }
+
+    public void fillRemoteBranches (final Map<String, GitBranch> branches, 
final GitBranch branchToSelect) {
+        fillRemoteBranchesInternal(Collections.<String,GitBranch>emptyMap(), 
Collections.<String,GitBranch>emptyMap(), null);

Review Comment:
   Is the intention here to start with an empty list in any case and then 
update when loading is done? That might be worth a comment.
   
   I was wondering why `null` is passed here as `branchToSelect`. That makes 
only sense to me, if we just want to initialize.



##########
ide/git/src/org/netbeans/modules/git/ui/fetch/PullAction.java:
##########
@@ -182,55 +189,71 @@ protected void perform () {
                         return;
                     }
                 }
-                GitUtils.runWithoutIndexing(new Callable<Void>() {
-                    @Override
-                    public Void call () throws Exception {
-                        for (String branch : toDelete) {
-                            client.deleteBranch(branch, true, 
getProgressMonitor());
-                            
getLogger().outputLine(Bundle.MSG_PullAction_branchDeleted(branch));
-                        }
-                        setDisplayName(Bundle.MSG_PullAction_fetching());
-                        Map<String, GitTransportUpdate> fetchResult = 
FetchAction.fetchRepeatedly(
-                                client, getProgressMonitor(), target, 
fetchRefSpecs);
-                        if (isCanceled()) {
-                            return null;
+                GitUtils.runWithoutIndexing(() -> {
+                    for (String branch : toDelete) {
+                        client.deleteBranch(branch, true, 
getProgressMonitor());
+                        
getLogger().outputLine(Bundle.MSG_PullAction_branchDeleted(branch));
+                    }
+                    setDisplayName(Bundle.MSG_PullAction_fetching());
+                    Map<String, GitTransportUpdate> fetchResult = 
FetchAction.fetchRepeatedly(
+                        client, getProgressMonitor(), target, fetchRefSpecs);
+                    if (isCanceled()) {
+                        return null;
+                    }
+                    FetchUtils.log(repository, fetchResult, getLogger());
+                    if (!isCanceled()) {
+                        
setDisplayName(Bundle.MSG_PullAction_progress_syncBranches());
+                        FetchUtils.syncTrackingBranches(repository, 
fetchResult, GitProgressSupportImpl.this, 
GitProgressSupportImpl.this.getProgress(), false);
+                    }
+                    if (isCanceled() || branchToMerge == null) {
+                        return null;
+                    }
+                    new BranchSynchronizer(branchToMerge, repository, new 
BranchSynchronizer.GitProgressSupportDelegate() {
+                        
+                        @Override
+                        public GitClient getClient () throws GitException {
+                            return client;
                         }
-                        FetchUtils.log(repository, fetchResult, getLogger());
-                        if (!isCanceled()) {
-                            
setDisplayName(Bundle.MSG_PullAction_progress_syncBranches());
-                            FetchUtils.syncTrackingBranches(repository, 
fetchResult, GitProgressSupportImpl.this, 
GitProgressSupportImpl.this.getProgress(), false);
+                        
+                        @Override
+                        public OutputLogger getLogger () {
+                            return GitProgressSupportImpl.this.getLogger();
                         }
-                        if (isCanceled() || branchToMerge == null) {
-                            return null;
+                        
+                        @Override
+                        public ProgressDelegate getProgress () {
+                            return GitProgressSupportImpl.this.getProgress();
                         }
-                        new BranchSynchronizer(branchToMerge, repository, new 
BranchSynchronizer.GitProgressSupportDelegate() {
-
-                            @Override
-                            public GitClient getClient () throws GitException {
-                                return client;
-                            }
-
-                            @Override
-                            public OutputLogger getLogger () {
-                                return GitProgressSupportImpl.this.getLogger();
-                            }
-                            
-                            @Override
-                            public ProgressDelegate getProgress () {
-                                return 
GitProgressSupportImpl.this.getProgress();
-                            }
-                            
-                        }).execute();
-                        return null;
+                        
+                    }).execute();
+                    
+                    if (!isCanceled()) {
+                        pullSuccessful = true;
                     }
-                }, repository);
+                    
+                    return null;
+                }, repository);                
             } catch (GitException ex) {
                 setError(true);
                 GitClientExceptionHandler.notifyException(ex, true);
             } finally {
                 setDisplayName(NbBundle.getMessage(GitAction.class, 
"LBL_Progress.RefreshingStatuses")); //NOI18N
                 
Git.getInstance().getFileStatusCache().refreshAllRoots(Collections.<File, 
Collection<File>>singletonMap(repository, 
Git.getInstance().getSeenRoots(repository)));
                 GitUtils.headChanged(repository);
+
+                // Check if tracking should be set up after successful pull
+                if (pullSuccessful && branchToMerge != null) {
+                    RepositoryInfo info = 
RepositoryInfo.getInstance(repository);
+                    info.refresh();
+                    GitBranch activeBranch = info.getActiveBranch();
+                    if (activeBranch != null && 
activeBranch.getTrackedBranch() == null) {
+                        // branchToMerge is the remote branch name (e.g., 
"origin/master")
+                        if (shallSetupTracking(activeBranch, branchToMerge)) {
+                            
SystemAction.get(SetTrackingAction.class).setupTrackedBranchImmediately(
+                                    repository, activeBranch.getName(), 
branchToMerge);
+                        }
+                    }

Review Comment:
   Am I correct in assuming, that this not called on the EDT?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to