Don't use "AddSettingBinding" in "FormBrowseRepoSettingsPage"

[ghost]

Fixes #8446

Proposed changes

• The use "AddSettingBinding" in "FormBrowseRepoSettingsPage" doesn't make sense because the page implements only IGlobalSettingsPage

Screenshots

Before

After

Test methodology

• Manual

How to test

Before

After

Test environment(s)

• Git Extensions 3.4.3.9999
• Build d4b0f48
• Git 2.28.0.windows.1
• Microsoft Windows NT 10.0.19041.0
• .NET Framework 4.8.4220.0
• DPI 96dpi (no scaling)

---

✒️ I contribute this code under The Developer Certificate of Origin.

[codecov]

Codecov Report

Merging #8458 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #8458      +/-   ##
==========================================
- Coverage   53.11%   53.10%   -0.01%     
==========================================
  Files         882      882              
  Lines       62380    62382       +2     
  Branches    11349    11349              
==========================================
- Hits        33136    33131       -5     
- Misses      26576    26579       +3     
- Partials     2668     2672       +4     

Flag	Coverage Δ
#production	41.02% <0.00%> (-0.02%)	⬇️
#tests	94.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[pmiossec]

It fixes the loading problem with the checkbox.
Was it the only page doing that? Because there are other pages with only "Global for all repositories"...

Don't you think there is a way to fix AddSettingBinding() to handle pages with only "Global for all repositories" to not be obliged to do something like that on all the pages like that?

[ghost]

Was it the only page doing that?

Only used here.

Don't you think there is a way to fix AddSettingBinding() to handle pages with only "Global for all repositories" to not be obliged to do something like that on all the pages like that?

Something tells me that this is not implemented.

Perhaps you can clarify the purpose.

The purpose of the "Effective" setting is not clear. I would be glad if someone could clarify the purpose.

[ghost]

It may be necessary to download "Effective" instead of "Global". Looking deeper, there is a lot of code to touch.

Better to put it out as a separate task with clear goals.

We want to refactor

void PageToSettings()
void SettingsToPage()

to

void Init(ISettingsPageHost pageHost)
{
   AddSettingBinding(AppSettings.ShowConEmuTab, chkChowConsoleTab);
}

for all pages?

Or just for simple settings.

[RussKie]

Perhaps you can clarify the purpose.

The purpose of the "Effective" setting is not clear. I would be glad if someone could clarify the purpose.

Some settings can be distributed:

• global settings - %UserAppData%\GitExtensions.settings
• team settings - settings that are distributed with the repo stored in .\GitExtensions.settings
• user settings - user-only repo-specific settings that reside in .\.git folder

Each layer overrides the above, so user > team > global. "Effective" is the lowest-level settings available.

Many years ago there was a discussion that you may want to read: #1707

[RussKie]

MC

[ghost]

Alternatively, we can add distributed flag.

public BoolSetting(string name, bool defaultValue, bool distributed)
            : this(name, name, defaultValue)
{
}

Or

public ISettingControlBinding CreateControlBinding(bool distributed)
{
    return new CheckBoxBinding(this, CustomControl, distributed);
}

public override void LoadSetting(ISettingsSource settings, CheckBox control)
{
    var applyDefaultValue = (_distributed && settings.SettingLevel == SettingLevel.Effective)
        || !_distributed;

    bool? settingVal = applyDefaultValue
        ? Setting.ValueOrDefault(settings)
        : Setting[settings];

    control.SetNullableChecked(settingVal);
}

But this will require changing the interfaces, which will affect the work of plugins.

If we go down this path, I would first separate Setting and Binding.
What do you think?

[RussKie]

But this will require changing the interfaces, which will affect the work of plugins.

I would avoid this for the moment.

[RussKie]

@pmiossec any objections?

[pmiossec]

@pmiossec any objections?

no. LGTM