Clipboard helpers were added #15 #16

Trapov · 2018-01-17T12:49:33Z

Few functions connected to the windows clipboard were added and a wrapper around it.

will update my pull request in the future, please don't merge yet. created it for the sake of knowing what im doing

Clipboard
- Can pass IClipboardData
- Can read IClipboardData
- Allows subscribe option(Post event is not implemented)
Changed samples a bit
Added a console application sample

Wasn't able to solve my problem with a cancelation token in Clipboard tho, maybe make EventLoop().Run() async? I know it should be wraped in the the using statement, but still.

Issue -> #15

22.06.18 Update:

The Win32Clipboard class was moved into a different PR

Trapov · 2018-01-20T13:29:50Z

@prasannavl any thoughts?

prasannavl · 2018-01-20T13:33:32Z

@Trapov, thank you for the PR. Sorry, was a busy week. Had to put this off until the weekend. Will have a look by the weekend.

Trapov

suggestions

Trapov · 2018-01-20T14:45:05Z

WinApi.Windows.Controls/Clipboard/Clipboard.cs

+                    new EventLoop().Run(_wind);
+                else
+                {
+                    throw new Exception("Can't subscribe to the clipboard");


probably should throw bounded to context exceptions, like SubscribeException?

Trapov · 2018-01-20T14:49:11Z

WinApi.Windows.Controls/Clipboard/Clipboard.cs

+        private void ReleaseUnmanagedResources()
+        {
+            //TODO: implement cancelation token
+            _cancellationTokenSource.Cancel();


should probably somehow interact with the EventLoop().Run() method, just to stop it.

Trapov · 2018-01-20T14:50:22Z

WinApi.Windows.Controls/Clipboard/Clipboard.cs

+                        if (User32Methods.OpenClipboard(new IntPtr()))
+                        {
+                            var handle = User32Methods.GetClipboardData((uint) ClipboardFormat.CF_BITMAP);
+                            data = GdiplusHelpers.CreateBitmap(handle) ?? throw new Exception("Can't read the bitmap");


probably should throw bounded to context exceptions, like ClipboardReadException?

Trapov · 2018-01-20T14:54:02Z

WinApi/Gdiplus/Helpers.cs

+{
+    public static class GdiplusHelpers
+    {
+        public static byte[] CreateBitmap(IntPtr hGlobal)


since this method generates an image NOT in memory, maybe return the image itself(or filename, a handle, ect) and not bytes?

Trapov · 2018-05-21T11:15:30Z

@prasannavl well, i know it's been a while ago since i opened the PR but i still want it to be merged.

prasannavl · 2018-05-21T11:31:37Z

Hi @Trapov, I'm sorry I haven't been able to catch up on this. I've been struggling with some hand surgeries for the last few months, and have been completely off programming. Just slowly easing back in. Unfortunately, I'm still catching up with a lot of the backlog and it may take a while for me to look into the design.

That being said, could you please make a separate PR for the Constants, and User32/Methods? That should really be helpful, but they don't really require any design thought.

PS: For future reference, I also suggest making separate PRs for direct Win32 items (Methods, Constants, etc), helpers, and library features. That helps in merging smaller isolated items without delays. :)

Trapov · 2018-05-21T11:49:32Z

@prasannavl sorry to hear that, get better. Ok, will do like you said.

Also the feature is not nearly done, there are still some things that aren't tested and have bad design, i want to hear your opinion on those things, such us: cancellation token, getting an image from the cliboard and etc.

Trapov · 2018-05-22T11:04:54Z

@prasannavl ok, now this PR is solely dedicated to pinvoke/helper-methods and constants. I can make another one with the Win32Clipboard control, but it will depend from this branch, since it will use the clipboard functions, constants and helpers; not really sure if that's a great idea.

prasannavl

@Trapov - While I do not want to discourage you -- as suggested, could you please isolate the Constants+Methods (which are direct Win32 derivations) from Helpers into separate PRs?

Contants + Methods, I can merge instantly. This way, helpers can be reverted later if needed. Bunching up the PRs make it difficult to do that.

prasannavl · 2018-05-22T11:55:37Z

WinApi/User32/Helpers.cs

+
+                    var result = User32Methods.SetClipboardData((uint)ClipboardFormat.CF_TEXT, allocatedMemory);
+
+                    if (result == IntPtr.Zero)


Throwing exceptions on a native API that indicates an error result is a no no. Instead, a better design would be to make it return a bool if IntPtr is zero, so the application can act in whichever way it deems necessary.

prasannavl

Also, I'm not entirely sure about the symmetry of the APIs. PasteTextToClipboard doesn't actually paste text, just data -- it's a byte array. But copying returns a string (that could be null). Too many caveats. It'll be confusing to use this API.

So, I think a better design would be PasteToClipboard. And the GetFromClipboard that's just the data again. And then have helper method GetUnicodeTextFromClipboard, that always returns a valid string -- String.Empty instead of null.

prasannavl · 2018-05-22T12:13:22Z

WinApi/User32/Helpers.cs

+            {
+                var bytesLength = data.Length;
+                var allocatedMemory = Marshal.AllocHGlobal(bytesLength);
+                try


The try-finally is useless here. Marshal.Copy is not going to throw. Infact AllocHGlobal is the one that can throw that's outside the scope - though it's not needed to handle it. Just simple statements will do, since if OutOfMemory is thrown, the application will have to react to it anyway.

prasannavl · 2018-05-22T12:15:54Z

WinApi/User32/Helpers.cs

+
+        public static unsafe string GetUnicodeTextFromClipboard()
+        {
+            var outString = default(string);


The default of string is null. I'd try to be explicit, so there are less chances of confusion of a consumer thinking it'll always return a string, and it's more readable. Just don't initialise it instead, and move it into the if statement. Just return a null in the end.

prasannavl · 2018-05-22T12:16:11Z

WinApi/User32/Helpers.cs

+                var data = (char*)User32Methods.GetClipboardData((uint) ClipboardFormat.CF_UNICODETEXT);
+                outString = new string(data);
+            }
+            User32Methods.CloseClipboard();


This has to be moved above into the if statement.

@prasannavl, Oh, right, and in the TrySetClipboardData method as well. Damn, didn't see it at first. Will update it shortly, sorry to keep you waiting.

prasannavl · 2018-05-22T12:16:44Z

WinApi/User32/Helpers.cs

+            return outString;
+        }
+
+        public static unsafe string GetTextFromClipboard()


Similar comments as the above method.

Trapov · 2018-05-22T13:15:48Z

Thank you for the response.

And the GetFromClipboard that's just the data again

@prasannavl I would do that, but since GetClipboardData returns a pointer, i would have to know it exact length/size, to get a byte[] array from that. I could just go thru it and determine where is the end of the array by checking null values, don't really think it's a good idea tho.

Will fix everything else.

prasannavl · 2018-05-22T13:31:20Z

Yeah, Clipboard is not a simple API. It's handles a lot of conversions and complexities. In that case, what do you think about just focusing on text instead in the helpers. Just take a string input for SetClipboardUnicodeText, and GetClipboardUnicodeText, and use CF_UNICODETEXT for both. And use SetClipboardData as a raw method that takes a byte array as you've used and uses NULL for the format internally. But leave out the Get part for that until we can come up with a better design. Meanwhile, the User32 function can always be used directly utilising the pointer.

Trapov · 2018-05-22T13:40:46Z

Yeah, sounds good.

prasannavl · 2018-05-22T13:52:51Z

Additionally, SetClipboardData could have an overload with the exact format constants.

Trapov · 2018-05-23T06:14:57Z

@prasannavl what do you think about those apis?

prasannavl · 2018-05-23T08:35:10Z

This is great, actually 👍. I like it.

Just thinking out loud here: Would it be better to have TryGetUnicodeTextFromClipboard as TryGetClipboardUnicodeText? So it has parity with TrySetClipboardUnicodeText.

And also has the minor side advantage of getting nice and relevant autocomplete with or without fuzzy matching, when you type TryGetClip.

Trapov · 2018-05-23T08:59:03Z

@prasannavl, yea that makes sense. Will do.

…#15

Trapov · 2018-05-23T09:11:59Z

@prasannavl done. You probably want to squash all my commits, or rebase, because they don't make much sense in the context of your repo history, imho.

Trapov · 2018-08-14T18:24:06Z

@prasannavl, any other problems with the pull request?

BinToss · 2021-09-21T01:51:26Z

@Trapov If you're still active, you should rebase and push --force-with-lease. That tends to have a cleaner history across repositories. If prasannavl rebased your branch manually or by using GitHub's "Rebase and Merge Pull Request" feature, then the commit history here will be slightly different than your commit history. This makes tracing commits by their hashes back to their origin a but more difficult.
If there are merge conflicts, then prasannavl's conflict resolutions will likely require a new commit, separate from the merge commit. This is because using a merge commit to store conflict resolutions can lead to those conflict resolutions being lost if merge commits are rebased (--rebase-merges).

Trapov commented Jan 20, 2018

View reviewed changes

Trapov changed the title ~~#15-Issue-Feature: Clipboard interaction was added~~ #15: Clipboard interaction was added Mar 16, 2018

Trapov changed the title ~~#15: Clipboard interaction was added~~ #15: Clipboard functions/constants were added May 22, 2018

Clipboard functions were added. #15

4fd9fa5

prasannavl reviewed May 22, 2018

View reviewed changes

Trapov mentioned this pull request May 22, 2018

Clipboard functions were added #15 #19

Merged

Trapov changed the title ~~#15: Clipboard functions/constants were added~~ Clipboard helpers were added #15 May 22, 2018

dmitriy afonin and others added 2 commits May 22, 2018 20:35

Merged with master

2743694

Clipboard helpers were fixed #15

f3640b8

TryGetUnicodeTextFromClipboard renamed into TryGetClipboardUnicodeText …

bdc1551

…#15

CloseClipboard return value is now handled #15

1d3fdbe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clipboard helpers were added #15 #16

Clipboard helpers were added #15 #16

Trapov commented Jan 17, 2018 •

edited

Loading

Trapov commented Jan 20, 2018

prasannavl commented Jan 20, 2018

Trapov left a comment

Trapov Jan 20, 2018

Trapov Jan 20, 2018

Trapov Jan 20, 2018

Trapov Jan 20, 2018

Trapov commented May 21, 2018

prasannavl commented May 21, 2018 •

edited

Loading

Trapov commented May 21, 2018 •

edited

Loading

Trapov commented May 22, 2018

prasannavl left a comment

prasannavl May 22, 2018 •

edited

Loading

prasannavl left a comment •

edited

Loading

prasannavl May 22, 2018

prasannavl May 22, 2018

prasannavl May 22, 2018

Trapov Jul 30, 2018

prasannavl May 22, 2018

Trapov commented May 22, 2018

prasannavl commented May 22, 2018 •

edited

Loading

Trapov commented May 22, 2018

prasannavl commented May 22, 2018

Trapov commented May 23, 2018

prasannavl commented May 23, 2018 •

edited

Loading

Trapov commented May 23, 2018

Trapov commented May 23, 2018 •

edited

Loading

Trapov commented Aug 14, 2018

BinToss commented Sep 21, 2021


		var result = User32Methods.SetClipboardData((uint)ClipboardFormat.CF_TEXT, allocatedMemory);

		if (result == IntPtr.Zero)

Clipboard helpers were added #15 #16

Are you sure you want to change the base?

Clipboard helpers were added #15 #16

Conversation

Trapov commented Jan 17, 2018 • edited Loading

will update my pull request in the future, please don't merge yet. created it for the sake of knowing what im doing

22.06.18 Update:

Trapov commented Jan 20, 2018

prasannavl commented Jan 20, 2018

Trapov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trapov commented May 21, 2018

prasannavl commented May 21, 2018 • edited Loading

Trapov commented May 21, 2018 • edited Loading

Trapov commented May 22, 2018

prasannavl left a comment

Choose a reason for hiding this comment

prasannavl May 22, 2018 • edited Loading

Choose a reason for hiding this comment

prasannavl left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trapov commented May 22, 2018

prasannavl commented May 22, 2018 • edited Loading

Trapov commented May 22, 2018

prasannavl commented May 22, 2018

Trapov commented May 23, 2018

prasannavl commented May 23, 2018 • edited Loading

Trapov commented May 23, 2018

Trapov commented May 23, 2018 • edited Loading

Trapov commented Aug 14, 2018

BinToss commented Sep 21, 2021

Trapov commented Jan 17, 2018 •

edited

Loading

prasannavl commented May 21, 2018 •

edited

Loading

Trapov commented May 21, 2018 •

edited

Loading

prasannavl May 22, 2018 •

edited

Loading

prasannavl left a comment •

edited

Loading

prasannavl commented May 22, 2018 •

edited

Loading

prasannavl commented May 23, 2018 •

edited

Loading

Trapov commented May 23, 2018 •

edited

Loading