-
Notifications
You must be signed in to change notification settings - Fork 323
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
Execute yjs from insight script #11965
base: develop
Are you sure you want to change the base?
Conversation
🧪 Storybook is successfully deployed!📊 Dashboard:
|
That's working:
What's in progress:
|
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.
Thanks Dmitry for finding a way to convert insight
offsets to ASTs!
const result = ctx.returnValue(frame) | ||
if (result && spanMap) { | ||
const key = Ast.nodeKey(ctx.charIndex, ctx.charEndIndex - ctx.charIndex) | ||
const asts = spanMap.nodes.get(key) |
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 see. Whenever we get an insight.on('return', ...)
event, we look appropriate asts
and update them. I am not sure how fast this is, but it certainly seems good enough for demonstration purposes.
} | ||
}, { | ||
expressions: true, | ||
statements: false, |
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 believe n = 42
would be a statement
- e.g. if you want to observe when "IDE nodes" are being assigned to, you may want to observe statements.
@@ -316,6 +316,11 @@ class MainModule(serverConfig: LanguageServerConfig, logLevel: Level) { | |||
RuntimeOptions.JOB_PARALLELISM, | |||
Runtime.getRuntime.availableProcessors().toString | |||
) | |||
if (System.getProperty("enso.dev.inspectPort") != null) { | |||
val inspectPort = Integer.getInteger("enso.dev.inspectPort", 34567); |
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 don't think this is necessary. Just use regular enso --inspect
:
--inspect Start the Chrome inspector when
--run is used.
but yeah, I am not sure if it works in "language server" mode. I'd rather got it working then designing new enso.dev.inspectPort
property.
@@ -252,7 +254,7 @@ public Context build() { | |||
} | |||
|
|||
private static HostAccess allWithTypeMapping() { | |||
return HostAccess.newBuilder() | |||
return HostAccess.newBuilder(HostAccess.EXPLICIT) |
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.
Starting with EXPLICIT
and then allowing allPublicAccess
is strange. Doesn't it create HostAccess.PUBLIC
?
@@ -165,6 +166,7 @@ public Context build() { | |||
Context.newBuilder() | |||
.allowExperimentalOptions(true) | |||
.allowAllAccess(true) | |||
.allowIO(IOAccess.ALL) |
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 is this good for? I don't think our JavaScript is performing any I/O by itself! The JavaScript code calls into the ydoc-polyfill
Java classes and only then the Java classes do some I/O - we shouldn't need a GraalVM permission for that.
Pull Request Description
Allow populating Yjs metadata with values provided by the insight script.
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.