Choose between multiple remotes instead of showing an error #8922
Conversation
Move "Add remote" action close to "Clone" action
Chris2011
added
enhancement
git
|
I will do a correct squash and merge at the end. Unfortunately I merged master after my commit. Sry for that. |
|
I also added a null check, due to a NPE throwing. I see failing tests, I will fix them soon. |
|
Seemed that there are no failing tests, pipeline had a hickup probably. |
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
Sorry for the delay in reviewing this. The Change looks fine to me. I only eyeballed this, so I have to assume this was tested by you.
I left to inline comments, that are more questions to confirm my assumptions.
Before merging please rebase this onto master and retest. Intermediate merges from master make it unnecessary hard to revert if necessary (we learnt that the hard way). As rebasing is not clean and the changes are small, I think this might be a good alternative would be:
- rename your local branch of this PR
- checkout current master and recreate the branch
feature/git-multi-remote-solutionfrom that - apply the changes onto that
git diff a2a2958133b0d6ce07519419d56bf8fdbbfec201..dfc9ce9f6ff5 | git apply(a2a2958 is the last state of master you merged intofeature/git-multi-remote-solution, dfc9ce9 is the original head offeature/git-multi-remote-solution)
| } | ||
|
|
||
| public void fillRemoteBranches (final Map<String, GitBranch> branches, final GitBranch branchToSelect) { | ||
| fillRemoteBranchesInternal(Collections.<String,GitBranch>emptyMap(), Collections.<String,GitBranch>emptyMap(), null); |
There was a problem hiding this 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.
There was a problem hiding this comment.
I tested it yes (Maybe I missed a thing, as always. When you pull a branch without upstream, nothing is selected. When you do a pull from upstream, the error came up "no tracked brach specified for local branch....". And if I remove this, nothing is selected anymore on pull from upstrean. It can be, that I need to modify the line 169 - 173.
There was a problem hiding this comment.
Sorry, I don't get it. line 169-173 can't be reached for the marked case. I mean this call:
fillRemoteBranchesInternal(Collections.<String,GitBranch>emptyMap(), Collections.<String,GitBranch>emptyMap(), null);This will basicly execute (the maps are empty, so all loops in fillRemoteBranchesInternal will not execute):
List<BranchMapping> l = new ArrayList<BranchMapping>(0);
Set<String> displayedBranches = new HashSet<String>(0);
this.branches.setBranches(l);
stateChanged(new ChangeEvent(this));I read this as "reset the item selector into a clean state".
There was a problem hiding this comment.
Yes, at the end, it is just to make everything set to the default. The null value. If the branchToSelect is set, line 175 is reached. Everything else, is for the default behaviour.
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
Am I correct in assuming, that this not called on the EDT?
There was a problem hiding this comment.
If this is the hint then thx for this. It never gets into the finally block, beause of another thread is cancelling this. The magic happens in GitUtils.java:886.
There was a problem hiding this comment.
runWithoutIndexing can't have part in this. It is inside the try-block, so the finally still must be executed (try-block is line 170-236, finally is lines 240-256). I'm not aware of ways to skip execution of a finally block.
There was a problem hiding this comment.
It could be, that runWithoutindexing is not part here, but I set breakpoints and see, that only the finally block inside of the runWithoutIndexing is executed, after this , no other finally block is reached. So the inner finally block is reached, the outer not. This is why I'm confused. As said, my assumption is something with different threads?
|
Just to clarify: my comments were not meant to block. When rebased, I think this is fine to merge. |
|
@matthiasblaesing I really appreciate your help, so no worries :). Thx. |
2 participants
If you have add more than one remote and you want to push an untracked branch, NetBeans just shows an error that it can't choose the correct remote due to many remotes.
I added this for push to upstream and pull from upstream. If we have multiple remotes, it just opens the same UI for pull/push and preselect the current branch.
For push it will also ask for setting this branch tracked to the remote branch. I have a problem for doing the same for pull. I guess it is a thread thing.
Inside of PullAction in the method perform, it never hits the finally block, due to removing the threadlocal variable inside the runWithoutInding method. I dunno whether this is a bug but for me it feels like that. So I couldn't test it.
The PR is not finished yet, but you already can have a look and left comments please.
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)