Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RevGrid: Lazy handling for current branch #9885

Merged

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Mar 5, 2022

Followup to #9864

Proposed changes

Avoid Git calls where no branch is checked out and in submodules
This is similar to how HEAD is cached in CurrentCheckout.

For repos where a Git branch is checked out, this is handled by reading files, so some IO is needed.
This adds some ms here and there, nothing critical (but affecting performance a lot more than most programming constructs).

For submodules where normally no branch is checked out, this requires Git commands, both when loading revisions and opening menues etc.

Screenshots

No UI changed, just some commands (only startup shown).

Before

image

After

image

Test methodology

manual
Removed an incorrect test and adopted another.
(A branch name like (test) is actually legal and should be displayed as ((test)) in the title.

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


✒️ I contribute this code under The Developer Certificate of Origin.

Avoid Git calls where no branch is checked out and in submodules
@ghost ghost assigned gerhardol Mar 5, 2022
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines -2619 to +2623
var headBranchName = Module.RevParse(headBranch);

if (headBranchName is not null)
{
UICommands.ShowFormDiff(baseCommit.ObjectId, headBranchName, baseCommit.Subject, headBranch);
}
UICommands.ShowFormDiff(baseCommit.ObjectId, CurrentCheckout, baseCommit.Subject, CurrentBranch.Value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be correct. But I could not test this with detached head because CompareWithCurrentBranch is disabled then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference in behavior is that return is early, with a popup. (That should not occur as you mention).

@gerhardol gerhardol merged commit 89eba84 into gitextensions:master Mar 9, 2022
@ghost ghost added this to the vNext milestone Mar 9, 2022
@gerhardol gerhardol deleted the feature/p9864-lazy-current-branch branch March 9, 2022 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants