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

Conditional Region Support #268

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Conditional Region Support #268

merged 5 commits into from
Nov 1, 2024

Conversation

phschaad
Copy link
Collaborator

@phschaad phschaad commented Oct 11, 2024

Make sure the extension works with Conditoinal regions and Loop regions, to the extent they will be present / used in DaCe 1.0.

Relies on spcl/dace-webclient#180.

@phschaad phschaad changed the title Adapt to Diff / Conditionals Conditional Region Support Oct 14, 2024
@phschaad phschaad requested review from tbennun and removed request for tbennun October 14, 2024 07:28
@phschaad phschaad marked this pull request as draft October 14, 2024 13:21
@tbennun
Copy link
Contributor

tbennun commented Oct 14, 2024

Could we run that again? @phschaad

@phschaad phschaad marked this pull request as ready for review October 31, 2024 11:01
@phschaad phschaad requested a review from tbennun October 31, 2024 11:03
Copy link
Contributor

@tbennun tbennun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See question

tsconfig.json Outdated
@@ -1,10 +1,10 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "ES2022",
"target": "ESNext",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh dear why. Which feature necessitates this and is there good support in Electron?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good catch, I don't recall why I modified this. I have reverted it to ES2022, which is also what the webclient now depends on following this PR: spcl/dace-webclient#181

Note that we would in theory be fine with ESNext, as VSCode's version of Electron uses a version of the V8 engine which supports quite a few features that are not even in the most recent ECMA standard yet, but this is definitely the safer option.

@phschaad phschaad merged commit 38de0ba into main Nov 1, 2024
@phschaad phschaad deleted the diff_view branch November 1, 2024 16:24
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.

2 participants