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