Skip to content
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

fire event when a value is bound at the root of a script evaluation #7919

Merged
merged 2 commits into from
Dec 3, 2019

Conversation

brettfo
Copy link
Member

@brettfo brettfo commented Dec 2, 2019

Also adds a function to get runtime type and value given a (root) identifier's name.

This is to enable a hypothetical "watch" or "locals" window in notebooks.

@brettfo brettfo requested a review from KevinRansom December 2, 2019 22:34
@abelbraaksma
Copy link
Contributor

I may be OT here, but 'given an identifier's name' is not enough, as often the same names are in scope.

We'd need name and location in source to du that correctly.

Currently, in VS, the locals window works (it'll show multiple times the same name, but with different values), but hovering over a value doesn't. Neither the correct stack frame, nor the actual value belonging to the variable will be shown when names overlap.

@brettfo
Copy link
Member Author

brettfo commented Dec 3, 2019

If using this in a debugger then yes, you'll need the location to properly map an identifier to a variable, but in a script context where this is to be used the source location is essentially line 1, column 0. The added function could have been implemented as fsi.EvalExpression(variableName) because the return value of that function is a type and value (and some other stuff), and perhaps it should have been done that way; I'm open to either approach, but I went with what's in this PR because that's how the value of the special variable it is extracted after a submission here and I wanted to mirror that.

@KevinRansom
Copy link
Member

@abelbraaksma
the key is, this Api is retrieving top level values created in fsi, and so everything here is at the same scope, so existing values with the same name are hidden, and we currently have no scenarios for retrieving the hidden values.

So ...

> let a =
-     let a = 1
-     let b = 3
-     a * b;;
val a : int = 3

Essentially, the only retrievable item is the top level a.

val a : int = 3

if a had been assigned a different value prior to this code submission then it is no longer retrievable. And if a was defined twice in a single submission then it would generate a duplicate definition error.

> let a = 10
- let a =
-     let a = 1
-     let b = 3
-     a * b;;

  let a =
  ----^

stdin(21,5): error FS0037: Duplicate definition of value 'a'

I hope this helps.

@brettfo
Copy link
Member Author

brettfo commented Dec 3, 2019

After speaking with @KevinRansom and finding a better place to add the relevant event, I've decided to remove the function GetIdentifierValueType simply because that information can already be retrieved from interactive with the EvalExpression that I mentioned above.

The benefit is that this PR is now much simpler.

Thanks to @abelbraaksma and Kevin for the probing questions that led to a better implementation and design.

@brettfo brettfo merged commit 4978145 into dotnet:master Dec 3, 2019
@brettfo brettfo deleted the bound-values branch December 3, 2019 22:40
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
…otnet#7919)

* fire event when a value is bound at the root of a script evaluation

* simplify event notifying bound values in interactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants