Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(windows): clean up logging #11921

Merged
merged 8 commits into from
Jul 27, 2024

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Jul 3, 2024

  • Remove unused parameters from SendDebugMessage* functions
  • Add SendDebugEntry and SendDebugExit functions for tracking function entry/exit
  • Add indenting and function names to log entries
  • Remove unused debug functions
  • Eliminate now-unused hwnd parameter in initialization functions
  • Replace Log, LogEntry, LogExit functions with SendDebug* equivalents in kmtip
  • Added 3 new exports: Keyman_WriteDebugEvent2W, Keyman_SendDebugEntry, Keyman_SendDebugExit

Many functions now have SendDebugEntry/SendDebugExit (or return_SendDebugExit) pairs. It is important to SendDebugExit on all returns from a function to keep the log indent depth consistent. In some cases I chose not to add these logging calls, e.g. on frequently called functions such as the message hooks.

User Testing

  • TEST_WINDOWS: Please run a basic test of Windows functionality, verifying that Keyman operates as expected.

* Remove unused parameters from SendDebugMessage functions
* Add SendDebugEntry and SendDebugExit functions for tracking
  function entry/exit
* Add indenting and function names to log entries
* Remove unused debug functions
* Eliminate now-unused hwnd parameter in initialization functions
* Replace Log,LogEntry,LogExit functions with SendDebug equivalents in
  kmtip

Many functions now have SendDebugEntry/SendDebugExit (or
return_SendDebugExit) pairs. It is important to SendDebugExit on all
returns from a function to keep the log indent depth consistent. In some
cases I chose not to add these logging calls, e.g. on frequently called
functions such as the message hooks.
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jul 3, 2024

User Test Results

Test specification and instructions

  • TEST_WINDOWS (PASSED): I tested this issue with the attached "keyman-18.0.67-alpha-test-11921" build on the Windows 10 & 11 OS environment: Here is my observation. (notes)

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S5 milestone Jul 3, 2024
@dinakaranr
Copy link

Test Results

  • TEST_WINDOWS (passed): I tested this issue with the attached "keyman-18.0.67-alpha-test-11921" build on the Windows 10 & 11 OS environment: Here is my observation.
  1. Installed the "keyman-18.0.67.exe" file.
  2. Keyman keyboard added in the system tray.
  3. Open the keyman "Configuration" window.
  4. Installed and uninstalled keyboards on the "Keyboard Layouts" tab.
  5. Hotkeys works which is mentioned on the "Hotkeys" tab.
  6. Keyboard switched between languages by (Left-ALT + Shift)
  7. The keyboard works well in Notepad and Libreoffice.
  8. Executed the suite_baseline, Suite_capslock and suite_TSF_app.
    The feature works well in Windows 10 & 11 OS environments. Thank you.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jul 3, 2024
Base automatically changed from refactor/windows/11917-remove-wm_keyman_key to master July 3, 2024 20:58
@@ -206,7 +206,7 @@ BOOL RegistryReadOnly::WrapError(DWORD res)
{
/*if(res != ERROR_SUCCESS)
{
SendDebugMessageFormat(0,KDS_PROGRAM,0,"RegistryFullAccess::WrapError = %d", res);
__logerror(0,KDS_PROGRAM,0,"RegistryFullAccess::WrapError = %d", res);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is commented out; just removing ref to SendDebugMessageFormat as this is a shared file

@mcdurdin
Copy link
Member Author

mcdurdin commented Jul 4, 2024

Noteː there may be something going on with performance once debugging is enabled -- I was finding significant lag while the debugging was enabled. Needs further verification before we merge

@mcdurdin
Copy link
Member Author

mcdurdin commented Jul 4, 2024

Need to disable CKMTipTextService::_PreserveAltKeys: Preserved key 100: K_oE2 0, 0 etc

@darcywong00 darcywong00 modified the milestones: A18S5, A18S6 Jul 5, 2024
Copy link
Contributor

@rc-swag rc-swag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment about the va_end(args);
lgtm

windows/src/engine/keyman32/k32_dbg.cpp Show resolved Hide resolved
windows/src/engine/keyman32/k32_dbg.cpp Show resolved Hide resolved
@darcywong00 darcywong00 modified the milestones: A18S6, A18S7 Jul 19, 2024
@mcdurdin mcdurdin merged commit 0175f90 into master Jul 27, 2024
19 checks passed
@mcdurdin mcdurdin deleted the refactor/windows/cleanup-engine-logging branch July 27, 2024 01:31
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.77-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants