Syntax highlighting for JSM #5250
Conversation
| private static readonly Lazy<Dictionary<string, Regex>> _regexes = new Lazy<Dictionary<string, Regex>>(ParseRegexes); | ||
| #pragma warning disable VSTHRD012 // Provide JoinableTaskFactory where allowed | ||
| private static readonly AsyncLazy<Dictionary<string, Regex>> _regexByExtension = new AsyncLazy<Dictionary<string, Regex>>(ParseRegexesAsync); | ||
| #pragma warning restore VSTHRD012 // Provide JoinableTaskFactory where allowed |
There was a problem hiding this comment.
@sharwell could you verify that this diagnostic can be skipped in this scenario? The async operation only awaits disk IO operations so as I see it there's no opportunity for deadlock.
Getting a JTF in this static initialiser is awkward. We'd likely need a new Initialise instance method, or convert the ctor to a static factory.
Codecov Report
@@ Coverage Diff @@
## master #5250 +/- ##
==========================================
- Coverage 35.95% 35.95% -0.01%
==========================================
Files 587 587
Lines 45179 45179
Branches 6205 6205
==========================================
- Hits 16243 16242 -1
Misses 28181 28181
- Partials 755 756 +1 |
| var regex = GetRegexForExtension(Path.GetExtension(file.Name)); | ||
| var extension = Path.GetExtension(file.Name); | ||
|
|
||
| var regexByExtension = await _regexByExtension.GetValueAsync(cancellationToken); |
There was a problem hiding this comment.
Previous code parsed file synchronously during the first call to Lazy<Dictionary<...>>.Value. That happend during show of FormCommit, which would block during IO, reading the file.
This change uses AsyncLazy which allows the factory function to be async, which allows async IO.
There was a problem hiding this comment.
we can probably initiate this process when we start the app, and thus won't need such complex structure, can probably replace it with ConcurrentDictionary...
thoughts?
There was a problem hiding this comment.
Could do. It's not a big saving here anyway as the file is small and fast to read. On the order of a few milliseconds in my testing with an SSD. I mostly wanted to experiment a little more with vs-threading.
There was a problem hiding this comment.
I'm happy to drop the second commit from this PR.
There was a problem hiding this comment.
Keep It SynchronouS ? ;)
drewnoakes
force-pushed
the
fix-2155-jsm-syntax
branch
from
f8fadb4
to
c9837b3
Compare
drewnoakes
changed the title
|
Peeled the last commit off. |
|
LOL!!!
See now that's just going to confuse us stupid people. :D
Keep
It
Simple
Stupid
…On Tue, Jul 31, 2018 at 9:21 AM Drew Noakes ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In GitUI/AutoCompletion/CommitAutoCompleteProvider.cs
<#5250 (comment)>
:
> @@ -34,13 +37,15 @@ public CommitAutoCompleteProvider(GitModule module)
{
cancellationToken.ThrowIfCancellationRequested();
- var regex = GetRegexForExtension(Path.GetExtension(file.Name));
+ var extension = Path.GetExtension(file.Name);
+
+ var regexByExtension = await _regexByExtension.GetValueAsync(cancellationToken);
Keep It SynchronouS ? ;)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#5250 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADdhsdO3lltLd6T5URrV57AQCHyqFjg6ks5uMFnOgaJpZM4Vn87s>
.
|
|
Nice one :)))
…On Tue, Jul 31, 2018, 11:21 PM Drew Noakes ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In GitUI/AutoCompletion/CommitAutoCompleteProvider.cs
<#5250 (comment)>
:
> @@ -34,13 +37,15 @@ public CommitAutoCompleteProvider(GitModule module)
{
cancellationToken.ThrowIfCancellationRequested();
- var regex = GetRegexForExtension(Path.GetExtension(file.Name));
+ var extension = Path.GetExtension(file.Name);
+
+ var regexByExtension = await _regexByExtension.GetValueAsync(cancellationToken);
Keep It SynchronouS ? ;)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#5250 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEMyXp3y2fJ6IohX7Ut1hYdB2JVd4we-ks5uMFnOgaJpZM4Vn87s>
.
|
Fixes #2155
Changes proposed in this pull request:
.jsmfilesWhat did I do to test the code and ensure quality:
Has been tested on: