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.

Sign up for GitHub

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

Jump to bottom

Updates to support writing to new Notepad on Windows 11 #60

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Updates to support writing to new Notepad on Windows 11 #60

wants to merge 17 commits into from

Conversation

bstordrup
Copy link

@bstordrup bstordrup commented May 14, 2023 •
edited
Loading

  • Added methods to User32 class to allow for enumerating child windows.
  • When trying to find the editor handle in Windows 11 Notepad, and FindWindowEx does not find it, an attempt is now made to enumerate the child windows of the notepadProcess.MainWindowHandle and returning the first found instance of RichEditD2DPT class. This is the last opened document in Notepad.
  • If Notepad is started with a parameter, the editor handle picked up may not be completely correct. So if the text length is not changed after sending a message to the editor handle, it is reset in order to find the editor handle again.

Closing #59

@bstordrup
Implementing an alternate way of finding editor handle for new Notepad
1728be2 
* Adding needed interop definitions to enumerate child windows
* If first attempt using FindWindowEx with notepad process MainWindowHandle does not find editor handle, the search is done by enumerating child window handles for the notepad process MainWindowHandle.
@bstordrup
Changes to handle changes in Notepad that allows multiple documents.
65f5399
@bstordrup
In some situations, a notepad window opened after logging start would…
0084257 
… not receive log messages.
@CLAassistant
Copy link

CLAassistant commented May 14, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@bstordrup
Sometimes, the editor handle seems not correct. So if the text length…
ab9ac54 
… does not change by sending text to it, it is cleared in order to reget it.
@bstordrup
Copy link
Author

@augustoproiete, I pushed another commit to the pull request

@bstordrup
Copy link
Author

@augustoproiete, any chance of looking at the PR? I need a new package with the fixes for a issue fix in my own project using this package.

@bstordrup
Copy link
Author

@augustoproiete, any news on completing the PR?

@augustoproiete
Copy link
Member

Hi @bstordrup thanks for the ping. I'll get this one merged & released over the weekend

bstordrup and others added 7 commits May 30, 2023 12:26
@bstordrup
Implementing an alternate way of finding editor handle for new Notepad
1b5bec4 
* Adding needed interop definitions to enumerate child windows
* If first attempt using FindWindowEx with notepad process MainWindowHandle does not find editor handle, the search is done by enumerating child window handles for the notepad process MainWindowHandle.
@bstordrup
Changes to handle changes in Notepad that allows multiple documents.
7647b4b
@bstordrup
In some situations, a notepad window opened after logging start would…
756b269 
… not receive log messages.
@bstordrup
Sometimes, the editor handle seems not correct. So if the text length…
7802dec 
… does not change by sending text to it, it is cleared in order to reget it.
@bstordrup
Merge branch 'Issue59_FindEditorHandleThroughChildWindowEnumeration' of
7fdcf57 
https://github.com/bstordrup/serilog-sinks-notepad into Issue59_FindEditorHandleThroughChildWindowEnumeration
@bstordrup @augustoproiete
Implementing an alternate way of finding editor handle for new Notepad
6db95e1 
* Adding needed interop definitions to enumerate child windows
* If first attempt using FindWindowEx with notepad process MainWindowHandle does not find editor handle, the search is done by enumerating child window handles for the notepad process MainWindowHandle.
@augustoproiete
PR rebase & minor tweaks
7ab2698
@augustoproiete augustoproiete changed the title Issue59 find editor handle through child window enumeration Updates to support writing to new Notepad on Windows 11 Jun 4, 2023
@augustoproiete augustoproiete linked an issue Jun 4, 2023 that may be closed by this pull request
Add support for writing to new Notepad on Windows 11 #59
Open
@augustoproiete augustoproiete force-pushed the Issue59_FindEditorHandleThroughChildWindowEnumeration branch from 7fdcf57 to 7ab2698 Compare June 4, 2023 06:17
augustoproiete
augustoproiete reviewed Jun 4, 2023
View reviewed changes
Copy link
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

Thanks a lot @bstordrup and apologies for the delay. I haven't been using Windows for some time now, and took me sometime to get both a Windows 10 and Windows 11 VMs ready to debug/run this sink.

Overall looks great, and everything works as expect on both Windows 10 and Windows 11 as per my tests.

I only have one concern about a possible memory leak in case the buffer is not cleared. I'm pushing a retry policy to address that. Let me know what you think.

src/Serilog.Sinks.Notepad/Sinks/Notepad/Interop/NotepadTextWriter.cs Outdated Show resolved Hide resolved
@bstordrup
Copy link
Author

bstordrup commented Jun 5, 2023 via email

@bstordrup
Resynced
3b920cc
@bstordrup
Rewriting logic to avoid Goto statement. The buffer is retrieved usin…
e86d7d3 
…g base.GetStringBuilder - which just returns the StringBuilder _sb from the base TextWriter class. And this one may or may not have created the StringBuilder instance - depending on which constructor has been called. But it does not destroy it.
@bstordrup
Setting succeeded after trying to write.
c733e95
@cdupetit
Copy link

cdupetit commented Oct 2, 2023

Hello @augustoproiete,
Any release date for a version with this pull request ?
Best regards

@alexachso
Copy link

alexachso commented Oct 4, 2023 •
edited
Loading

Hello @augustoproiete,
I am also interested in an official release with this P-R to use on a Windows 11 machine.
I think it would also be great if this sink could support .NET 7 or event .NET 8
Thank you!

@augustoproiete
Copy link
Member

Thanks for the ping gents. Definitely keen to get this in, and have been short on time for OSS. Will try to get some time on one of the upcoming weekends this month

@schnow265
Copy link

@augustoproiete any update on when this will be completed?

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

@augustoproiete augustoproiete augustoproiete left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for writing to new Notepad on Windows 11
6 participants
@bstordrup @CLAassistant @augustoproiete @cdupetit @alexachso @schnow265