Re: [PATCH v2 1/3] conhost: Save console settings as global defaults on first run

[Jacek Caban]

On 4/19/22 13:26, Hugh McMaster wrote:
> On Windows, HKCU\Console holds global default settings, while subkeys hold
> any app-specific settings that differ from the defaults.
>
> Wine's conhost.exe implementation currently saves all console settings to
> an app-specific subkey on the first run, while global defaults are only
> saved to HKCU\Console if the user selects the Default pop-up menu option.
> This is the opposite of the behaviour on Windows.
> Signed-off-by: Hugh McMaster
>
> programs/conhost/window.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/programs/conhost/window.c b/programs/conhost/window.c
> index fb02ef9fd94..130be396820 100644
> --- a/programs/conhost/window.c
> +++ b/programs/conhost/window.c
> @@ -817,9 +817,9 @@ static int WINAPI get_first_font_sub_enum( const LOGFONTW *lf, const TEXTMETRICW
>           fc->done = 1;
>
> -
>        /* since we've modified the current config with new font information,
> -
>         * set this information as the new default.
> -
>         */
> -
>        /* Save default font and console configuration to the registry */
> -
>        load_config( NULL, &config );
> -
>         load_config( fc->console->window->config_key, &config );

The new call seems to be no-op, the second load_config() will replace 
the result of the first one.
>           config.cell_width  = fc->console->active->font.width;
>           config.cell_height = fc->console->active->font.height;
>
> @@ -827,10 +827,8 @@ static int WINAPI get_first_font_sub_enum( const LOGFONTW *lf, const TEXTMETRICW
>                       fc->console->active->font.face_len * sizeof(WCHAR) );
>               config.face_name[fc->console->active->font.face_len] = 0;
> -
>        /* Force also its writing back to the registry so that we can get it
> -
>         * the next time.
> -
>         */
> -
>        save_config( fc->console->window->config_key, &config );
> -
>        save_config( NULL, &config );
> -

While you mention first run in the commit message, note that this is 
used in set_output_info() code path, so it will modify registry in 
response to SetCurrentConsoleFontEx() call. What's the desired effect of 
that? An app-specific key seems more appropriate for that case, but 
maybe we shouldn't write to registry at all?

Thanks,

Jacek