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

Inline includes and mark them as jsNode where necessary #612

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

b-studios
Copy link
Collaborator

This ought to address #611

@@ -24,26 +24,26 @@ def console[R] { program: () => R / Console }: R = {

namespace js {

extern js """
extern jsNode """
const readline = require('node:readline');
"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Specific to node js and doesn't work in the webbrowser, for now simply disable

extern pure def errorNumber(errno: Int): Int =
js "errorNumber(${errno})"
jsNode "errorNumber(${errno})"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using os

"""

extern async def open(path: String, mode: Mode): Int =
js "$effekt.callcc(callback => open(${path}, ${mode}, callback))"
jsNode "$effekt.callcc(callback => open(${path}, ${mode}, callback))"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using fs which is only available in node, but not web

@@ -4,7 +4,7 @@ import bytes
import io

namespace js {
extern js """
extern jsNode """
const net = require('node:net');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

node:net is only available on node

@b-studios
Copy link
Collaborator Author

@phischu I tried to establish the principle that each file only includes its own forward declarations. To this end, and to deduplicate the forward declaration of c_timer_start, I introduced c_yield which is just c_timer_start(0, _).

However, while this is a tiny function that just forwards and seems useless, semantically I think it makes sense to distinguish the two.

@b-studios b-studios marked this pull request as ready for review September 26, 2024 09:57
@b-studios
Copy link
Collaborator Author

b-studios commented Sep 26, 2024

We can merge this PR in my opinion, the moment CI passes. Merging this quickly is important, since it blocks #536.

@b-studios b-studios merged commit 52be3e5 into master Sep 26, 2024
2 checks passed
@b-studios b-studios deleted the refactor/inline-includes branch September 26, 2024 10:34
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