Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/conhost.h | 1 + programs/conhost/window.c | 15 +++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 8ca09bb80d0..2cb9515037a 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -77,6 +77,7 @@ struct console HANDLE server; /* console server handle */ unsigned int mode; /* input mode */ struct screen_buffer *active; /* active screen buffer */ + WCHAR *config_key; /* registry key name for app-specific settings */ int is_unix; /* UNIX terminal mode */ int use_relative_cursor; /* use relative cursor positionning */ INPUT_RECORD *records; /* input records */ diff --git a/programs/conhost/window.c b/programs/conhost/window.c index fb02ef9fd94..77e17313f9b 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -49,7 +49,6 @@ struct console_window COORD selection_start; /* selection coordinates */ COORD selection_end; unsigned int ui_charset; /* default UI charset */ - WCHAR *config_key; /* config registry key name */ LONG ext_leading; /* external leading for font */
BOOL quick_edit; /* whether mouse ops are sent to app or used for content selection */ @@ -820,7 +819,7 @@ static int WINAPI get_first_font_sub_enum( const LOGFONTW *lf, const TEXTMETRICW /* since we've modified the current config with new font information, * set this information as the new default. */ - load_config( fc->console->window->config_key, &config ); + load_config( fc->console->config_key, &config ); config.cell_width = fc->console->active->font.width; config.cell_height = fc->console->active->font.height; memcpy( config.face_name, fc->console->active->font.face_name, @@ -830,7 +829,7 @@ static int WINAPI get_first_font_sub_enum( const LOGFONTW *lf, const TEXTMETRICW /* 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( fc->console->config_key, &config ); return 0; } } @@ -1955,7 +1954,7 @@ static BOOL config_dialog( struct console *console, BOOL current ) update_window( di.console ); } if (save) - save_config( current ? console->window->config_key : NULL, &di.config ); + save_config( current ? console->config_key : NULL, &di.config ); return TRUE; }
@@ -2405,14 +2404,14 @@ BOOL init_window( struct console *console ) if (si.lpTitle) { size_t i, title_len = wcslen( si.lpTitle ); - if (!(console->window->config_key = malloc( (title_len + 1) * sizeof(WCHAR) ))) + if (!(console->config_key = malloc( (title_len + 1) * sizeof(WCHAR) ))) return FALSE; for (i = 0; i < title_len; i++) - console->window->config_key[i] = si.lpTitle[i] == '\' ? '_' : si.lpTitle[i]; - console->window->config_key[title_len] = 0; + console->config_key[i] = si.lpTitle[i] == '\' ? '_' : si.lpTitle[i]; + console->config_key[title_len] = 0; }
- load_config( console->window->config_key, &config ); + load_config( console->config_key, &config ); if (si.dwFlags & STARTF_USECOUNTCHARS) { config.sb_width = si.dwXCountChars;
Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/window.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/programs/conhost/window.c b/programs/conhost/window.c index 77e17313f9b..7564dce51b7 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -265,11 +265,11 @@ static void load_config( const WCHAR *key_name, struct console_config *config ) config->edition_mode = 0;
/* read global settings */ - if (!RegOpenKeyW( HKEY_CURRENT_USER, L"Console", &key )) + if (!RegOpenKeyExW( HKEY_CURRENT_USER, L"Console", 0, KEY_READ, &key )) { load_registry_key( key, config ); /* if requested, load part related to console title */ - if (key_name && !RegOpenKeyW( key, key_name, &app_key )) + if (key_name && !RegOpenKeyExW( key, key_name, 0, KEY_READ, &app_key )) { load_registry_key( app_key, config ); RegCloseKey( app_key );
Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/window.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/programs/conhost/window.c b/programs/conhost/window.c index 7564dce51b7..6cccba69d57 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -816,20 +816,17 @@ 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. - */ - load_config( fc->console->config_key, &config ); + /* Save default font and console configuration to the registry */ + load_config( NULL, &config ); + config.cell_width = fc->console->active->font.width; config.cell_height = fc->console->active->font.height; memcpy( config.face_name, fc->console->active->font.face_name, 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->config_key, &config ); + save_config( NULL, &config ); + return 0; } }
Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/window.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/programs/conhost/window.c b/programs/conhost/window.c index 6cccba69d57..cb93ad257a4 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -346,31 +346,29 @@ static void save_registry_key( HKEY key, const struct console_config *config )
static void save_config( const WCHAR *key_name, const struct console_config *config ) { - HKEY key, app_key; - - TRACE( "%s %s\n", debugstr_w( key_name ), debugstr_config( config )); + WCHAR *key_path = NULL; + HKEY hkey;
- if (RegCreateKeyW( HKEY_CURRENT_USER, L"Console", &key )) + if (key_name) { - ERR("Can't open registry for saving\n"); - return; + int len = lstrlenW( key_name ) + 9; + key_path = malloc( len * sizeof(WCHAR) ); + wsprintfW( key_path, L"%s\%s", L"Console", key_name ); }
- if (key_name) + if (!RegCreateKeyExW( HKEY_CURRENT_USER, key_name ? key_path : L"Console", 0, NULL, + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hkey, NULL )) { - if (RegCreateKeyW( key, key_name, &app_key )) - { - ERR("Can't open registry for saving\n"); - } - else - { - /* FIXME: maybe only save the values different from the default value ? */ - save_registry_key( app_key, config ); - RegCloseKey( app_key ); - } + TRACE( "Saving %s console settings\n", key_name ? debugstr_w( key_name ) : "default" ); + + save_registry_key( hkey, config ); + RegCloseKey( hkey ); + } + else + { + ERR( "Error saving %s console settings (%lu)\n", + key_name ? debugstr_w( key_name ) : "default", GetLastError() ); } - else save_registry_key( key, config ); - RegCloseKey(key); }
/* fill memory DC with current cells values */
Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/window.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/programs/conhost/window.c b/programs/conhost/window.c index cb93ad257a4..d713a7f573b 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -238,7 +238,7 @@ static void load_config( const WCHAR *key_name, struct console_config *config )
HKEY key, app_key;
- TRACE("loading %s registry settings.\n", wine_dbgstr_w( key_name )); + TRACE( "Loading default console settings\n" );
memcpy( config->color_map, color_map, sizeof(color_map) ); memset( config->face_name, 0, sizeof(config->face_name) ); @@ -264,13 +264,15 @@ static void load_config( const WCHAR *key_name, struct console_config *config ) config->win_pos.Y = 0; config->edition_mode = 0;
- /* read global settings */ + /* load default console registry settings */ if (!RegOpenKeyExW( HKEY_CURRENT_USER, L"Console", 0, KEY_READ, &key )) { load_registry_key( key, config ); - /* if requested, load part related to console title */ + + /* load app-specific console registry settings (if any) */ if (key_name && !RegOpenKeyExW( key, key_name, 0, KEY_READ, &app_key )) { + TRACE( "Loading %s console settings\n", wine_dbgstr_w( key_name ) ); load_registry_key( app_key, config ); RegCloseKey( app_key ); }
On Windows, app-specific console settings are always saved unless the user cancels the dialog.
Signed-off-by: Hugh McMaster hugh.mcmaster@outlook.com --- programs/conhost/conhost.h | 4 --- programs/conhost/conhost.rc | 12 ------- programs/conhost/window.c | 69 +++++-------------------------------- 3 files changed, 9 insertions(+), 76 deletions(-)
diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 2cb9515037a..242d0ae7769 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -186,7 +186,6 @@ static inline unsigned int get_bounded_cursor_x( struct screen_buffer *screen_bu #define IDD_OPTION 0x0100 #define IDD_FONT 0x0200 #define IDD_CONFIG 0x0300 -#define IDD_SAVE_SETTINGS 0x0400
/* dialog boxes controls */ #define IDC_OPT_CURSOR_SMALL 0x0101 @@ -217,6 +216,3 @@ static inline unsigned int get_bounded_cursor_x( struct screen_buffer *screen_bu #define IDC_CNF_WIN_HEIGHT_UD 0x0308 #define IDC_CNF_CLOSE_EXIT 0x0309 #define IDC_CNF_EDITION_MODE 0x030a - -#define IDC_SAV_SAVE 0x0401 -#define IDC_SAV_SESSION 0x0402 diff --git a/programs/conhost/conhost.rc b/programs/conhost/conhost.rc index e7c4416b634..820220b9d60 100644 --- a/programs/conhost/conhost.rc +++ b/programs/conhost/conhost.rc @@ -114,18 +114,6 @@ FONT 8, "MS Shell Dlg" COMBOBOX IDC_CNF_EDITION_MODE, 119, 69, 75, 60, CBS_DROPDOWNLIST|WS_VSCROLL|WS_TABSTOP }
-IDD_SAVE_SETTINGS DIALOG 20, 20, 210, 60 -STYLE DS_MODALFRAME | WS_POPUP | WS_CAPTION -CAPTION "Console parameters" -FONT 8, "MS Shell Dlg" -{ - AUTORADIOBUTTON "Retain these settings for later sessions", IDC_SAV_SAVE, 10, 8, 200, 10, WS_TABSTOP - AUTORADIOBUTTON "Modify only current session", IDC_SAV_SESSION, 10, 22, 200, 10, WS_TABSTOP - - DEFPUSHBUTTON "OK", IDOK, 100, 43, 50, 14 - PUSHBUTTON "Cancel", IDCANCEL, 155, 43, 50, 14 -} - LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
#define WINE_FILEDESCRIPTION_STR "Wine conhost" diff --git a/programs/conhost/window.c b/programs/conhost/window.c index d713a7f573b..db407f6d0a7 100644 --- a/programs/conhost/window.c +++ b/programs/conhost/window.c @@ -1700,34 +1700,6 @@ static INT_PTR WINAPI config_dialog_proc( HWND dialog, UINT msg, WPARAM wparam, return TRUE; }
-/* dialog proc for choosing how to handle modification to the console settings */ -static INT_PTR WINAPI save_dialog_proc( HWND dialog, UINT msg, WPARAM wparam, LPARAM lparam ) -{ - switch (msg) - { - case WM_INITDIALOG: - SendMessageW( dialog, WM_NEXTDLGCTL, (WPARAM)GetDlgItem( dialog, IDC_SAV_SESSION ), TRUE ); - SendDlgItemMessageW( dialog, IDC_SAV_SESSION, BM_SETCHECK, BST_CHECKED, 0 ); - return FALSE; - - case WM_COMMAND: - switch (LOWORD(wparam)) - { - case IDOK: - EndDialog( dialog, - (IsDlgButtonChecked(dialog, IDC_SAV_SAVE) == BST_CHECKED) ? - IDC_SAV_SAVE : IDC_SAV_SESSION ); - break; - case IDCANCEL: - EndDialog( dialog, IDCANCEL ); break; - } - break; - default: - return FALSE; - } - return TRUE; -} - static void apply_config( struct console *console, const struct console_config *config ) { if (console->active->width != config->sb_width || console->active->height != config->sb_height) @@ -1845,19 +1817,17 @@ static BOOL config_dialog( struct console *console, BOOL current ) PROPSHEETPAGEW psp; WNDCLASSW wndclass; WCHAR buff[256]; - BOOL modify_session = FALSE; - BOOL save = FALSE;
InitCommonControls();
memset( &di, 0, sizeof(di) ); di.console = console; - if (!current) - { + + if (current) + current_config( console, &di.config ); + else load_config( NULL, &di.config ); - save = TRUE; - } - else current_config( console, &di.config ); + prev_config = di.config;
wndclass.style = 0; @@ -1923,35 +1893,14 @@ static BOOL config_dialog( struct console *console, BOOL current )
TRACE( "%s\n", debugstr_config(&di.config) );
- if (!save) - { - switch (DialogBoxW( GetModuleHandleW( NULL ), MAKEINTRESOURCEW(IDD_SAVE_SETTINGS), - console->win, save_dialog_proc )) - { - case IDC_SAV_SAVE: - save = TRUE; - modify_session = TRUE; - break; - case IDC_SAV_SESSION: - modify_session = TRUE; - break; - default: - ERR( "dialog failed\n" ); - /* fall through */ - case IDCANCEL: - modify_session = FALSE; - save = FALSE; - break; - } - } - - if (modify_session) + if (current) { apply_config( console, &di.config ); update_window( di.console ); } - if (save) - save_config( current ? console->config_key : NULL, &di.config ); + + save_config( current ? console->config_key : NULL, &di.config ); + return TRUE; }
Hi Hugh,
On 4/11/22 14:35, Hugh McMaster wrote:
Signed-off-by: Hugh McMasterhugh.mcmaster@outlook.com
programs/conhost/conhost.h | 1 + programs/conhost/window.c | 15 +++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 8ca09bb80d0..2cb9515037a 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -77,6 +77,7 @@ struct console HANDLE server; /* console server handle */ unsigned int mode; /* input mode */ struct screen_buffer *active; /* active screen buffer */
- WCHAR *config_key; /* registry key name for app-specific settings */ int is_unix; /* UNIX terminal mode */ int use_relative_cursor; /* use relative cursor positionning */ INPUT_RECORD *records; /* input records */
Why do you need that change? It seems to me that when it's needed, we always have console_window struct available anyway. Similar question to patches 2, 3, and 4 of the series, it's not obvious to me what's the plan for them.
Thanks,
Jacek
Hi Jacek,
On Thu, 14 Apr 2022 at 04:29, Jacek Caban wrote:
Hi Hugh,
On 4/11/22 14:35, Hugh McMaster wrote:
Signed-off-by: Hugh McMasterhugh.mcmaster@outlook.com
programs/conhost/conhost.h | 1 + programs/conhost/window.c | 15 +++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 8ca09bb80d0..2cb9515037a 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -77,6 +77,7 @@ struct console HANDLE server; /* console server handle */ unsigned int mode; /* input mode */ struct screen_buffer *active; /* active screen buffer */
- WCHAR *config_key; /* registry key name for app-specific settings */ int is_unix; /* UNIX terminal mode */ int use_relative_cursor; /* use relative cursor positionning */ INPUT_RECORD *records; /* input records */
Why do you need that change? It seems to me that when it's needed, we always have console_window struct available anyway. Similar question to patches 2, 3, and 4 of the series, it's not obvious to me what's the plan for them.
Patch 1. I know it doesn't make a difference to how we use or access config_key, but moving it to struct console seems to be a more natural location than struct console_window. Moving config_key to struct console is also consistent with pointers to other app- and window-specific members, such as struct screen_buffer *active and struct console_window *window, both of which are accessed from struct console. You'll note that struct console holds *title, history-related members etc., while other active config sits in struct screen_buffer and struct console_window.
Patch 2. Calls to RegOpenKey ultimately call RegOpenKeyEx (with the broadest access), so it's better to just call RegOpenKeyEx directly. There should also be a tiny speed improvement by reducing the number of calls, but I haven't done any testing on that.
Patch 3. I should have added a commit message on this. On Windows, HKCU\Console holds global console default settings, and HKCU\Console<app_path> holds any app-specific settings that differ from the global settings. If there are no differences, the app-specific key does not exist.
Wine currently stores all console settings in an app-specific key after setting a default font on first init. Global console settings are only saved in HKCU\Console if using the Default settings dialog option, but this results in two sets of console settings in the registry. This patch stores default settings in HKCU\Console on first run. App-specific settings are then saved in the relevant subkey if any specific settings are set by the user via the Properties dialog. This change is required so we can eventually only save app-specific settings that differ from the global settings, mirroring Windows.
Patch 4. With this patch, I wanted to clean up the multiple calls to RegCreateKey and save_registry_key, and avoid duplicate error handling, when we could handle this code logic once. As we only ever save to HKCU\Console or a subkey, it seems unnecessary to open HKCU\Console via RegCreateKey and then open the subkey as well.
I don't how strongly you feel about these changes. From my perspective, I think they're all nice improvements, but I do need patch 3. I'm working on an additional patch so we only save different settings to the app-specifc registry key, mirroring the behaviour on Windows.
Thanks for seeking clarification. Happy to discuss further.
Hugh
-
Hugh McMaster -
Jacek Caban