QuickInfo should show the captures for lambdas and local functions #23970

jcouv · 2017-12-29T18:21:10Z

Customer scenario

Performance-sensitive code should beware of captured variables when using lambdas. But it's not always easy to tell what variables are captured and will cause a closure to be created.
This feature adds a "Captures" section the QuickInfo bubble:

Bugs this fixes

Fixes #23307

Workarounds, if any

Figures out the captures mentally.

Risk

Performance impact

Is this a regression from a previous update?

No.

Note for the record: this was also implemented in monodevelop (mono/monodevelop#4569)

alrz · 2017-12-29T23:18:40Z

This is awesome. just two suggestions: it would be nice if we underline function name and lambda arrow as a visual clue if there is any capture (so you don't need to hover over any function/lambda to make sure it's not capturing, for example). also, it might make sense to group parameters and locals (if that's helpful at all). #WontFix

jcouv · 2017-12-30T17:20:33Z

I think I'll need Ali's PR (#23428) to get scenarios with multiple capture sets to work (N(() =$$> { M(); }, () => { i++; });).
I'm also having a few tests fail on trying to do the region analysis on a MethodBodySemanticModel (throws "not supported" exception). Still investigating.

Update:
Rebased to 15.7 branch and using the newly added CapturedInside API.
Still have an issue with MethodBodySemanticModel. #Resolved

sharwell · 2017-12-30T22:17:54Z

@jcouv I'm a big fan of this and a few related items. I've marked this as a work in progress (in a way the bot understands) and will follow up in the original issue. #Resolved

MaStr11 · 2017-12-31T00:50:25Z

src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs

@@ -587,6 +587,11 @@ public bool IsBindableToken(SyntaxToken token)
                return true;
            }

+            if (token.IsKind(SyntaxKind.EqualsGreaterThanToken))


I did something comparable while adding support for QuickInfo for Linq query syntax here in PR #23049. I was wondering whether this might have unwanted side effects (In my case it is even worse, because it is a CommaToken). If you experience something unexpected it would be nice to get me informed. #Resolved

I'd say @MaStr11's concerns are valid here, but maybe it's fine...? #Resolved

@[email protected] Can you clarify the concern?
The method is only used in GetSemanticInfoAtPositionAsync which now supports finding a symbol when the position is in =>.
GetSemanticInfoAtPositionAsync itself is used in GoToDefinition, Peek and SymbolFinder (which is used in numerous places, like FindUsages, FindReferences, ...).

In reply to: 161932149 [](ancestors = 161932149)

jasonmalinowski · 2018-01-03T00:03:11Z

@jcouv I'll ignore the actual code for review until the WIP is removed, but this should probably target master and not be carried along with features/compiler. This is an IDE-only feature that can be shipped independent of new language features. #Resolved

jcouv · 2018-01-03T02:31:23Z

@jasonmalinowski This cannot go into master, as it depends on a new compiler API (CapturesInside) which was recently added to features/compiler for 15.7.

There are two test regressions, which I'm still investigating. When I ask for a semantic model for the some lambda syntaxes, I get back a MethodBodySemanticModel, which does not support flow analysis. I'm not sure why I'm getting that semantic model.

Also, @sharwell asked for IDE team to discuss this. It's probably better to hold-off review until design is approved. #Resolved

CyrusNajmabadi · 2018-01-03T15:50:28Z

captures @this? Why the @? #Resolved

CyrusNajmabadi · 2018-01-03T15:51:06Z

src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs

+        void local() { i++; this.M(); }
+    }
+}",
+                Captures($"\r\n{WorkspacesResources.Captures_colon} @this, i"));


@this? #Closed

CyrusNajmabadi · 2018-01-03T15:52:06Z

src/EditorFeatures/TestUtilities/QuickInfo/AbstractSemanticQuickInfoSourceTests.cs

+                AssertTextAndClassifications(expectedText, expectedClassifications: null, actualContent: ((QuickInfoDisplayDeferredContent)content).CapturesText);
+            };
+        }
+


consider removing all the curlies here. Catures() => content => ... #Closed

Felt harder to visually parse. Also, it's inconsistent with neighboring code. #Closed

Also, it's inconsistent with neighboring code.

Fix the neighboring code :)

(But if you feel like it's actually harder to parse, that's a totally legitimate reason to not do it). #Resolved

CyrusNajmabadi · 2018-01-03T15:52:40Z

...rvices/SymbolDisplayService/AbstractSymbolDisplayService.AbstractSymbolDescriptionBuilder.cs

+                        parts.AddRange(ToMinimalDisplayParts(captured, s_formatForCaptures));
+                        first = false;
+                    }
+                    AddToGroup(SymbolDescriptionGroups.Captures, parts);


newlines after } (above as well). #Resolved

CyrusNajmabadi · 2018-01-03T15:53:24Z

src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs

+        public bool IdentifiesLambda(SyntaxToken token)
+        {
+            return token.IsKind(SyntaxKind.EqualsGreaterThanToken);
+        }


nit: use => #Closed

CyrusNajmabadi · 2018-01-03T15:53:45Z

src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs

@@ -341,6 +341,8 @@ internal interface ISyntaxFactsService : ILanguageService
        Location GetDeconstructionReferenceLocation(SyntaxNode node);

        SyntaxToken? GetDeclarationIdentifierIfOverride(SyntaxToken token);
+
+        bool IdentifiesLambda(SyntaxToken token);


on the fence if this is the right location for this method. #Resolved

I'm planning to leave as-is unless I hear some better suggestion ;-)

In reply to: 159457900 [](ancestors = 159457900)

CyrusNajmabadi · 2018-01-03T15:53:56Z

src/Workspaces/Core/Portable/Shared/Extensions/SemanticModelExtensions.cs

+                allSymbols = semanticModel.GetSymbolInfo(bindableParent, cancellationToken)
+                                              .GetBestOrAllSymbols()
+                                              .WhereAsArray(s => !s.Equals(declaredSymbol))
+                                              .SelectAsArray(s => MapSymbol(s, type));


align dots. #Closed

CyrusNajmabadi

LGTM (with changes + vb support).

CyrusNajmabadi · 2018-01-06T02:03:38Z

src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.cs

@@ -85,7 +85,7 @@ private static bool IsEscapable(SymbolDisplayPartKind kind)
        private static string EscapeIdentifier(string identifier)
        {
            var kind = SyntaxFacts.GetKeywordKind(identifier);
-            return kind == SyntaxKind.None
+            return kind == SyntaxKind.None || kind == SyntaxKind.ThisKeyword


This looks wrong. this means we would not properly escape things in something like Foo(int @this) anymore. #Resolved

CyrusNajmabadi · 2018-01-06T02:04:04Z

src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs

@@ -4934,18 +4954,18 @@ public static void Main()
            Assert.Empty(flowAnalysis.Captured);
            Assert.Empty(flowAnalysis.CapturedInside);
            Assert.Empty(flowAnalysis.CapturedOutside);
-            Assert.Equal("MyClass @this", flowAnalysis.DataFlowsIn.Single().ToTestDisplayString());
+            Assert.Equal("MyClass this", flowAnalysis.DataFlowsIn.Single().ToTestDisplayString());


yup. these are all bad changes. #Resolved

No, I think those changes are correct. Added some explicit tests with "this" and "@this" to illustrate the point.

In reply to: 160014222 [](ancestors = 160014222)

Gotcha! #Closed

CyrusNajmabadi · 2018-01-19T23:41:48Z

...es/Core/Implementation/IntelliSense/QuickInfo/Providers/AbstractSemanticQuickInfoProvider.cs

+            var syntaxFacts = document.Project.LanguageServices.GetService<ISyntaxFactsService>();
+
+            ImmutableArray<ISymbol> symbols;
+            if (syntaxFacts.IdentifiesLambda(token))


yeah. i would just have IdentifiesLambda just pulled into this type as an abstract method. #Resolved

Done :-) #Resolved

CyrusNajmabadi · 2018-01-19T23:42:26Z

src/EditorFeatures/VisualBasic/QuickInfo/SemanticQuickInfoProvider.vb

@@ -92,6 +92,13 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.QuickInfo
            Return Await MyBase.BuildContentAsync(document, token, cancellationToken).ConfigureAwait(False)
        End Function

+        ''' <summary>
+        ''' Given a Sub or Function token for a lambda, returns the syntax for the whole lambda
+        ''' </summary>


do we have tests for single/multiline sub/function lambdas? #Resolved

Yes, I have all four cases #Resolved

jcouv · 2018-01-23T17:57:49Z

Fixed a couple of test regressions.
One case required a small code change: when invoking symbol completion on a local, we will build a description for that local using speculation, but we'll only display the "main description" portion of that description (not the captures).

@dotnet/roslyn-ide for review. Thanks #Resolved

jcouv · 2018-01-24T17:17:41Z

@dotnet/roslyn-ide for review. All the feedback so far was addressed. Thanks #Resolved

CyrusNajmabadi · 2018-01-24T19:53:23Z

src/EditorFeatures/CSharp/QuickInfo/SemanticQuickInfoProvider.cs

+                && token.Parent.IsKind(SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.SimpleLambdaExpression))
+            {
+                // () =>
+            }


not sure how i feel about this style of no-body then falling out to common code :) #Resolved

CyrusNajmabadi · 2018-01-24T19:55:28Z

src/EditorFeatures/VisualBasic/QuickInfo/SemanticQuickInfoProvider.vb

+        ''' If the token is a 'Sub' or 'Function' in a lambda, returns the syntax for the whole lambda
+        ''' </summary>
+        Protected Overrides Function GetBindableNodeForTokenIndicatingLambda(token As SyntaxToken, <Out> ByRef found As SyntaxNode) As Boolean
+            If token.IsKind(SyntaxKind.SubKeyword, SyntaxKind.FunctionKeyword) AndAlso token.Parent.IsKind(SyntaxKind.SubLambdaHeader, SyntaxKind.FunctionLambdaHeader) Then


to verify: this works for single and multi-line VB lambdas? #Resolved

Yes. Here's what the syntax tree looks like (for Sub):

Multi-line Sub

Single-line Sub

Also, I have tests for both multi-line and single-line:

<Fact, Trait(Traits.Feature, Traits.Features.QuickInfo)> <WorkItem(23307, "https://github.com/dotnet/roslyn/issues/23307")> Public Async Function TestQuickInfoCaptures() As Task Await TestAsync(" Class C Sub M(x As Integer) Dim a As System.Action = Sub$$() x = x + 1 End Sub End Sub End Class ", Captures($"{vbCrLf}{WorkspacesResources.Variables_captured_colon} x")) End Function <Fact, Trait(Traits.Feature, Traits.Features.QuickInfo)> <WorkItem(23307, "https://github.com/dotnet/roslyn/issues/23307")> Public Async Function TestQuickInfoCaptures2() As Task Await TestAsync(" Class C Sub M(x As Integer) Dim a As System.Action = S$$ub() x = x + 1 End Sub End Class ", Captures($"{vbCrLf}{WorkspacesResources.Variables_captured_colon} x")) End Function ``` #Resolved

CyrusNajmabadi · 2018-01-24T19:57:05Z

src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs

            }
        }

-        private async Task TestWithOptionsAsync(TestWorkspace workspace, params Action<object>[] expectedResults)
+        private async Task TestWithOptionsAsync(TestWorkspace workspace, bool skipSpeculative, params Action<object>[] expectedResults)


i'm a little confused as to why we would still need this. Since quick-info is no longer using any speculative semantic models, there should be no effect of trying with/without speculation. Indeed, i would be fine if the CanUseSpeculativeSemanticModelAsync part of the test was removed entirely (though keeping it would be fine.) #Resolved

Sorry about that. It was a left-over I should have cleaned up. Thanks #Resolved

CyrusNajmabadi · 2018-01-26T02:50:33Z

...rvices/SymbolDisplayService/AbstractSymbolDisplayService.AbstractSymbolDescriptionBuilder.cs

+                var semanticModel = GetSemanticModel(syntax.SyntaxTree);
+                if(semanticModel.IsSpeculativeSemanticModel)
+                {
+                    // In the context of symbol completion, it is possible we'll make a description for a symbol while speculating (but it won't be displayed)


lolwut? we make a description, but don't display it?

Also, if that's the case, why add the group at all? why not just insta bail?

Finally, if you are going to check IsSpeculativeSemanticModel, it's worth calling that that's because data flow doens't work with SpeculativeSemanticModels, so you need to avoid the code below this check. #Resolved

I'll clarify the comment. When you complete a local symbol, we make a description (with many parts: main description, exceptions, captures, etc), but only the main description seems to be displayed.
The reason I put captures here anyways, is in case I'm wrong and missed some scenario. Captures listing "?" is less ambiguous/misleading than displaying no captures at all.

Thanks! #Pending

jasonmalinowski

Looks good, but I'd expect that the this/Me captures in quick info are tagged as keywords, not parameters to be consistent with how the IDE would classify it. I'd expect that to be a one-line fix...somewhere?

jasonmalinowski · 2018-01-26T23:04:11Z

...es/Core/Implementation/IntelliSense/QuickInfo/Providers/AbstractSemanticQuickInfoProvider.cs

@@ -266,16 +273,28 @@ internal abstract partial class AbstractSemanticQuickInfoProvider : AbstractQuic
            return CreateDocumentationCommentDeferredContent(null);
        }

+        protected abstract bool GetBindableNodeForTokenIndicatingLambda(SyntaxToken token, out SyntaxNode found);


To check: this isn't being used by TypeScript or F# that this is a breaking change to derivers? I don't see how they could , but want to confirm.

I checked with F# folks (Will and Brett) and they do not reference AbstractSemanticQuickInfoProvider from Roslyn (they only depend on some interfaces for quick info provider.

Same thing with TypeScript (Mine Starks), which only depends on interfaces, but not this abstract type.

jasonmalinowski · 2018-01-26T23:07:37Z

...rvices/SymbolDisplayService/AbstractSymbolDisplayService.AbstractSymbolDescriptionBuilder.cs

+                        }
+
+                        parts.AddRange(Space(count: 1));
+                        parts.AddRange(ToMinimalDisplayParts(captured, s_formatForCaptures));


I guess I'd expect the magic this parameter to be called a keyword, not a parameter. Our symbol model indeed calls it parameter; users don't.

jcouv · 2018-01-26T23:50:41Z

@jasonmalinowski Good catch. I fixed the compiler tests to check part kinds too. this and Me are now keywords.
This does result in nicer UI, and the code is also simpler that way.

Thanks!

@dotnet/roslyn-compiler for review. The compiler portion of this PR is a small change with how we display parts for this versus @this (and Me versus [Me]).

AlekseyTs

Compiler changes LGTM (iteration 12)

cston

Compiler changes LGTM.

jasonmalinowski · 2018-01-27T00:09:06Z

This does result in nicer UI, and the code is also simpler that way.

Love it when that happens!

jcouv · 2018-01-27T00:16:44Z

@VSadov raised a good question with nested scopes. Here's what I observe with nesting:

The inner local looks good:

But the outer local says that it captures j, ~~which is weird...~~ Vlad said that sounds right.

VSadov

LGTM

jcouv added the Area-IDE label Dec 29, 2017

jcouv added this to the 15.6 milestone Dec 29, 2017

jcouv self-assigned this Dec 29, 2017

jcouv requested a review from a team as a code owner December 29, 2017 18:21

jcouv added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Dec 29, 2017

jcouv mentioned this pull request Dec 29, 2017

Show captures when hovering over local functions and lambda arrow #23307

Closed

jcouv force-pushed the show-captures branch from 4b46a1c to c20b1bf Compare December 29, 2017 18:31

List captured variables in QuickInfo

b0eff3c

jcouv force-pushed the show-captures branch from 0f6d53e to b0eff3c Compare December 30, 2017 19:38

jcouv changed the base branch from master to features/compiler December 30, 2017 19:38

sharwell changed the title ~~QuickInfo should show the captures for lambdas and local functions~~ [WIP] QuickInfo should show the captures for lambdas and local functions Dec 30, 2017

MaStr11 reviewed Dec 31, 2017

View reviewed changes

CyrusNajmabadi reviewed Jan 3, 2018

View reviewed changes

CyrusNajmabadi approved these changes Jan 3, 2018

View reviewed changes

Remove @ on this. Test delegate case. Fix two test regressions.

3fa5ed1

jcouv force-pushed the show-captures branch from 14d5778 to 3fa5ed1 Compare January 5, 2018 23:12

WIP on test for symbol display

e031014

CyrusNajmabadi reviewed Jan 6, 2018

View reviewed changes

CyrusNajmabadi reviewed Jan 19, 2018

View reviewed changes

Avoid speculation. Refactoring IdentifiesLambda

ee17350

jcouv added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jan 22, 2018

Fixing test regressions

9f0ff23

jcouv force-pushed the show-captures branch from 3a72751 to 9f0ff23 Compare January 23, 2018 06:36

jcouv removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jan 23, 2018

CyrusNajmabadi reviewed Jan 24, 2018

View reviewed changes

Joe4evr mentioned this pull request Jan 25, 2018

Proposal: modifier (perhaps "static") for local functions, to explicitly disallow captured state semantics dotnet/csharplang#1277

Closed

Remove skipSpeculative option from tests

88532b9

CyrusNajmabadi reviewed Jan 26, 2018

View reviewed changes

CyrusNajmabadi approved these changes Jan 26, 2018

View reviewed changes

Clarify comment

614db1b

jasonmalinowski approved these changes Jan 26, 2018

View reviewed changes

This/Me keywords should display with correct part kind

262f71c

AlekseyTs approved these changes Jan 27, 2018

View reviewed changes

cston approved these changes Jan 27, 2018

View reviewed changes

VSadov approved these changes Jan 27, 2018

View reviewed changes

jcouv merged commit ccdc45c into dotnet:features/compiler Jan 27, 2018

jcouv deleted the show-captures branch January 27, 2018 00:32

MaStr11 mentioned this pull request Mar 1, 2018

Support GotoDefinition in LINQ query syntax #23049

Merged

MaStr11 mentioned this pull request Mar 19, 2018

Show captured variables in quickinfo for query syntax #25593

Closed

Joe4evr mentioned this pull request May 20, 2020

Proposal: Use static keyword for lambdas to disallow capturing locals and parameters (VS 16.8, .NET 5) dotnet/csharplang#275

Open

5 tasks

QuickInfo should show the captures for lambdas and local functions #23970

QuickInfo should show the captures for lambdas and local functions #23970

Conversation

jcouv commented Dec 29, 2017 • edited Loading

Customer scenario

Bugs this fixes

Workarounds, if any

Risk

Performance impact

Is this a regression from a previous update?

alrz commented Dec 29, 2017 • edited by jcouv Loading

jcouv commented Dec 30, 2017 • edited Loading

sharwell commented Dec 30, 2017 • edited by jcouv Loading

MaStr11 Dec 31, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

jasonmalinowski Jan 17, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 19, 2018

Choose a reason for hiding this comment

jasonmalinowski commented Jan 3, 2018 • edited by jcouv Loading

jcouv commented Jan 3, 2018 • edited Loading

CyrusNajmabadi commented Jan 3, 2018 • edited by jcouv Loading

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 5, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 6, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 19, 2018

Choose a reason for hiding this comment

CyrusNajmabadi Jan 3, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

CyrusNajmabadi Jan 6, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 6, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 6, 2018

Choose a reason for hiding this comment

CyrusNajmabadi Jan 19, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 19, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 19, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

jcouv commented Jan 23, 2018 • edited Loading

jcouv commented Jan 24, 2018 • edited Loading

CyrusNajmabadi Jan 24, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 24, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 24, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Jan 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jan 26, 2018 • edited Loading

Choose a reason for hiding this comment

jasonmalinowski left a comment

Choose a reason for hiding this comment

jasonmalinowski Jan 26, 2018

Choose a reason for hiding this comment

jcouv Jan 27, 2018

Choose a reason for hiding this comment

jcouv commented Dec 29, 2017 •

edited

Loading

alrz commented Dec 29, 2017 •

edited by jcouv

Loading

jcouv commented Dec 30, 2017 •

edited

Loading

sharwell commented Dec 30, 2017 •

edited by jcouv

Loading

MaStr11 Dec 31, 2017 •

edited by jcouv

Loading

jasonmalinowski Jan 17, 2018 •

edited by jcouv

Loading

jasonmalinowski commented Jan 3, 2018 •

edited by jcouv

Loading

jcouv commented Jan 3, 2018 •

edited

Loading

CyrusNajmabadi commented Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

jcouv Jan 5, 2018 •

edited

Loading

CyrusNajmabadi Jan 6, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 3, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 6, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 6, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 19, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 19, 2018 •

edited by jcouv

Loading

jcouv Jan 20, 2018 •

edited

Loading

CyrusNajmabadi Jan 19, 2018 •

edited by jcouv

Loading

jcouv Jan 20, 2018 •

edited

Loading

jcouv commented Jan 23, 2018 •

edited

Loading

jcouv commented Jan 24, 2018 •

edited

Loading

CyrusNajmabadi Jan 24, 2018 •

edited by jcouv

Loading

CyrusNajmabadi Jan 24, 2018 •

edited by jcouv

Loading

jcouv Jan 25, 2018 •

edited

Loading

CyrusNajmabadi Jan 24, 2018 •

edited by jcouv

Loading

jcouv Jan 25, 2018 •

edited

Loading

CyrusNajmabadi Jan 26, 2018 •

edited by jcouv

Loading

jcouv Jan 26, 2018 •

edited

Loading

jcouv commented Jan 27, 2018 •

edited

Loading