Limit HEAD parents when inserting artificial in grid #11266
Conversation
HEAD may not be included in git-log in some scenarios. One example is an old checkout in the Linux repo where 100K commits is about a year. Another is when filtering. In this scenario there is no HEAD the artificial commits cannot be inserted while loading. The HEAD parents are then listed to find the first descendant in the grid, or first in the grid if no head parent is found. Similar scenario for the revision to be reselected. When switching repo the HEAD is always selected. Previously, a match was tried in all HEAD parents. This command requires 1.5 s in the linux repo, delaying the the time until the spinner is dismissed. The number of parents to try is limited to around 50 ms instead. In addition, the command ran a second time as the result was not enumerated when trying to find the revision to select.
| } | ||
|
|
||
| // If parents are rewritten HEAD may not be included | ||
| // Insert the artificial commits where relevant if possible, otherwise first | ||
| refresh = AddArtificialRevisions(headParents); | ||
| refresh = AddArtificialRevisions(headParents ?? new List<ObjectId>()); |
There was a problem hiding this comment.
| refresh = AddArtificialRevisions(headParents ?? new List<ObjectId>()); | |
| refresh = AddArtificialRevisions(headParents ?? Array.Empty<ObjectId>()); |
Why do it, if AddArtificialRevisions accepts null?
There was a problem hiding this comment.
Updating the comment
// HEAD where to insert the artificial was not found
// (can occur when filtering or limiting number of loaded commits)
// Try to find the revision where to attach the artificial, first is not found
// null means that no matching should be done (just add), so use an empty list if no parents were found
There was a problem hiding this comment.
Astonishingly, Array.Empty<ObjectId>() works without .ToList().
There was a problem hiding this comment.
Will not compile
Severity Code Description Project File Line Suppression State Details
Error CS0426 The type name 'Empty<>' does not exist in the type 'Array' GitUI C:\dev\gc\gitextensions\GitUI\UserControls\RevisionGrid\RevisionGridControl.cs 1385 Active
Points to System.Array as in other usages, but the disassembly has no Empty....
The only reason to change this is to not get someone try to change this later
There was a problem hiding this comment.
Will not compile
It does and seems to work well.
There was a problem hiding this comment.
Maybe something with vs 17.8.0 preview 3?
What I get in RevisionGridControl
Other Array
There was a problem hiding this comment.
Today ite compiles, no changes what I am aware of...
Pushed the changes.
I would like to merge this, so the follow ups can be changed from drafts.
|
The signature is IReadOnlyList, which is an essence is an Array, since it
can't be resized. So, Array.Empty works.
|
The problem was that it was not possible to compile.... |
Proposed changes
HEAD may not be included in git-log in some scenarios. One example is an old checkout in the Linux repo where 100K commits is about a year. Another is when filtering.
In this scenario there is no HEAD the artificial commits cannot be inserted while loading.
The HEAD parents are then listed to find the first descendant in the grid, or first in the grid if no head parent is found.
Similar scenario for the revision to be reselected. When switching repo the HEAD is always selected.
Previously, a match was tried in all HEAD parents. This command requires 1.5 s in the linux repo,
delaying the the time until the spinner is dismissed. The number of parents to try is limited to around 50 ms instead.
In addition, the command ran a second time as the
result was not enumerated when trying to find the revision to select.
--
This is not a normal user problem, this is a "cloning Linux a year ago and not checking out new branches" problem.
Still annoying when investigating other improvements.
Note that Artificial are not inserted correctly after this, likely due to a timing problem.
Fixed in later PR.
Test methodology
Manual
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.