fix: label "No changes" no more hiding first file of the list

[pmiossec]

when opening "FormLog"
even if it was set as not visible.

Fixes #11679

Reproduce step: double click on a revision in the revision grid

Regression introduced by f8c0353

/!\ Technical reason of the bug:
Previous code was:

        FilterComboBox.Visible = filesPresent || (SearchComboBox.Visible && !string.IsNullOrEmpty(SearchComboBox.Text));
        NoFiles.Visible = !FilterComboBox.Visible;

But as FilterComboBox.Visible is not just a property storing the value directly to a backed field
(it has a complex WinFoms internal logic),
the value retrieved by the getter ( to after set NoFiles.Visible) is sometimes wrong and not the one set to the property :(
The value is not "well" set due to complex internal logic and so a wrong value is used after.

Using a temporary variable fix the issue because the set of the 2 properties has no more impact on each others!

Screenshots

Before

After

Test methodology

• Manual

Test environment(s)

• Git Extensions 33.33.33
• Build 10b42c5
• Git 2.45.0.windows.1
• Microsoft Windows NT 10.0.22631.0
• .NET 8.0.1
• DPI 96dpi (no scaling)
• Portable: False
• Microsoft.WindowsDesktop.App Versions

    Microsoft.WindowsDesktop.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 7.0.15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 8.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

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.

[mstv]

Rather set the visibility in dependency on whether the commit is empty (tested with branch mstv/repro/empty_commit).

[gerhardol]

Set visibility is preferred, bring to front disrupts but just changing Visible seem to change order in some situations at least.
I had some problems here...
Whatever that works is OK for me.

[pmiossec]

I will have a look again but the problem is that visibility is already set to false and the label is still visible 🤷

So I tried that as we already play with that to bring the label to front.

[RussKie]

Can we push the lable to the back rather change the z-order of other controls?

[pmiossec]

I have spent some time investigating and found the real reason behind the regression in SetFileStatusListVisibility() (introduced by f8c0353 ).

It was a difficult investigation partly because I didn't know the search feature.

Explained in the commit message (that I reproduce here):

Previous code was:

       FilterComboBox.Visible = filesPresent || (SearchComboBox.Visible && !string.IsNullOrEmpty(SearchComboBox.Text));
       NoFiles.Visible = !FilterComboBox.Visible;

But as FilterComboBox.Visible is not just a property storing the value directly to a backed field
(it has a complex WinFoms internal logic),
the value retrieved by the getter ( to after set NoFiles.Visible) is sometimes wrong and not the one set to the property :(
The value is not "well" set due to complex internal logic and so a wrong value is used after.

Using a temporary variable fix the issue because the set of the 2 properties has no more impact on each others!

[pmiossec]

Investigation was painful and time consuming (also partly because, as I said I didn't know the search feature and the bug let me think that I missed something logical) and make me doubt about my knowledge of the De Morgan's Law Formula !(A||B) = !A && !B because it seems the 2 parts were not returning the same value 🤯

but in fact the "bad behavior" lie in FilterComboBox.Visible getter/setter implementation/behavior....: we don't always get back the value that have just been set 😕

[mstv]

Investigation was painful and time consuming
We don't always get back the value that have just been set

Heavy! Good to know!

[mstv]

❔ Then also

Suggested change

	if (NoFiles.Visible)
	if (!filesToFilter)

Or is only ComboBox affected?

[pmiossec]

Or is only ComboBox affected?

It seems to be OK for the label but as I can't be sure, you're right it's better to be careful. Done.

[gerhardol]

approved pending mstv comment

I spent a lot of time on this too, handled many cases but missed this...
Thanks

[RussKie]

Investigation was painful and time consuming (also partly because, as I said I didn't know the search feature and the bug let me think that I missed something logical) and make me doubt about my knowledge of the De Morgan's Law Formula !(A||B) = !A && !B because it seems the 2 parts were not returning the same value 🤯

but in fact the "bad behavior" lie in FilterComboBox.Visible getter/setter implementation/behavior....: we don't always get back the value that have just been set 😕

Ah yes, Control.Visible (and few other properties) have complex logic behind them, and one shouldn't rely on those directly for state keeping purposes.