WIP GPG Improvements Part 1

[vbjay]

Proposed changes

• GPG key id selection from gpg -K output in a dropdown
• Reflecting default actions better for signing tags/commits.
  • If commit.gpgSign is true then show in commit dialog that a commit will be signed by default and what key is being used.
  • Do the same for tags but tag.gpgSign config.
• pass no-gpg-sign or config in command line to actually not sign the commit or tag when user does select do not sign commit or relevant tag type. Fixes flaw where user selected the option but git config defaults the signing.

Screenshots

Before

TODO

After

Test methodology

• Added tests
• Debugged and verified functionality of ui and gpg signing info of commits

Test environment(s)

• GIT
• Windows

Merge strategy

• Merge commit. (PR submitter to rebase and squash before merges).

[RussKie]

@vbjay a friendly nudge. Is this still something you're working on?

[vbjay]

Yes. I need to rework some forms and add more tests.

[vbjay]

Verified appveyor generates a key that tests can access and the controls I wrote see the keys.

[gerhardol]

Some comments from my initial review before I look at this again:
#10473 (review)

This adds some time to the startup of these forms: Two git-config and gpg.exe (without any keys for me). This adds like 180 ms for me. Probably not much more in big repos, but in slower file systems.
Still, gpg signing should maybe be something that is enabled. I do not see an obvious way to do this with the Git configuration. Maybe if there are no keys, do not bother with the git-config (2*55ms). Or read the keys if signing is requested (including if e.g. commit.gpgSign is set).

[vbjay]

Some comments from my initial review before I look at this again: #10473 (review)

This adds some time to the startup of these forms: Two git-config and gpg.exe (without any keys for me). This adds like 180 ms for me. Probably not much more in big repos, but in slower file systems.
Still, gpg signing should maybe be something that is enabled. I do not see an obvious way to do this with the Git configuration. Maybe if there are no keys, do not bother with the git-config (2*55ms). Or read the keys if signing is requested (including if e.g. commit.gpgSign is set).

I am trying to make it reflect what will happen when committing. As in if user has commit/tag.GpgSign set then it will automatically say using default key in the right ui. Same with what key. For this to be reliable it MUST fetch to the minute config. Same with the idea of getting what keys exist at the moment. I am sorry but 108MS is nothing and I don't see the issue.

[gerhardol]

Some comments from my initial review before I look at this again: #10473 (review)

This adds some time to the startup of these forms: Two git-config and gpg.exe (without any keys for me). This adds like 180 ms for me. Probably not much more in big repos, but in slower file systems.
Still, gpg signing should maybe be something that is enabled. I do not see an obvious way to do this with the Git configuration. Maybe if there are no keys, do not bother with the git-config (2*55ms). Or read the keys if signing is requested (including if e.g. commit.gpgSign is set).

I am trying to make it reflect what will happen when committing. As in if user has commit/tag.GpgSign set then it will automatically say using default key in the right ui. Same with what key. For this to be reliable it MUST fetch to the minute config. Same with the idea of getting what keys exist at the moment. I am sorry but 108MS is nothing and I don't see the issue.

180 ms is certainly noticeable (and this may be a lot more in other setups). Some more features like this and a few seconds are added. If you use the functions, the delay is fine, but irritating otherwise.
At least limit the added delay to one command instead of three commands, probably a setting too.

[vbjay]

Some comments from my initial review before I look at this again: #10473 (review)

This adds some time to the startup of these forms: Two git-config and gpg.exe (without any keys for me). This adds like 180 ms for me. Probably not much more in big repos, but in slower file systems.
Still, gpg signing should maybe be something that is enabled. I do not see an obvious way to do this with the Git configuration. Maybe if there are no keys, do not bother with the git-config (2*55ms). Or read the keys if signing is requested (including if e.g. commit.gpgSign is set).

I am trying to make it reflect what will happen when committing. As in if user has commit/tag.GpgSign set then it will automatically say using default key in the right ui. Same with what key. For this to be reliable it MUST fetch to the minute config. Same with the idea of getting what keys exist at the moment. I am sorry but 108MS is nothing and I don't see the issue.

180 ms is certainly noticeable (and this may be a lot more in other setups). Some more features like this and a few seconds are added. If you use the functions, the delay is fine, but irritating otherwise. At least limit the added delay to one command instead of three commands, probably a setting too.

So basically neuter the feature behind a flag that will never get set like it took forever for us to turn on the show gpg tab before? KInda irritated with your feedback. You are basically complaining because I am adding functionality that it take more time to load. I reject this idea of neutering behind a flag because then it will fail to reflect what will happen and is kinda the whole point of this feature. It is to not only make it easy to manage the signing in the ui and also provide visibility into what will happen when git is told to create the commit or tag.

1. I am exposing visibility into what git will do. with the commit/tag.Commit.GpgSign reading and setting the dropdown to show the initial state
2. I am reading the default key to know if one is selected at the time of run of code. So that if someone went in and changed it in cli or eventually in settings dalog
3. I have to load the keys so that the dropdown has values to show and reflects this key is being used
4. The config of DEFAULT sign the commit/tag is driven by the commit.GpgSign config value

[vbjay]

Some comments from my initial review before I look at this again: #10473 (review)

This adds some time to the startup of these forms: Two git-config and gpg.exe (without any keys for me). This adds like 180 ms for me. Probably not much more in big repos, but in slower file systems.
Still, gpg signing should maybe be something that is enabled. I do not see an obvious way to do this with the Git configuration. Maybe if there are no keys, do not bother with the git-config (2*55ms). Or read the keys if signing is requested (including if e.g. commit.gpgSign is set).

I am trying to make it reflect what will happen when committing. As in if user has commit/tag.GpgSign set then it will automatically say using default key in the right ui. Same with what key. For this to be reliable it MUST fetch to the minute config. Same with the idea of getting what keys exist at the moment. I am sorry but 108MS is nothing and I don't see the issue.

180 ms is certainly noticeable (and this may be a lot more in other setups). Some more features like this and a few seconds are added. If you use the functions, the delay is fine, but irritating otherwise. At least limit the added delay to one command instead of three commands, probably a setting too.

So basically neuter the feature behind a flag that will never get set like it took forever for us to turn on the show gpg tab before? KInda irritated with your feedback. You are basically complaining because I am adding functionality that it take more time to load. I reject this idea of neutering behind a flag because then it will fail to reflect what will happen and is kinda the whole point of this feature. It is to not only make it easy to manage the signing in the ui and also provide visibility into what will happen when git is told to create the commit or tag.

1. I am exposing visibility into what git will do. with the commit/tag.Commit.GpgSign reading and setting the dropdown to show the initial state
2. I am reading the default key to know if one is selected at the time of run of code. So that if someone went in and changed it in cli or eventually in settings dalog
3. I have to load the keys so that the dropdown has values to show and reflects this key is being used
4. The config of DEFAULT sign the commit/tag is driven by the commit.GpgSign config value

What I can do is maybe see if I can put some of the setup in onshown so it doesn't affect first show time.

diff --git a/GitUI/CommandsDialogs/FormCommit.cs b/GitUI/CommandsDialogs/FormCommit.cs
index ac9882b48..711533eaa 100644
--- a/GitUI/CommandsDialogs/FormCommit.cs
+++ b/GitUI/CommandsDialogs/FormCommit.cs
@@ -156,6 +156,8 @@ public sealed partial class FormCommit : GitModuleForm
         private TranslationString _invalidSignCaption = new("Commit Signing Options");
         #endregion
 
+        private Stopwatch _st = new();
+
         private event Action? OnStageAreaLoaded;
 
         private readonly ICommitTemplateManager _commitTemplateManager;
@@ -347,8 +349,6 @@ public FormCommit(GitUICommands commands, CommitKind commitKind = CommitKind.Nor
 
             InitializeComplete();
 
-            toolStripGpgKeyComboBox.UICommandsSource = this;
-
             // By calling this in the constructor, we prevent flickering caused by resizing the
             // form, for example when it is restored to maximised, but first drawn as a smaller window.
             RestorePosition();
@@ -514,14 +514,6 @@ protected override void OnLoad(EventArgs e)
             // The problem is likely caused by 'splitRight.FixedPanel = FixedPanel.Panel2' fact, but other forms
             // have the same setting, and don't appear to suffer from the same bug.
             splitRight.SplitterDistance -= 6;
-
-            // If not changed, by default show "no sign commit"
-            if (gpgSignCommitToolStripComboBox.SelectedIndex == -1)
-            {
-                var gpg = toolStripGpgKeyComboBox.KeysUIController;
-                bool commitSign = gpg.GetCommitGPGSign();
-                gpgSignCommitToolStripComboBox.SelectedIndex = commitSign ? 1 : 0;
-            }
         }
 
         protected override void OnShown(EventArgs e)
@@ -571,6 +563,7 @@ protected override void OnShown(EventArgs e)
             }
 
             base.OnShown(e);
+            Debugger.Log(0, "GpgInfo", $"GPG stuff: {_st.ElapsedMilliseconds}ms{Environment.NewLine}");
 
             return;
 
@@ -1021,6 +1014,23 @@ private async Task UpdateBranchNameDisplayAsync()
 
         private void Initialize(bool loadUnstaged = true)
         {
+            if (!_initialized)
+            {
+                _st.Start();
+
+                // If not changed, by default show "no sign commit"
+
+                toolStripGpgKeyComboBox.UICommandsSource = this;
+                if (gpgSignCommitToolStripComboBox.SelectedIndex == -1)
+                {
+                    var gpg = toolStripGpgKeyComboBox.KeysUIController;
+                    bool commitSign = gpg.GetCommitGPGSign();
+                    gpgSignCommitToolStripComboBox.SelectedIndex = commitSign ? 1 : 0;
+                }
+
+                _st.Stop();
+            }
+
             _initialized = true;
 
             ThreadHelper.JoinableTaskFactory.RunAsync(() => UpdateBranchNameDisplayAsync());

[mstv]

What I can do is maybe see if I can put some of the setup in onshown so it doesn't affect first show time.

Why not async (as for user name & e-mail)?

[vbjay]

What I can do is maybe see if I can put some of the setup in onshown so it doesn't affect first show time.

Why not async (as for user name & e-mail)?

Sure. Just need to figure ot a way to show hey still loading.

just ran sandbox and told it to generate 1000 keys along with the 3 it already generated and messageboxed the load time

From fb01bbf4f981bfca4515b950ec28606e68a9a633 Mon Sep 17 00:00:00 2001
From: Jay Asbury <[email protected]>
Date: Sat, 5 Aug 2023 12:05:05 -0400
Subject: [PATCH] test

---
 GitCommands/Gpg/GPGLine.cs          | 104 ++++++++++---------
 GitCommands/Gpg/GpgKeyInfo.cs       | 156 ++++++++--------------------
 GitUI/CommandsDialogs/FormCommit.cs |  29 ++++--
 scripts/wsb/setup.ps1               |   8 ++
 4 files changed, 126 insertions(+), 171 deletions(-)
 rewrite GitCommands/Gpg/GpgKeyInfo.cs (78%)

diff --git a/GitCommands/Gpg/GPGLine.cs b/GitCommands/Gpg/GPGLine.cs
index 57528c499..5aa38c3b8 100644
--- a/GitCommands/Gpg/GPGLine.cs
+++ b/GitCommands/Gpg/GPGLine.cs
@@ -1,19 +1,39 @@
 namespace GitCommands.Gpg
 {
-    internal sealed partial class GPGLine
+    // Convert the following class to a record struct when C# 10 is released
+
+    /// <summary>
+    /// Models the records from running gpg -K --with-colons.  See <see href="https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob_plain;f=doc/DETAILS"/>
+    /// </summary>
+    internal record struct GPGLine()
     {
+        #region Properties
+        public bool Disabled { get; init; } = false;
+#pragma warning disable SA1313 // Parameter names should begin with lower-case letter
+
+#pragma warning restore SA1313 // Parameter names should begin with lower-case letter
+        public DateTimeOffset? ExpirationDate { get; init; } = null;
+        public readonly record struct Field(int Index, string Value);
+        public IEnumerable<Field> Fields { get; init; } = Enumerable.Empty<Field>();
+        public Capabilities LineCapabilities { get; init; } = Capabilities.None;
+        public int LineNumber { get; init; } = -1;
+        public LineTypes LineType { get; init; } = LineTypes.Other;
+
+        public Validities Validity { get; init; } = Validities.Unknown;
+        #endregion
+
         /// <summary>
         /// Models the records from running gpg -K --with-colons.  See <see href="https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob_plain;f=doc/DETAILS"/>
         /// </summary>
         /// <param name="lineNumber">Line row number</param>
         /// <param name="fields">The : split fields from the line</param>
-        public GPGLine(int lineNumber, IEnumerable<Field> fields)
+        public GPGLine(int lineNumber, IEnumerable<Field> fields) : this()
         {
             LineNumber = lineNumber;
             Fields = fields;
             LineType = DetermineLineType(fields?.FirstOrDefault().Value ?? "");
             Validity = DetermineLineValidity(fields?.Skip(FieldIndexes.Validity)?.FirstOrDefault().Value ?? "");
-            Disabled = fields?.Skip(FieldIndexes.KeyCapabilities)?.FirstOrDefault().Value?.Contains('D') ?? false;
+            Disabled = fields?.Skip(FieldIndexes.KeyCapabilities)?.FirstOrDefault().Value?.ToUpper().Contains('D') ?? false;
             LineCapabilities = DetermineCapabilities(fields?.Skip(FieldIndexes.KeyCapabilities)?.FirstOrDefault().Value ?? "");
             ExpirationDate = DettermineExpiration(fields?.Skip(FieldIndexes.ExpirationDate)?.FirstOrDefault().Value ?? "");
         }
@@ -33,7 +53,7 @@ public sealed class FieldIndexes
 
         #region RecordType
         /*
-*** Field 1 - Type of record
+    *** Field 1 - Type of record
 
     - pub :: Public key
     - crt :: X.509 certificate
@@ -82,7 +102,7 @@ private static LineTypes DetermineLineType(string lineTypeKey)
         /*
         Records marked with an asterisk are described at [[*Special%20field%20formats][*Special fields]].
 
-*** Field 2 - Validity
+    *** Field 2 - Validity
 
     This is a letter describing the computed validity of a key.
     Currently this is a single letter, but be prepared that additional
@@ -126,7 +146,7 @@ private static Validities DetermineLineValidity(string validityKey)
         /*
     If the validity information is given for a UID or UAT record, it
     describes the validity calculated based on this user ID.  If given
-for a key record it describes the validity taken from the best
+    for a key record it describes the validity taken from the best
     rated user ID.
 
 
@@ -134,37 +154,37 @@ private static Validities DetermineLineValidity(string validityKey)
     certificate (i.e. for the trust anchor) and an 'f' for all other
     valid certificates.
 
-In "sig" records, this field may have one of these values as first
+    In "sig" records, this field may have one of these values as first
     character:
 
-- ! :: Signature is good.
-- - :: Signature is bad.
-- ? :: No public key to verify signature or public key is not usable.
-- % :: Other error verifying a signature
+    - ! :: Signature is good.
+    - - :: Signature is bad.
+    - ? :: No public key to verify signature or public key is not usable.
+    - % :: Other error verifying a signature
 
-More values may be added later.The field may also be empty if
-gpg has been invoked in a non-checking mode (--list-sigs) or in a
-fast checking mode.Since 2.2.7 '?' will also be printed by the
-command --list-sigs if the key is not in the local keyring.
+    More values may be added later.The field may also be empty if
+    gpg has been invoked in a non-checking mode (--list-sigs) or in a
+    fast checking mode.Since 2.2.7 '?' will also be printed by the
+    command --list-sigs if the key is not in the local keyring.
     */
         #endregion
         /*
 
-*** Field 3 - Key length
+    *** Field 3 - Key length
 
     The length of key in bits.
 
-*** Field 4 - Public key algorithm
+    *** Field 4 - Public key algorithm
 
     The values here are those from the OpenPGP specs or if they are
     greater than 255 the algorithm ids as used by Libgcrypt.
 
-*** Field 5 - KeyID
+    *** Field 5 - KeyID
 
     This is the 64 bit keyid as specified by OpenPGP and the last 64
     bit of the SHA-1 fingerprint of an X.509 certifciate.
 
-*** Field 6 - Creation date
+    *** Field 6 - Creation date
 
     The creation date of the key is given in UTC.For UID and UAT
     records, this is used for the self-signature date.  Note that the
@@ -175,7 +195,7 @@ valid certificates.
     without using the =--fixed-list - mode = option used a "yyyy-mm-tt"
     format.
 
-* **Field 7 - Expiration date
+    * **Field 7 - Expiration date
 
     Key or UID / UAT expiration date or empty if it does not expire.
          */
@@ -192,21 +212,21 @@ valid certificates.
 
         /*
 
-* **Field 8 - Certificate S / N, UID hash, trust signature info
+    * **Field 8 - Certificate S / N, UID hash, trust signature info
 
     Used for serial number in crt records.For UID and UAT records,
     this is a hash of the user ID contents used to represent that
     exact user ID.For trust signatures, this is the trust depth
     separated by the trust value by a space.
 
-* **Field 9 - Ownertrust
+    * **Field 9 - Ownertrust
 
     This is only used on primary keys.This is a single letter, but
     be prepared that additional information may follow in future
     versions.For trust signatures with a regular expression, this is
     the regular expression value, quoted as in field 10.
 
-* **Field 10 - User - ID
+    * **Field 10 - User - ID
 
     The value is quoted like a C string to avoid control characters
     (the colon is quoted =\x3a =).For a "pub" record this field is
@@ -216,7 +236,7 @@ valid certificates.
     records store the fingerprints here.The fingerprint of a
     revocation key is stored here.
 
-***Field 11 - Signature class
+    ***Field 11 - Signature class
 
     Signature class as per RFC-4880.  This is a 2 digit hexnumber
     followed by either the letter 'x' for an exportable signature or
@@ -230,7 +250,7 @@ valid certificates.
         */
         #region KeyCapabilities
         /*
-*** Field 12 - Key capabilities
+    *** Field 12 - Key capabilities
 
     The defined capabilities are:
 
@@ -265,7 +285,7 @@ private static Capabilities DetermineCapabilities(string capabilities)
         #endregion
         /*
 
-*** Field 13 - Issuer certificate fingerprint or other info
+    *** Field 13 - Issuer certificate fingerprint or other info
 
     Used in FPR records for S/MIME keys to store the fingerprint of
     the issuer certificate.  This is useful to build the certificate
@@ -286,28 +306,28 @@ private static Capabilities DetermineCapabilities(string capabilities)
     if the key is missing but the signature carries an issuer
     fingerprint as meta data.
 
-*** Field 14 - Flag field
+    *** Field 14 - Flag field
 
     Flag field used in the --edit-key menu output
 
-*** Field 15 - S/N of a token
+    *** Field 15 - S/N of a token
 
     Used in sec/ssb to print the serial number of a token (internal
     protect mode 1002) or a '#' if that key is a simple stub(internal
     protect mode 1001).  If the option --with-secret is used and a
     secret key is available for the public key, a '+' indicates this.
 
-*** Field 16 - Hash algorithm
+    *** Field 16 - Hash algorithm
 
     For sig records, this is the used hash algorithm.  For example:
     2 = SHA-1, 8 = SHA-256.
 
-*** Field 17 - Curve name
+    *** Field 17 - Curve name
 
     For pub, sub, sec, ssb, crt, and crs records this field is used
     for the ECC curve name.
 
-*** Field 18 - Compliance flags
+    *** Field 18 - Compliance flags
 
     Space separated list of asserted compliance modes and
     screening result for this key.
@@ -318,7 +338,7 @@ private static Capabilities DetermineCapabilities(string capabilities)
     - 23 :: The key is compliant with compliance mode "de-vs".
     - 6001 :: Screening hit on the ROCA vulnerability.
 
-*** Field 19 - Last update
+    *** Field 19 - Last update
 
     The timestamp of the last update of a key or user ID.  The update
     time of a key is defined a lookup of the key via its unique
@@ -326,31 +346,17 @@ private static Capabilities DetermineCapabilities(string capabilities)
     update time of a user ID is defined by a lookup of the key using a
     trusted mapping from mail address to key.
 
-*** Field 20 - Origin
+    *** Field 20 - Origin
 
     The origin of the key or the user ID.This is an integer
     optionally followed by a space and an URL.  This goes along with
     the previous field.The URL is quoted in C style.
 
-*** Field 21 - Comment
+    *** Field 21 - Comment
 
     This is currently only used in "rev" and "rvs" records to carry
     the the comment field of the recocation reason.The value is
     quoted in C style.
- */
-        #region Properties
-        public bool Disabled { get; private set; }
-#pragma warning disable SA1313 // Parameter names should begin with lower-case letter
-        public readonly record struct Field(int Index, string Value);
-#pragma warning restore SA1313 // Parameter names should begin with lower-case letter
-        public DateTimeOffset? ExpirationDate { get; private set; }
-        public IEnumerable<Field> Fields { get; private set; }
-        public Capabilities LineCapabilities { get; private set; }
-        public int LineNumber { get; private set; }
-        public LineTypes LineType { get; private set; }
-
-        public Validities Validity { get; private set; }
-
-        #endregion
+    */
     }
 }
diff --git a/GitCommands/Gpg/GpgKeyInfo.cs b/GitCommands/Gpg/GpgKeyInfo.cs
dissimilarity index 78%
index b53df81b2..ee0731358 100644
--- a/GitCommands/Gpg/GpgKeyInfo.cs
+++ b/GitCommands/Gpg/GpgKeyInfo.cs
@@ -1,112 +1,44 @@
-namespace GitCommands.Gpg
-{
-    public class GpgKeyInfo : IEquatable<GpgKeyInfo>
-    {
-        private DateTimeOffset? _expires;
-
-        public GpgKeyInfo(
-            string fingerprint,
-            string keyID,
-            string userID,
-            DateTimeOffset? expires,
-            Capabilities capabilities,
-            Validities validities,
-            bool disabled,
-            bool isDefault = false)
-        {
-            Fingerprint = fingerprint;
-            KeyID = keyID;
-            UserID = userID;
-            Expires = expires;
-            Capabilities = capabilities;
-            Validities = validities;
-            IsDefault = isDefault;
-            Disabled = disabled;
-        }
-
-        public override string ToString() => $"{UserID} ({KeyID})";
-
-        public Capabilities Capabilities { get; private set; }
-        public bool Disabled { get; private set; }
-
-        public DateTimeOffset? Expires { get => _expires; private set => _expires = value?.ToLocalTime(); }
-
-        public string Fingerprint { get; private set; }
-
-        public bool IsDefault { get; private set; } = false;
-
-        public string KeyID { get; private set; }
-
-        public string UserID { get; private set; }
-
-        public Validities Validities { get; private set; }
-
-        #region Equality
-        public static bool operator !=(GpgKeyInfo first, GpgKeyInfo second)
-        {
-            return !(first == second);
-        }
-
-        public static bool operator ==(GpgKeyInfo first, GpgKeyInfo second)
-        {
-            if ((object)first == null)
-            {
-                return (object)second == null;
-            }
-
-            return first.Equals(second);
-        }
-
-        public override bool Equals(object obj)
-        {
-            if (obj is GpgKeyInfo gpgKeyInfo)
-            {
-                return Equals(gpgKeyInfo);
-            }
-
-            return base.Equals(obj);
-        }
-
-        public bool Equals(GpgKeyInfo other)
-        {
-            if (ReferenceEquals(null, other))
-            {
-                return false;
-            }
-
-            if (ReferenceEquals(this, other))
-            {
-                return true;
-            }
-
-            return Equals(Fingerprint, other.Fingerprint)
-                   && Equals(KeyID, other.KeyID)
-                   && Equals(UserID, other.UserID);
-        }
-
-        public override int GetHashCode()
-        {
-            unchecked
-            {
-                int hashCode = 47;
-                if (Fingerprint != null)
-                {
-                    hashCode = (hashCode * 53) ^ EqualityComparer<string>.Default.GetHashCode(Fingerprint);
-                }
-
-                if (KeyID != null)
-                {
-                    hashCode = (hashCode * 53) ^ EqualityComparer<string>.Default.GetHashCode(KeyID);
-                }
-
-                if (UserID != null)
-                {
-                    hashCode = (hashCode * 53) ^ EqualityComparer<string>.Default.GetHashCode(UserID);
-                }
-
-                return hashCode;
-            }
-        }
-        #endregion
-    }
-}
+namespace GitCommands.Gpg
+{
+    public readonly record struct GpgKeyInfo : IEquatable<GpgKeyInfo>
+    {
+        private readonly DateTimeOffset? _expires;
+
+        public GpgKeyInfo(
+            string fingerprint,
+            string keyID,
+            string userID,
+            DateTimeOffset? expires,
+            Capabilities capabilities,
+            Validities validities,
+            bool disabled,
+            bool isDefault = false) : this()
+        {
+            Fingerprint = fingerprint;
+            KeyID = keyID;
+            UserID = userID;
+            Expires = expires;
+            Capabilities = capabilities;
+            Validities = validities;
+            IsDefault = isDefault;
+            Disabled = disabled;
+        }
+
+        public override string ToString() => $"{UserID} ({KeyID})";
+
+        public Capabilities Capabilities { get; init; }
+        public bool Disabled { get; init; }
+
+        public DateTimeOffset? Expires { get => _expires; init => _expires = value?.ToLocalTime(); }
+
+        public string Fingerprint { get; init; }
+
+        public bool IsDefault { get; init; } = false;
+
+        public string KeyID { get; init; }
+
+        public string UserID { get; init; }
+
+        public Validities Validities { get; init; }
+    }
+}
diff --git a/GitUI/CommandsDialogs/FormCommit.cs b/GitUI/CommandsDialogs/FormCommit.cs
index ac9882b48..2f1eda7f0 100644
--- a/GitUI/CommandsDialogs/FormCommit.cs
+++ b/GitUI/CommandsDialogs/FormCommit.cs
@@ -156,6 +156,8 @@ public sealed partial class FormCommit : GitModuleForm
         private TranslationString _invalidSignCaption = new("Commit Signing Options");
         #endregion
 
+        private Stopwatch _st = new();
+
         private event Action? OnStageAreaLoaded;
 
         private readonly ICommitTemplateManager _commitTemplateManager;
@@ -347,8 +349,6 @@ public FormCommit(GitUICommands commands, CommitKind commitKind = CommitKind.Nor
 
             InitializeComplete();
 
-            toolStripGpgKeyComboBox.UICommandsSource = this;
-
             // By calling this in the constructor, we prevent flickering caused by resizing the
             // form, for example when it is restored to maximised, but first drawn as a smaller window.
             RestorePosition();
@@ -514,14 +514,6 @@ protected override void OnLoad(EventArgs e)
             // The problem is likely caused by 'splitRight.FixedPanel = FixedPanel.Panel2' fact, but other forms
             // have the same setting, and don't appear to suffer from the same bug.
             splitRight.SplitterDistance -= 6;
-
-            // If not changed, by default show "no sign commit"
-            if (gpgSignCommitToolStripComboBox.SelectedIndex == -1)
-            {
-                var gpg = toolStripGpgKeyComboBox.KeysUIController;
-                bool commitSign = gpg.GetCommitGPGSign();
-                gpgSignCommitToolStripComboBox.SelectedIndex = commitSign ? 1 : 0;
-            }
         }
 
         protected override void OnShown(EventArgs e)
@@ -571,6 +563,8 @@ protected override void OnShown(EventArgs e)
             }
 
             base.OnShown(e);
+            Debugger.Log(0, "GpgInfo", $"GPG stuff: {_st.ElapsedMilliseconds}ms{Environment.NewLine}");
+            MessageBox.Show(this, $"GPG stuff: {_st.ElapsedMilliseconds}ms", "GPG", MessageBoxButtons.OK, MessageBoxIcon.Information);
 
             return;
 
@@ -1021,6 +1015,21 @@ private async Task UpdateBranchNameDisplayAsync()
 
         private void Initialize(bool loadUnstaged = true)
         {
+            if (!_initialized)
+            {
+                _st.Start();
+
+                // If not changed, by default show "no sign commit"
+
+                toolStripGpgKeyComboBox.UICommandsSource = this;
+
+                GPGKeysUIController gpg = toolStripGpgKeyComboBox.KeysUIController;
+                bool commitSign = gpg.GetCommitGPGSign();
+                gpgSignCommitToolStripComboBox.SelectedIndex = commitSign ? 1 : 0;
+
+                _st.Stop();
+            }
+
             _initialized = true;
 
             ThreadHelper.JoinableTaskFactory.RunAsync(() => UpdateBranchNameDisplayAsync());
diff --git a/scripts/wsb/setup.ps1 b/scripts/wsb/setup.ps1
index 866286a3e..6fa0c74e4 100644
--- a/scripts/wsb/setup.ps1
+++ b/scripts/wsb/setup.ps1
@@ -55,6 +55,14 @@ $expired = $expired -replace 'Name-Real.*', 'Name-Real: GitExtensions Tester Sho
 $expired | Set-Content -Path $env:USERPROFILE\Desktop\Expired.dat
 gpg --batch --generate-key $env:USERPROFILE\Desktop\Expired.dat 2>&1
 
+$expired = Get-Content .\wsb\gen-gpg.dat 
+for (($i = 0); $i -lt 1000; $i++)
+{
+    $expired = $expired -replace 'Name-Real.*', "Name-Real: GitExtensions Tester $i"
+    $expired | Set-Content -Path $env:USERPROFILE\Desktop\Tons.dat
+    gpg --batch --generate-key $env:USERPROFILE\Desktop\Tons.dat 2>&1
+}
+
 $keyLines = gpg -K --with-colons | awk -F: '/^sec:/ { print $5 }'
 
 Set-Variable -Name 'keys' -Scope 'script' -Value @{
-- 
2.41.0.windows.1

So yes variable timing based on count of keys.

• Async... just need to hook into a progress of loading to show hey loading it
• some kind of key cache that can be forced to clear and refresh by user or by some kind of timeout could help also

I wanted to go async with it but had to test it without to deal with the basic functionality. Yes I did not want to run all of it in ui thread.

Thoughts on timeout of cache? 10 minutes too long?

[vbjay]

...

• Async... just need to hook into a progress of loading to show hey loading it
• some kind of key cache that can be forced to clear and refresh by user or by some kind of timeout could help also

I wanted to go async with it but had to test it without to deal with the basic functionality. Yes I did not want to run all of it in ui thread.

Thoughts on timeout of cache? 10 minutes too long?

I tried it out with a MemoryCache

and then open commit form again quickly

so I can at least make it so that if the keys were fetched recently, the cache will return the value but if not, it will run gpg again. I can play with the expiry timing and come up with a good value.

[RussKie]

I'm not convinced this is efficient. This method essentially re-runs the method that was run by executable.Start (line 130). There's no guarantee this method is side-effect free.

I suggest we do something like this:

diff --git a/GitCommands/Git/Executable.cs b/GitCommands/Git/Executable.cs
index 3942ddec3..c81d29056 100644
--- a/GitCommands/Git/Executable.cs
+++ b/GitCommands/Git/Executable.cs
@@ -13,6 +13,7 @@ public sealed class Executable : IExecutable
     {
         private readonly string _workingDir;
         private readonly Func<string> _fileNameProvider;
+        private string? _executableFileName;
         private readonly string _prefixArguments;
 
         public Executable(string fileName, string workingDir = "")
@@ -27,6 +28,10 @@ public Executable(Func<string> fileNameProvider, string workingDir = "", string
             _prefixArguments = prefixArguments;
         }
 
+#if DEBUG
+        internal string? ExecutableFileName => _executableFileName;
+#endif
+
         /// <inheritdoc />
         public IProcess Start(ArgumentString arguments = default,
                               bool createWindow = false,
@@ -41,9 +46,9 @@ public Executable(Func<string> fileNameProvider, string workingDir = "", string
 
             var args = (arguments.Arguments ?? "").Replace("$QUOTE$", "\\\"");
 
-            var fileName = _fileNameProvider();
+            _executableFileName = _fileNameProvider();
 
-            return new ProcessWrapper(fileName, _prefixArguments, args, _workingDir, createWindow, redirectInput, redirectOutput, outputEncoding, useShellExecute, throwOnErrorExit);
+            return new ProcessWrapper(_executableFileName, _prefixArguments, args, _workingDir, createWindow, redirectInput, redirectOutput, outputEncoding, useShellExecute, throwOnErrorExit);
         }

[vbjay]

Sure.

[RussKie]

Please keep these ordered.

[RussKie]

This looks as though we're leaking implementation details. This should be a validation event on the combobox itself.

[vbjay]

It is a validation of 2 fields. Index 0 is Do not sign Commit 1 and 2 are sign with default or sign with selected key. This validation is a you can't commit with these 2 values of these fields. The cases this covers are

• User has configured the commit.gpgSign value to true but does not have a default key so the key combobox will be no key selected. Since git config is not fully in our hands this state is possible
  • would result in state of git commit ... -S along with the user.signingKey not being set. Git would complain and fail the commit.
• User has selected sign with specific key and then failed to select a key.
  

User can skip the options menu completely. So I disagree with the validation event. Where the validation needs to occur is when they hit commit.
The validation logic is the same for tag.gpgSign also. Along with the validation being coded once in the controller and the ui needing to use it can do what it needs when it detects the state.

[RussKie]

For example:

gitextensions/GitUI/CommandsDialogs/FormBrowse.cs

Lines 266 to 268 in 99ece76

	fileToolStripMenuItem.Initialize(() => UICommands);
	helpToolStripMenuItem.Initialize(() => UICommands);
	toolsToolStripMenuItem.Initialize(() => UICommands);

gitextensions/GitUI/CommandsDialogs/FormBrowse.cs

Line 298 in 99ece76

ToolStripFilters.Bind(() => Module, RevisionGrid);

[RussKie]

We should avoid newing up concrete implementations as that makes the testing story much more complicated. Absent of a DI, this should be passed in via Bind() or Initialize mechanisms as implemented in other parts of the codebase.

[vbjay]

Ok. WIll look into. Yes. DI would have helped a ton in this.

[RussKie]

Ditto