GPG: don't crash on artificial commits #11389
Conversation
GitUI/CommandsDialogs/FormBrowse.cs
Outdated
| bool showGpgInfoTab = revision?.IsArtificial != true && AppSettings.ShowGpgInformation.Value; | ||
|
|
||
| if (showGpgInfoTab) | ||
| { |
GitUI/CommandsDialogs/FormBrowse.cs
Outdated
| @@ -1263,6 +1263,24 @@ private void FillCommitInfo(GitRevision? revision) | |||
|
|
|||
| private async Task FillGpgInfoAsync(GitRevision? revision) | |||
| { | |||
| // Don't show the "GPG" tab for artificial commits | |||
| bool showGpgInfoTab = revision?.IsArtificial != true && AppSettings.ShowGpgInformation.Value; | |||
There was a problem hiding this comment.
| bool showGpgInfoTab = revision?.IsArtificial != true && AppSettings.ShowGpgInformation.Value; | |
| bool showGpgInfoTab = revision?.IsArtificial is false && AppSettings.ShowGpgInformation.Value; |
There was a problem hiding this comment.
That's what is false means. null is not false, i.e. do not show the tab if we do not have a real revision.
There was a problem hiding this comment.
I am still unconvinced. These two expressions aren't equivalent:
There was a problem hiding this comment.
I'm not sure we are discussing about the logic but more on the business which is: does it make sense to display the tab when the there is no revision selected (i.e. empty repo)? I think that's not really important....
It has been decided to show the "File tree" in this case (code I borrowed) but as @mstv says, maybe it's better to not show the GPG tab in this case. Or you prefer @RussKie?
I tend to share @mstv point of view so I will change for "is false".
There was a problem hiding this comment.
== and is are equivalent - as it should be.
If it does not work here, add parens.
sb.AppendLine("\tnull\tfalse\ttrue");
foreach (bool? value in new bool?[] { null, false, true })
{
sb.AppendLine($"{value?.ToString() ?? "null"} is\t{value is null}\t{value is false}\t{value is true}")
.AppendLine($"{value?.ToString() ?? "null"} ==\t{value == null}\t{value == false}\t{value == true}")
.AppendLine($"{value?.ToString() ?? "null"} is !\t{value is not null}\t{value is not false}\t{value is not true}")
.AppendLine($"{value?.ToString() ?? "null"} !=\t{value != null}\t{value != false}\t{value != true}");
}
MessageBox.Show(sb.ToString());
There was a problem hiding this comment.
I feel we have had this discussion before...
Just to clarify:
revision?.IsArtificial is false is equivalent to revision is not null && revision?.IsArtificial == false
== and is are equivalent - as it should be.
...unless you override the operator ==...
and hide the tab Fixes gitextensions#11388
0572667 to
5ac8966
Compare
and hide the tab Fixes gitextensions#11388 (cherry picked from commit 0fc9a06)
and hide the tab
Fixes #11388
Screenshots
Before
Crash
After
We don't see the tab anymore:
Test methodology
Test environment(s)
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.