-
-
Notifications
You must be signed in to change notification settings - Fork 396
Fix copy to clipboard STA thread issue & Support retry for copy & Async build-in shortcut model & Fix build shortcuts text replace issue & Fix startup window hide issue #3314
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
Changes from 3 commits
9f8e829
d7a29e5
e5ad777
04f6d14
17ecebf
b034f0d
c98c419
338c8e2
ff2cb96
817d247
e0a651c
a49c465
9897213
123802b
246b1fb
3b30209
c1602b0
8168d8a
3fc69ab
0314a9f
ff9f1e1
8db8d5c
f742460
ae7cea1
3be057d
2f27fa6
1b412da
370be2a
0c592b1
d08deab
e55f79e
0eeaaea
bd1da94
882b7dc
ba5a76a
632dbeb
8f23870
9dc9cde
5990820
72c2d7f
9242f8e
0e70519
290750a
a780719
00c496f
5002449
396cd69
893ec48
a22306a
8e15e7f
67ed7a1
14cdf8a
be0c1ad
43826cb
5441fc4
20b5c47
9eb7c9c
b9e04ff
236abc8
96bb07d
56a3551
4ca8c60
ab95342
186cd2c
4aa9a34
e9a0868
854e61d
7961615
e5b5ec5
30a840a
b918d00
adde491
6859232
c78a558
c0b792c
7669bba
ccfc6fd
282f486
a143cb4
1ffbbca
836d09e
6bb638b
dd467e2
023f511
74f274a
7800540
15e6754
7a879c9
fa49c08
ebc81c0
593a360
4283ef5
0347703
ebaffc6
d446824
cb673f6
96eb673
68e1aad
1cec01c
d023ecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,4 +16,7 @@ WM_KEYUP | |
WM_SYSKEYDOWN | ||
WM_SYSKEYUP | ||
|
||
EnumWindows | ||
EnumWindows | ||
|
||
OleInitialize | ||
OleUninitialize |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -248,7 +248,7 @@ public SearchPrecisionScore QuerySearchPrecision | |||||
[JsonIgnore] | ||||||
public ObservableCollection<BuiltinShortcutModel> BuiltinShortcuts { get; set; } = new() | ||||||
{ | ||||||
new BuiltinShortcutModel("{clipboard}", "shortcut_clipboard_description", Clipboard.GetText), | ||||||
new BuiltinShortcutModel("{clipboard}", "shortcut_clipboard_description", () => Win32Helper.StartSTATaskAsync(Clipboard.GetText).Result), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using .Result to synchronously wait on an async STA task may lead to deadlocks or UI blocking; consider refactoring to await the result asynchronously if possible.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it causes build issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually how do you think about making the BuiltinShortcutModel supports Async operation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah, it is a good idea There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
new BuiltinShortcutModel("{active_explorer_path}", "shortcut_active_explorer_path", FileExplorerHelper.GetActiveExplorerPath) | ||||||
}; | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to use GetAwaiter().GetResult().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe async void is better? I am not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think either is ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cannot work here, so I choose to keep as it was
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by not work here? It is almost semantically equal to Task.Result, except that the exception is handled in a different way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing
() => Win32Helper.StartSTATaskAsync(Clipboard.GetText).Result
toWin32Helper.StartSTATaskAsync(Clipboard.GetText).GetAwaiter().GetResult
cannot work, the result is always emptyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes no sense to me. They should perform the same...
https://stackoverflow.com/questions/17284517/is-task-result-the-same-as-getawaiter-getresult
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will test it.