Replace AppSettings.UseFastChecks with ShowGitStatusInBrowseToolbar #9791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the specific setting for the index watcher, use the GitStatusMonitor settings instead. UseFastChecks hardly adds any overhead, it adds FileSystemWatcher to index and refs (GitStatusMonitor has a separate watcher on index). If the file system is changed, the top-left refresh button becomes red. (As well as avoiding refreshing the grid in some situations, may not be working though.) UseFastChecks was visible in Performance settings and expected by users (and occasionally maintainers) to make a difference. Sometimes the setting is mixed up with the GitStatusMonitor setting that can make a difference (as git-status requires a lot of resources, not FileSystemWatcher).
ghost
assigned 
RussKie
approved these changes
Jan 4, 2022
|
Squash merge in a day |
gerhardol
added a commit
to gerhardol/gitextensions
that referenced
this pull request
Feb 17, 2022
The IndexWatcher controls the icon for the refresh button in Browse. Before gitextensions#9791 the setting could be controlled individually, now the same setting as for GitStatus (commit count). Previously it also tried to optimize grid refresh operations in a few situations, skipping when no changes was detected to the index or any refs. These checks are now removed, always refreshing the grid. The check for changes seem to miss updates occasionally (and never in WSL where FileSystemWatcher do not reeport updates). This seem to be improved by matching on creation of index.lock in addition to index, but then also reset of worktree files are trcked as index changes. Before the file watcher was initiated at creation of the control and when a new revision was loaded. This requires a sync call to git-rev-parse --git-common-dir delaying the revision loading 50-100 ms. This is changed to just clear the icon when loading a revision (as well as when a new repo is loaded even if that should not be required) and resetting the file system watcher after all revisions was loaded. The IndexWatcher was reset several additional times after the grid was refreshed. The IndexWatcher.IndexChanged was used to possibly refresh the revisions that was already refreshed when the revisions were selected in the grid. (Also, no changes should have occurred this time.)
gerhardol
added a commit
that referenced
this pull request
Feb 21, 2022
The IndexWatcher controls the icon for the refresh button in Browse. Before #9791 the setting could be controlled individually, now the same setting as for GitStatus (commit count). Previously it also tried to optimize grid refresh operations in a few situations, skipping when no changes was detected to the index or any refs. These checks are now removed, always refreshing the grid. The check for changes seem to miss updates occasionally (and never in WSL where FileSystemWatcher do not reeport updates). This seem to be improved by matching on creation of index.lock in addition to index, but then also reset of worktree files are trcked as index changes. Before the file watcher was initiated at creation of the control and when a new revision was loaded. This requires a sync call to git-rev-parse --git-common-dir delaying the revision loading 50-100 ms. This is changed to just clear the icon when loading a revision (as well as when a new repo is loaded even if that should not be required) and resetting the file system watcher after all revisions was loaded. The IndexWatcher was reset several additional times after the grid was refreshed. The IndexWatcher.IndexChanged was used to possibly refresh the revisions that was already refreshed when the revisions were selected in the grid. (Also, no changes should have occurred this time.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Remove the specific setting for the index watcher, use the GitStatusMonitor
settings instead. UseFastChecks hardly adds any overhead, it adds
FileSystemWatcher to index and refs (GitStatusMonitor has a separate
watcher on index).
If the file system is changed, the top-left refresh button becomes red.
(As well as avoiding refreshing the grid in some situations, may not be working though.)
UseFastChecks was visible in Performance settings and expected by
users (and occasionally maintainers) to make a difference.
Sometimes the setting is mixed up with the GitStatusMonitor setting
that can make a difference (as git-status requires a lot of resources,
not FileSystemWatcher).
Screenshots
Before
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.