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

Synchronous socket #34

Merged
merged 8 commits into from
Apr 5, 2024
Merged

Synchronous socket #34

merged 8 commits into from
Apr 5, 2024

Conversation

jmercouris
Copy link
Member

This adds A LOT of functionality to cl-electron. It particularly makes it possible for us to do flow-control within input events.

Note, there are not that many changes, there is only one large change that renames send-message-interface.

Copy link
Member

@aadcg aadcg left a comment

Choose a reason for hiding this comment

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

I understand the ideas behind the changes but I think it requires some polishing.

Commit b6530fa seems unrelated to this PR so I'd suggest moving it into a separate one.

Keep in mind the length of the summary commit message.

I've tested this branch on macOS, rebased on top of #35, and the electron-window-demo doesn't seem to be working as intended. Note that at this point, all work must be test on macOS too.

source/core.lisp Show resolved Hide resolved
source/server.js Show resolved Hide resolved
@jmercouris
Copy link
Member Author

I will be testing on macOS tomorrow to ensure that this works properly. I will also rebase and tweak #35 as necessary in conjunction with this pull request.

@jmercouris jmercouris force-pushed the synchronous-socket branch 2 times, most recently from 203b935 to abf141b Compare April 5, 2024 02:38
@jmercouris
Copy link
Member Author

Works on macOS! Now, I do have a branch that updates Nyxt to work with this latest set of changes. We'll have to update the submodule for cl-electron when we merge that branch.

Additionally, fix library path for cl-electron.
Create a class that uses execFileSync to synchronously read
information from a socket.

execFileSync calls query.js with some data. query.js will write this
data to the socket and read the response. query.js will then spit out
this data to stdout, from where it can be read by
execFileSync. Inspired by the Node.js sync-socket library.

server.js: Optionally specify a timeout.

Allow dynamic definition of the query.js script path.

This is useful for when server.js is moved to a tmp file for the
creation of protocols. The SynchronousSocket class must be able to
have a path to the module to execute it.
loop-connect-p predicate.

This predicate is useful for things like the SynchronousSocket. After
the process dies, we need the passive (listening) socket from Lisp to
keep accepting new connections. Thus, we need to loop
iolib:with-accept-connection to maintain a channel for new
SynchronousSocket requests.
control.

With this new interface, register-before-input-event allows the Lisp
side of cl-electron to control whether event.preventDefault() will be
invoked, and thus whether an event WILL/WILL NOT propagate throughout
the renderer. For example, if we want to intercept C-a in Lisp, we can
return "t" from our callback in register-before-input-event, and we'll
be able to avoid C-a being handled by the renderer.
Demonstrate the bubble handling control of the new
register-before-input-event function.
Additionally, create `(message ((remote-object remote-object)) ...)` to
avoid having to write `(message (interface x) ...)` everywhere in the codebase.
Allow the user to interactively handle cases where the
socket/interface already exists.
@jmercouris jmercouris merged commit 2c3b637 into master Apr 5, 2024
@jmercouris
Copy link
Member Author

The follow-up pull request is here: atlas-engineer/nyxt#3381

@jmercouris
Copy link
Member Author

Please also note that I will be working on a version of this change set that uses a C extension for Node.js to do synchronous socket reads rather than our sync-socket method. However, this does unblock us to do proper input handling!

@jmercouris jmercouris deleted the synchronous-socket branch April 5, 2024 03:33
aadcg added a commit that referenced this pull request Apr 17, 2024
Commit 099bcf8 introduced the dependency, but the change wasn't made in #34.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants