serilog-sinks-notepad icon indicating copy to clipboard operation
serilog-sinks-notepad copied to clipboard

Updates to support writing to new Notepad on Windows 11

Open bstordrup opened this issue 1 year ago • 9 comments

  • 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 avatar May 14 '23 20:05 bstordrup

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 14 '23 20:05 CLAassistant

@augustoproiete, I pushed another commit to the pull request

bstordrup avatar May 15 '23 17:05 bstordrup

@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 avatar May 17 '23 07:05 bstordrup

@augustoproiete, any news on completing the PR?

bstordrup avatar May 23 '23 05:05 bstordrup

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

augustoproiete avatar May 25 '23 19:05 augustoproiete

Did you verify if there is a memory leak? The buffer is retrieved and cleared in the Flush method, so it should be disposed after the method ends. But it might depend on how the GetStringBuilder method creates it. I will have a look.

If you don't mind, I will try to rewrite the retry logic. I am not very fond of the goto statement 😁

søn. 4. jun. 2023 08.41 skrev C. Augusto Proiete @.***>:

@.**** commented on this pull request.

Thanks a lot @bstordrup https://github.com/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.

In src/Serilog.Sinks.Notepad/Sinks/Notepad/Interop/NotepadTextWriter.cs https://github.com/serilog-contrib/serilog-sinks-notepad/pull/60#discussion_r1216287880 :

  •            // Otherwise, we clear the buffer
    
  •            buffer.Clear();
    

It seems that if we keep failing to write to the Notepad, the buffer would keep growing forever 🤔. I think we need to add a retry policy here (say, try 3 times, and give up)

— Reply to this email directly, view it on GitHub https://github.com/serilog-contrib/serilog-sinks-notepad/pull/60#pullrequestreview-1460973958, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKMB2RMMUIOD5D32LOECUVTXJQUZBANCNFSM6AAAAAAYBLNPEQ . You are receiving this because you were mentioned.Message ID: @.*** com>

bstordrup avatar Jun 05 '23 14:06 bstordrup

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

cdupetit avatar Oct 02 '23 14:10 cdupetit

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!

alexachso avatar Oct 04 '23 16:10 alexachso

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

augustoproiete avatar Oct 04 '23 20:10 augustoproiete