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

👪 Realtime collaboration #1729

Open
wants to merge 47 commits into
base: main
Choose a base branch
from
Open

👪 Realtime collaboration #1729

wants to merge 47 commits into from

Conversation

pankgeorg
Copy link
Collaborator

@pankgeorg pankgeorg commented Dec 4, 2021

GIF 04-12-2021 21-51-21

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2021

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/fonsp/Pluto.jl", rev="local-state-sync-main")
julia> using Pluto

@pankgeorg pankgeorg changed the title Sync local state (PR to master) Sync local state (PR to main) Dec 4, 2021
@pankgeorg

This comment has been minimized.

@pankgeorg pankgeorg changed the title Sync local state (PR to main) Sync local state Dec 4, 2021
@fonsp
Copy link
Owner

fonsp commented Dec 6, 2021

What is time_arrow? It looks like we only assign to it, but never use the value, so we can remove it?

@pankgeorg
Copy link
Collaborator Author

What is time_arrow? It looks like we only assign to it, but never use the value, so we can remove it?

Yeap, should be good to go!

@fonsp fonsp changed the title Sync local state 👪 Live collaboration Dec 6, 2021
@fonsp fonsp changed the title 👪 Live collaboration 👪 Realtime collaboration Dec 6, 2021
@fonsp
Copy link
Owner

fonsp commented Dec 6, 2021

Can you run this with

Pluto.run(simulated_lag=1.0)

and see if it works?

@fonsp
Copy link
Owner

fonsp commented Jan 17, 2022

@pankgeorg How ready is this?

@pankgeorg
Copy link
Collaborator Author

pankgeorg commented Jan 20, 2022

@pankgeorg How ready is this?

When you have lag and two people are typing, it is possible to get to a state when you see something different than the other client, as a 'final' state of the document, as the two states don't converge to the same 'final' for some reason. I'm not sure if this is fixable by our current approach; I'll investigate more. It works like a charm when only one party is making edits though.

Update: But maybe the last update fixes that too
Update 2: It does fix the convergence issue but it's much more unstable because older changes may be reapplied before convergence. (you write 'using Dates' and you see some of it disappear and reappear later)

@fonsp
Copy link
Owner

fonsp commented Jan 20, 2022

@pankgeorg
Copy link
Collaborator Author

pankgeorg commented Jan 27, 2022

Status: works pretty well with 7 connected clients! Had to refresh once because re-application of state failed (server was ok though)
Issue: When two people edit the same cell, the cursor goes back to the first line.
Options: keep your cursor at the same position as before (if it exists) OR move the cursor along with the last used cursor (which re quires syncing the cursor too)

@fonsp
Copy link
Owner

fonsp commented Jan 28, 2022

Issue: When two people edit the same cell, the cursor goes back to the first line. Options: keep your cursor at the same position as before (if it exists) OR move the cursor along with the last used cursor (which re quires syncing the cursor too)

I thought we were only going to allow editing by one person at a time? Disabling input for others

@fonsp
Copy link
Owner

fonsp commented Jan 28, 2022

First crash (browser freezed):

Schermafbeelding 2022-01-27 om 18 21 58

@fonsp
Copy link
Owner

fonsp commented Jan 28, 2022

Second crash (browser freezed):

Schermafbeelding 2022-01-27 om 18 23 04

The error is probably fine (and not the cause of the freeze), but you're not supposed to get that warning

@dralletje
Copy link
Collaborator

@pankgeorg looking at this now.. 😏

@fonsp
Copy link
Owner

fonsp commented Feb 3, 2022

f3fce3e (#1729)

This is great functionality but it really should not have been in this PR... We just want to get this PR done so that we can merge it, this makes that more difficult. (Right?) @dralletje or @pankgeorg let's make a new branch for those new features and undo it on this PR?

@dralletje
Copy link
Collaborator

Grrrrr

But you make a good point

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.

None yet

3 participants