Add scripts for BeforeFetch and AfterFetch #7026
Merged
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
ghost
assigned 
gerhardol
reviewed
Aug 18, 2019
|
|
||
| void executeBeforeScripts() | ||
| { | ||
| if (!Fetch.Checked) |
There was a problem hiding this comment.
Should pull be before fetch?
This is a little confusing. I had to think a little about the logic here, considering that either fetch or Pull scripts should run. The logic could be that Fetch only runs at fetch too.
Suggest adding a comment like:
// Request to pull/merge in addition to the fetch
This is a incompatible change, users may have to change their scripts.
There was a problem hiding this comment.
git pull is a shortcut for git fetch + git merge, so "fetch" is always run, and the "merge" part is only run for "pull" (in this case !Fetch).
There was a problem hiding this comment.
As RussKie mentioned, the fetch step ist part of the pull and therefore "locked in" by the pull Before/After.
You are right that the change is kind of "incompatible" since the old behaviour was to execute Before/AfterPull also on pure fetches (which IMO is not correct).
What I found kind of confusing is that the Before/AfterMerge scripts are not executed on pull (which is fetch+merge). Shouldn't a complete call-chain for pull be:
BeforePull - BeforeFetch - fetch - AfterFetch - BeforeMerge - merge - AfterMerge - AfterPull
?
Since git pull is a predefined command it would be a bit hard (and not wanted) to disassemble this, so it might be sufficient to consider the pull-merge as an own operation handled by the AfterPull.
There was a problem hiding this comment.
git pull is a shortcut for git fetch + git merge,
Sure, but the script runs in relation to git-fetch and git-git-pull. The call chain is not splitting the pull as KarstenRa's example. I have occasionally wanted this, but then is is the fetch-after that I needed (workaround is not so strange, only git-fetch and merge manually).
I accepted the functionality as is yesterday, but it is easier to explain that the commands are related to the Git commands, and it is normally better to implement something that can be explained.
I really had three observations in one comment here. The second must be resolved first anyway.
There was a problem hiding this comment.
I added the suggested comment since I get the confusion that !Fetch.Checked means fetch+merge in that context.
There was a problem hiding this comment.
The sequence is BeforePull-BeforeFetch-gitFetch/pull-AfterFetch-AfterPull
Is this what we want?
(I will probably approve anyway, this is not obvious)
There was a problem hiding this comment.
I think this is the most useful/evident policy as long as the pull is not splitted into fetch and merge (what I think would be a little overkill). I don't see any practical drawbacks in having the AfterFetch executed after the pull-merge (instead of before it), but do not exclude that I am missing a usecase.
RussKie
reviewed
Aug 19, 2019
Co-Authored-By: Igor Velikorossov <RussKie@users.noreply.github.com>
RussKie
reviewed
Aug 20, 2019
GitUI/CommandsDialogs/FormPull.cs
Outdated
| @@ -437,7 +437,7 @@ public DialogResult PullChanges(IWin32Window owner) | |||
| PopStash(); | |||
| } | |||
|
|
|||
| ScriptManager.RunEventScripts(this, ScriptEvent.AfterPull); | |||
| executeAfterScripts(); | |||
There was a problem hiding this comment.
It looks like an existing bug, I don't think we should be running scripts if there was an error or the operation was aborted.
There was a problem hiding this comment.
I updated the PR with a check if the pull was aborted or an error occured. I think if the pull results in merge conflicts this is not an error and the AfterPull scripts should run, correct me if I am missing something here.
gerhardol
approved these changes
Aug 21, 2019
There was a problem hiding this comment.
I would have preferred to separate git-fetch and git-pull actions, but I can live with this.
Looking forward for a documentation update!
https://github.com/gitextensions/GitExtensionsDoc/blob/master/source/settings.rst
|
@KarstenRa please send a PR updating the docs and we'll get this one merged. Thank you |
|
Thank you |
Fabman08
added a commit
to Fabman08/gitextensions
that referenced
this pull request
Oct 7, 2019
* Path must be well formatted for Path.GetDirectoryName() * Update namespace * Fix i8n strings in GitUICommands * GitHubHostedRemote: the underneath repository should be initialized with the GitProtocol already used * Fix Clone Url when using https for parent repository like what have been done in gitextensions#6515 (07acef9) * Add menu entry to add the GitHub "upstream" remote and fetch from it once added * refactor: remove `IGitModule` from `IRepositoryHostPlugin` to have a consistent api. Following comment: gitextensions#6328 (review) * Update translations * Move UI-only strings to GitUI project * Remove obsolete UI from TranslationApp * Add script options c/sRemoteBranchName and RepoName * Restore title of AppearanceSettingsPage * Check for empty language selection * Update/cleanup appveyor.yml * Enforce update to translations as part of a PR * stage *.xlf files - this way we ignore EOL issues, * check if there are any xlf files are modified, and if so: - capture and publish the diff for the xlf files as git-diff-xlf.txt - break the build with a meaningful error * Update translations * Deprecate and remove InvokeAsyncDoNotUseInNewCode Rewrite the callsite to use `InvokeAsync` instead, that switches the execution context to the UI thread. Closes gitextensions#6799 * RevisionLinks: Add templates for GitHub and AzureDevOps + Creation of GitHubRemoteParser & AzureDevOpsRemoteParser * Check that directory (still) exists before renabling the the FileWatcher for GitStatusMonitor * Show changed images in all diff tabs Previously, images were displayed as binary diffs, except some in FormCommit: .png and new worktree images Other: * FormCommit always show the worktree commit for index .png * Show all images in FormCommit (i.e. show index images and changed other than .png) * Regression from a94a30a (ObjectId) where blob guid were ignored and diff not shown * Combined guid should never be used to get file blob guid * openWithDiffTool action were not called from stash and RevisionDiff * StatusMonitor: CancelToken to limit multiple commands * Use CheckBox.Text instead of separate Label in order to indicate focus and to make hotkey work directly * Centralise the common logic * fix: Ensure configured merge tool is present KDiff3 was unbundled few versions ago, and some users no longer have a diff/merge tool installed. Until now the "resolve conflicts" dialog did not check whether a configured tool physically existed in the system. An error dialog is shown to the user if the tool is absent. Closes gitextensions#6687 * Enhance error messages * Rename to EnsureCommitMessageSecondLineEmpty * Do not skip empty lines fix regression introduced by commit template handling * Factor out method and add tests * Reposition buttons and add tooltips * Update the layout for scaling * Require repro steps before submission Ask a user to provide more information before a report can be submitted. Closes gitextensions#6607 * Update translations * SettingsCache: Tempfile in temp dir * Hide Watermark, too, if FilterComboBox invisible make appearance the same as "There are no unstaged changes" * refactor: Move email settings from General to Detailed * Fix TryParseAsciiHex bytes bounds check * Fix "Blame to previous revision" to select the first commit before the revision selected (so the previous, no!?!) Partly fix gitextensions#6605 * Blame: Add a context menu item to blame selected revision to make explicit what could be done by double-clicking * Refactoring & fixes following reviews... * feature: Add setup telemetry dialog Add a dialog to the setup that asks user's permission to capture telemetry information. The setup will execute a script that will update user's configuration that is stored in C:\Users\<user>\AppData\Roaming\GitExtensions\GitExtensions\GitExtensions.setting with the user's selection. * Disable blame menu items if revision is not in the revision grid * feature: Add ApplicationInsights telemetry Relates to gitextensions#6021 * Capture (selected) application telemetry information * Prompt user to allow telemetry upon start up, if it isn't configured * Allow telemetry information capture be toggled via a About menu * Allow telemetry information capture be toggled via the settings dialog The following information is captured: Application-Level includes: * Exception information * Version number (e.g. 2.0.x.x) * Is portable version * Build type (whether the application is an official release build or not) * Selected layout settings (such as visibility of the left panel, commit info position etc) * Change of selected layout settings * Git version (e.g. 2.19.0) * SSH client (e.g. OpenSSH or PuTTY) Operating System-Level includes: * Version (e.g. Windows 10.0.17763.0) * Machine Name (e.g. MyFastPC) * .NET CLR version (e.g. 4.0.30319.42000) * .NET SDK version (e.g. dotnet:2.10.0-24102) * Current culture * Current UI culture * Number of monitors * Resolution of all monitors * Primary monitor DPI / scale factor * feature: Add ApplicationInsights telemetry Relates to gitextensions#6021 * Capture (selected) application telemetry information * Prompt user to allow telemetry upon start up, if it isn't configured * Allow telemetry information capture be toggled via a About menu * Allow telemetry information capture be toggled via the settings dialog The following information is captured: Application-Level includes: * Exception information * Version number (e.g. 2.0.x.x) * Is portable version * Build type (whether the application is an official release build or not) * Selected layout settings (such as visibility of the left panel, commit info position etc) * Change of selected layout settings * Git version (e.g. 2.19.0) * SSH client (e.g. OpenSSH or PuTTY) Operating System-Level includes: * Version (e.g. Windows 10.0.17763.0) * Machine Name (e.g. MyFastPC) * .NET CLR version (e.g. 4.0.30319.42000) * .NET SDK version (e.g. dotnet:2.10.0-24102) * Current culture * Current UI culture * Number of monitors * Resolution of all monitors * Primary monitor DPI / scale factor * Fix gitextensions#5944 by fixing --follow and revision reading a306eb6 by mistake removed the "--follow" option when getting the git log for history. This is reverted by this commit. 1f6b0ca by mistake removed parsing of the file name data into GitRevision.Name. This is also reverted by this commit. Together, these should fix rename tracking in the file history. * Set recommended Git version to 2.22.0 * Fix FileHistory/Blame for files not existing in the working directory The bad behavior comes from the fact that `TryGetExactPath()` changes the path to the file system path (if not existing, the path is cleared) Alternative for gitextensions#6893 * Redesign form "Commit template Settings" * Commit designer changes due to /noscale * Better menu item label in form commit * Commit validation: better use of space for RegEx field * Use a tab control to split "template" & "Validation" * Left align controls * Improve test readability on `TryGetExactPathName()` * Stash: Add cherry pick of the currently selected file changes Fixes gitextensions#6902 * fix: Commit message incorrect height at 150% scale factor Closes gitextensions#6898 * Add support for "--rebase-merges" for newest version of git now that we recommend v2.22 (gitextensions#6769) that deprecate "--preserve-merges" See: * for option deprection: git/git@fa1b86e#diff-c7361e406139e8cd3a300b80b8f8cc8dR1220 * for some doc: https://stackoverflow.com/questions/15915430/what-exactly-does-gits-rebase-preserve-merges-do-and-why/50555740#50555740 * fixup! fix: Commit message incorrect height at 150% scale factor * Fix: Build error due to duplicate NSubstitue entry in csproj it's already in the proj as a package reference * Sign contributors.txt * signed contributors.txt * ReleaseNotesGenerator plugin To field defaults to "HEAD" (gitextensions#4444) * fix Delete tag not working Fixes gitextensions#6281 * improved validation on create new repository dialog (fixes gitextensions#6955) * Add an option to sort commits by author date in the revision graph. * Adding myself to contributors.txt * fix: Illegal characters in path Do not crash the app if a repository contains illegal characters. Closes gitextensions#6982 * fix tests * fix: Remote Repositories List Empty Restore saving a successfully cloned repository to the list of recent remotes. Fixes gitextensions#6983 * Add option for default pull action in setting page This change adds a dropdown for choosing default pull action on the general tab in main setting dialog. * Delete obsolete branches plugin - fix gitextensions#6813 * Add codescene.io analysis results (gitextensions#6272) Codescene.io doesn't require any build programming, just a free account and an integration button click at codescene.io. This may be a perfect fit for codecov analysis already in place. * Replace git-diff patience with histogram * Fix: Commit Dialog Spell Checker * TortoiseGitMerge.exe old default name * Adding myself to contributors.txt * Fix: Proper spacing (gitextensions#7002) Fix: Proper spacing * Updated changed github id * Fix: Keep push dialog open on failure (gitextensions#6367) * The GroupBox items contained in the form need to… (gitextensions#7021) * Fix for issue gitextensions#7020. The GroupBox items contained in the form need to have their AutoSize properties set to true, so they can resize when their contained controls do. * Added my name to the contributors * Settings: Display label to the top to make UI more readable on multiline controls * should fix: [NBug] Sequence contains no elements gitextensions#7011 * fix: Crash when suggest merge/diff tool paths If merge/diff tool paths enclosed in quotes, clicking on "Suggest" button would crash the app. Strip quotes from paths before attempting to suggest. Fixes gitextensions#7000 * Display icons in commit templates menu items * Fix potential bad end of line replacement If the value contains a `\r\n`, the result will contain `\r\r\n` which duplicate blank lines at the loading and each time a settings is changed and saved... * JiraCommitHintPlugin: fix bad condition * JiraCommitHintPlugin: Open the settings when not enabled * Add scripts for BeforeFetch and AfterFetch (gitextensions#7026) Additional scripts that should be executed before/after fetch can be added. Fixes gitextensions#4909 * Fix: Add "Force push with lease" to the push dialog (gitextensions#3934) * Generate GitHub OAuth token with github api * Settings: Ensure that `CustomControl` is set if we let the Setting(s) classes create the controls. Otherwise, it could will trow a NRE if we access it to get the current value * Add me (KvanTTT) to contributors.txt * fix: Overflow error in Commit Dialog DiffViewerLineNumberControl did not handle when MaxLineNumber was 0. Either an empty file can cause this (in this case _diffLines is empty so no problem) or a file where diff hunks were lost due to encoding errors. Fixes gitextensions#7023 * Add a `PseudoSetting` to display controls that are not settings i.e. that don't save a value in the settings files. It is used to easily display a control that is not a setting (linklabel, text,...) PS: Solve problem linklabels truncated horizontally of vertically if the good size is not set (because the size no more have to be set) * JiraCommitHintPlugin: Simplify settings definition by using`PseudoSetting` Solve LinkLabel slightly truncated vertically * Plugins: Being able to change CredentialsControl labels to specify other things than the currently "User name" / "Password". For example: "email" / "Token" * Add me (palver123) to contributors.txt * Update the GitExtensionsDoc submodule to 3.2 * Allow multi-select of files in FormStash * Clarify Settings-Stash label * File suffix incorrectly detected If the file has no extension but the directory path has a period * Ignore difftool for CombinedDiff * fix installer for Win7 * Fix scrolling for committers label in statistics plugin (gitextensions#7092) * Fix scrolling for committers label in statistics plugin * Fix opencover not submiting reports Looks like a regression on Appveyor side after they updated the VS2017 image. The opencover now needs to be run as admin (?) * Reintegration of PluginManager. * Fix duplicate ignored tests * Remove test assuming local file paths * correctly call hMSBuild batch file * Update version * Add dummy item for the menu entry to appear expandable (7046) Add dummy item for the menu entry to appear expandable (7046) * JiraPlugin: Fix way to define TranslationString that prevent translation + remove TranslationString redundant with settings caption (which is translated when defined in a private field!) * Remove KDiff3 hardcoding * Update README.md * Update ChangeLog.md * Update README.md * Set repo workingdir from env only if cmd line arguments This will allow starting with the dashboard in the debugger * Set recommended Git version to 2.23.0 * PathUtil.GetDirectoryName did not accept root dirs * Avoid exception when starting from commandline with rooted dir * fix incorrect user-agent Fixes gitextensions#7157 * Add hotkeys for middle of rebase etc. * exename not set for tortoisemerge * Consider the end-of-body marker everywhere in the detection of single-line commit messages (completes 53ebf95) * Add more test cases for empty lines in commit msgs addendum to PR gitextensions#6877 * Allow 'Amend Commit' for any CommitKind reverts 82f125d partially * Improve some mnemonics of commit dialog * ConEmu 19.7.14 * Refactor: argument always null; positive logic * correctly detect script options starting with "s" resolves regression from commit 1b8bb93 * FormBrowse: provide RevisionGrid to ScriptRunner * Add NUnit tests for ScriptRunner; handle nulls * Settings: Prevent the multiple creation of controls because `GetSettings()` (where controls are created) is call 3 times that ends with that the controls displayed are not the one that are the ones set in `CustomControl` property. And that can cause problems if we manipulate `CustomControl`in the setting page to get (until the setting is saved) or set a value. We **must** keep the strategy that `GetSettings()` return every time new controls because the previous instances of the controls are disposed when the settings form is closed. These changes ensure that the `GetSettings()` method is called only **1** time. The 2 other times (use of `.Any()`) has been replaced by a `HasSettings` property. * Add modifyCommitMessageButton * GitStatusMonitor: Timer wraps after 25 days * Always compare timer diff (to handle timer wraparound after 25 days) to avoid missing updates while command is running * Increase min update time 3s -> 5s at changes in the file system * Use locks consistently * Start interactive update only once to avoid race where two commands started * Gerrit Plugin fixes/improvements Fix syntax for gerrit push command Fix Gerrit Publish dialog layout Add CC and Hashtag to Gerrit Push See: https://gerrit-review.googlesource.com/Documentation/user-upload.html#_git_push * Try fix gitextensions#7205, gitextensions#7193 * Branch list could contain '+' Seen in CommitInfo Added test * Popup when error staring mergetool * Catch ConEmu exception for Done() * Support older Gerrit API Close gitextensions#7200 * Update contributors.txt (gitextensions#7220) * Update contributors.txt * Blame: Fix missing commit metadata on some commits due to "orphan" filename lines that has for effect to overwrite good commit data by incomplete ones when "Detect move and copy in all files" is enabled. For example, command: `git blame --porcelain -M -C -w -l "18fcca6d89c2d38ac1830697b5da3f78630b226e" -- "GitUI/Translation/English.Plugins.xlf"` returns (part of the file): `8f8393c283e7ad60c02805182d6778e5fc0c8b28 180 13 <trans-unit id="MsBuildEnabled.Caption"> b1d567e 7506 14 3 filename GitUI/Translation/English.xlf <= orphan line that breaks the blame <source>Enabled</source> b1d567e 7507 15 <target /> b1d567e 7508 16 </trans-unit> 8f8393c 184 17 12 <trans-unit id="MsBuildPath.Caption">` * Update contributors.txt Fix emails * Update readme * Exclude "fixup!" and "squash!" prefixes from commit message RegEx validation That prevent a boring question to be asked to the user because the commit message doesn't follow the regex when "fixup!" or "squash!" prefixes are used. * Dont confirm switch worktree option (gitextensions#6864) Dont confirm switch worktree option (gitextensions#6864) Dont confirm switch worktree option (gitextensions#6864) * GitIndexWatcher: check that directory exists before enabling * Settings: Regenerate the Controls when previous instance was disposed because we don't control the controls lifecycle and the settings controls are disposed when settings are closed. It depends on how the settings control is defined but could prevent exceptions (and make plugins easier to write) * Ordering of options Ordering of options * Add support for command line commit message when committing close gitextensions#7219 * sort fields * Add tests for `ArgumentBuilder`
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.
Fixes #4909
Proposed changes
Additional scripts that should be executed before/after fetch can be added.
Test methodology
Manual
Test environment(s)
✒️ I contribute this code under The Developer Certificate of Origin.