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

More robust algorithm and backend rewrite in Node #43

Merged
merged 24 commits into from
Apr 24, 2020
Merged

More robust algorithm and backend rewrite in Node #43

merged 24 commits into from
Apr 24, 2020

Conversation

olehermanse
Copy link
Owner

Algorithm loosely based on:

https://www.duo.uio.no/handle/10852/53012

Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
This is the first iteration of the algorithm / library which
will be shared by both frontend and backend. It is very simple
at the moment, but can be expanded.

Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
In preparation for changing positions to use row / column.
Negative indices can be reimplemented later, if necessary.

Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
The change events in ace editor look something like this:

```
{"start":{"row":1,"column":4},"action":"insert","lines":["o"]}
{"start":{"row":1,"column":5},"action":"insert","lines":["",""]}
```

(Simplified)

So, the new libbuf API aligns with this. The 2 main advantages are:

* Easy to implement, frontend doesn't have to translate events
* Concurrent editing on separate lines should just work

There's still more work to do to make the algorithm more robust,
but I think this is good enough for now. It should be better
than the old version that is deployed currently.

Both inserting and removing seems to work well, both in unit tests
and in editor.

Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Signed-off-by: Ole Herman Schumacher Elgesem <oleherman93@gmail.com>
Copy link
Collaborator

@michaelpande michaelpande left a comment

Choose a reason for hiding this comment

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

Bra jobba 🥇 👍

Bare småpirk egentlig.
Ta en gjennomgang av variabler og sørg for at de så ofte som mulig er const.

Du kan endre objektet selv om det er const, du kan bare ikke overskrive hele.

FYI: Var ikke veldig grundig på libbuf.js, men det så bra ut! :)

Dockerfile Show resolved Hide resolved
Makefile Show resolved Hide resolved
backend/app.js Show resolved Hide resolved
backend/app.js Show resolved Hide resolved
backend/app.js Show resolved Hide resolved
frontend/src/scripts/editor.js Show resolved Hide resolved
libbuf/libbuf.js Show resolved Hide resolved
libbuf/libbuf.js Show resolved Hide resolved
libbuf/libbuf.js Show resolved Hide resolved
libbuf/libbuf.js Show resolved Hide resolved
@olehermanse olehermanse merged commit cc57e53 into master Apr 24, 2020
@olehermanse olehermanse deleted the node branch April 24, 2020 14:56
@olehermanse
Copy link
Owner Author

@michaelpande Takk, alle kommentarene dine er besvart, og follow-up er her: #44

@michaelpande
Copy link
Collaborator

michaelpande commented Apr 26, 2020 via email

olehermanse added a commit that referenced this pull request Apr 27, 2020
Addressed review comments from #43
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