Skip to content

Commit

Permalink
feat: add up/down key support
Browse files Browse the repository at this point in the history 
* (WIP) Add support for up/down arrows.

Up and down arrows now have some behavior, but it is currently not as
intended.

* (WIP) Change up and down arrow support implementation.

* (WIP) Up and down arrows working.

Movement is working, but behavior is still undefined at the
and end of files, so there are a few bugs to be fixed.

* Change init of variables to avoid conflict.

Newline index variables n1 and n2 now initialize to -1 to avoid
conflict when running into the beginning of the document.

* (WIP) Fix down arrow behavior

Down arrow behavior seems to be working in standard use and some edge
cases. Up arrow has not been fixed.

* Fix down arrow behavior

* Fix up/down arrow bugs

Got an implementation of up/down arrow behavior working. The
implementation is rather convoluted and hard to reason about. There is
definitely room for tightening the logic and improving readability.

It's also worth considering a change to the way editor text is currently
represented, so that it maps more closely to Termbox's 2D structure. A
2D array of runes might make a more natural interface between the linear
CRDT document and the 2D Termbox terminal, and therefore make the code
cleaner and more maintainable.

Future:
- tighten logic
- remove logging
- refactor (explore alternate ways to store editor text)

* Change comments in MoveCursor function.

* Remove duplicate line.

* Remove logging and unused variable.

* Refactor MoveCursor into smaller functions.

* Fix unused parameter and duplicate initialization.

* Fix ineffassign linting error.

* Fix panics occurring with empty document.

Fixed a couple of crashes that happened due to out of bounds errors
if the document was empty. MoveCursor would try to do some invalid
calculations which are now avoided by checking the text length first.
Also, on delete, the cursor was getting reset to 1 instead of 0,
which caused a crash when characters were inserted.

* (WIP) Add tests and fix some cursor movement bugs.

Added many tests for cursor movement. Still in the process of fixing
the bugs found in the tests. Also temporarily added logging to help with
bug fixing.

* Upward cursor movement now passes tests.

Rewrote calcCursor up with slightly different logic. It is simpler and
cleaner than the previous attempts. Further refinement might be
possible.

* Remove old functions and extraneous comments.

* Refactor downward cursor movement.

Changed calcCursorDown to match calcCursorUp logic. Both functions
now pass tests.

* Add test cases.

* Fix cursor movement bugs and add/update comments.

Fixed a couple more edge cases with the new cursor movement logic,
including some cases where panics occurred. Behavior now seems to
match expectations.

Also updated comments for both calcCursorUp and calcCursorDown.

* Fix first character not able to be deleted.

---------

Co-authored-by: burntcarrot <[email protected]>
Co-authored-by: Aadhav Vignesh <[email protected]>
  • Loading branch information
@benmuth @burntcarrot
3 people authored Feb 21, 2023
1 parent 86779c0 commit aea18bb
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 14 deletions.
134 changes: 131 additions & 3 deletions client/editor/editor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,146 @@ func (e *Editor) showPositions() {
}

// MoveCursor updates the Cursor position.
func (e *Editor) MoveCursor(x, _ int) {
func (e *Editor) MoveCursor(x, y int) {
if len(e.Text) == 0 && e.Cursor == 0 {
return
}
// Move cursor horizontally.
newCursor := e.Cursor + x

if newCursor < 0 {
newCursor = 0
// Move cursor vertically.
if y > 0 {
newCursor = e.calcCursorDown()
}

if y < 0 {
newCursor = e.calcCursorUp()
}

// Reset to bounds.
if newCursor > len(e.Text) {
newCursor = len(e.Text)
}

if newCursor < 0 {
newCursor = 0
}

e.Cursor = newCursor
}

// For the functions calcCursorUp and calcCursorDown, newline characters are found by iterating
// backward and forward from the current Cursor position. These characters are taken as the "start"
// and "end" of the current line. The "offset" from the start of the current line to the Cursor
// is calculated and used to determine the final Cursor position on the target line, based on whether the
// offset is greater than the length of the target line. "pos" is used as a placeholder variable for
// the Cursor.

// calcCursorUp calculates the intended Cursor position after moving the Cursor up one line.
func (e *Editor) calcCursorUp() int {
pos := e.Cursor
offset := 0

// If the initial cursor is out of the bounds of the Text or already on a newline, move it.
if pos == len(e.Text) || e.Text[pos] == '\n' {
offset++
pos--
}

if pos < 0 {
pos = 0
}

start, end := pos, pos

// Find the start of the current line.
for start > 0 && e.Text[start] != '\n' {
start--
}

// If the Cursor is already on the first line, move to the beginning of the Text.
if start == 0 {
return 0
}

// Find the end of the current line.
for end < len(e.Text) && e.Text[end] != '\n' {
end++
}

// Find the start of the previous line.
prevStart := start - 1
for prevStart >= 0 && e.Text[prevStart] != '\n' {
prevStart--
}

// Calculate the distance from the start of the current line to the Cursor.
offset += pos - start
if offset <= start-prevStart {
return prevStart + offset
} else {
return start
}
}

func (e *Editor) calcCursorDown() int {
pos := e.Cursor
offset := 0

// If the initial Cursor is out of the bounds of the Text or already on a newline, move it.
if pos == len(e.Text) || e.Text[pos] == '\n' {
offset++
pos--
}

if pos < 0 {
pos = 0
}

start, end := pos, pos

// Find the start of the current line.
for start > 0 && e.Text[start] != '\n' {
start--
}

// This handles the case where the Cursor is on the first line. This is necessary because the start
// of the first line is not a newline character, unlike the other lines in the Text.
if start == 0 && e.Text[start] != '\n' {
offset++
}

// Find the end of the current line.
for end < len(e.Text) && e.Text[end] != '\n' {
end++
}

// This handles the case where the Cursor is on a newline. end has to be incremented, otherwise
// start == end.
if e.Text[pos] == '\n' && e.Cursor != 0 {
end++
}

// If the Cursor is already on the last line, move to the end of the Text.
if end == len(e.Text) {
return len(e.Text)
}

// Find the end of the next line.
nextEnd := end + 1
for nextEnd < len(e.Text) && e.Text[nextEnd] != '\n' {
nextEnd++
}

// Calculate the distance from the start of the current line to the Cursor.
offset += pos - start
if offset < nextEnd-end {
return end + offset
} else {
return nextEnd
}
}

// calcCursorXY calculates Cursor position from the index obtained from the content.
func (e *Editor) calcCursorXY(index int) (int, int) {
x := 1
Expand Down
84 changes: 78 additions & 6 deletions client/editor/editor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,92 @@ func TestMoveCursor(t *testing.T) {
description string
cursor int
x int
y int
expectedCursor int
text []rune
}{
{description: "move forward", cursor: 0, x: 1, expectedCursor: 1},
{description: "move backward", cursor: 1, x: -1, expectedCursor: 0},
{description: "negative (out of bounds)", cursor: 0, x: -10, expectedCursor: 0},
{description: "positive (out of bounds)", cursor: 12, x: 2, expectedCursor: 12},
// test horizontal movement
{description: "move forward (empty document)", cursor: 0, x: 1, expectedCursor: 0,
text: []rune("")},
{description: "move backward (empty document)", cursor: 0, x: -1, expectedCursor: 0,
text: []rune("")},
{description: "move forward", cursor: 0, x: 1, expectedCursor: 1,
text: []rune("foo\n")},
{description: "move backward", cursor: 1, x: -1, expectedCursor: 0,
text: []rune("foo\n")},
{description: "move backward (out of bounds)", cursor: 0, x: -10, expectedCursor: 0,
text: []rune("foo\n")},
{description: "move forward (out of bounds)", cursor: 3, x: 2, expectedCursor: 4,
text: []rune("foo\n")},
// test vertical movement
{description: "move up", cursor: 6, y: -1, expectedCursor: 2,
text: []rune("foo\nbar")},
{description: "move down", cursor: 1, y: 2, expectedCursor: 5,
text: []rune("foo\nbar")},
{description: "move up (empty document)", cursor: 0, y: -1, expectedCursor: 0,
text: []rune("")},
{description: "move down (empty document)", cursor: 0, y: 1, expectedCursor: 0,
text: []rune("")},
{description: "move up (first line)", cursor: 1, y: -1, expectedCursor: 0,
text: []rune("foo\nbar")},
{description: "move down (last line)", cursor: 4, y: 1, expectedCursor: 7,
text: []rune("foo\nbar")},
{description: "move up (middle line)", cursor: 5, y: -1, expectedCursor: 1,
text: []rune("foo\nbar\nbaz")},
{description: "move down (middle line)", cursor: 5, y: 1, expectedCursor: 9,
text: []rune("foo\nbar\nbaz")},
{description: "move up (on newline)", cursor: 3, y: -1, expectedCursor: 0,
text: []rune("foo\nbar\nbaz")},
{description: "move down (on newline)", cursor: 3, y: 1, expectedCursor: 7,
text: []rune("foo\nbar\nbaz")},
{description: "move up (on newline, first line)", cursor: 3, y: -1, expectedCursor: 0,
text: []rune("foo\nbar\nbaz")},
{description: "move down (on newline, last line)", cursor: 7, y: 1, expectedCursor: 11,
text: []rune("foo\nbar\nbaz")},
{description: "move up (different lengths, short to long)", cursor: 8, y: -1, expectedCursor: 3,
text: []rune("fool\nbar\nbaz")},
{description: "move down (different lengths, short to long)", cursor: 3, y: 1, expectedCursor: 7,
text: []rune("foo\nbare\nbaz")},
{description: "move up (different lengths, long to short)", cursor: 8, y: -1, expectedCursor: 3,
text: []rune("foo\nbare\nbaz")},
{description: "move down (different lengths, long to short)", cursor: 4, y: 1, expectedCursor: 8,
text: []rune("fool\nbar\nbaz")},
{description: "move up (from empty line)", cursor: 4, y: -1, expectedCursor: 0,
text: []rune("foo\n\nbaz")},
{description: "move down (from empty line)", cursor: 4, y: 1, expectedCursor: 5,
text: []rune("fool\n\nbaz")},
{description: "move up (from empty line to empty line)", cursor: 5, y: -1, expectedCursor: 4,
text: []rune("foo\n\n\n")},
{description: "move down (from empty line to empty line)", cursor: 1, y: 1, expectedCursor: 2,
text: []rune("\n\n\nfoo")},
{description: "move up (from empty last line to empty line)", cursor: 3, y: -1, expectedCursor: 2,
text: []rune("\n\n\n")},
{description: "move down (from empty first line to empty line)", cursor: 0, y: 1, expectedCursor: 1,
text: []rune("\n\n\n")},
{description: "move up (from empty last line)", cursor: 6, y: -1, expectedCursor: 2,
text: []rune("\n\nfoo\n")},
{description: "move down (from empty first line)", cursor: 0, y: 1, expectedCursor: 1,
text: []rune("\nfoo\n\n")},
{description: "move up (from empty first line)", cursor: 0, y: -1, expectedCursor: 0,
text: []rune("\n\nfoo\n")},
{description: "move down (from empty last line)", cursor: 6, y: 1, expectedCursor: 6,
text: []rune("\nfoo\n\n")},
{description: "move up (from last line to empty line)", cursor: 2, y: -1, expectedCursor: 1,
text: []rune("\n\nfoo")},
{description: "move down (from first line to empty line)", cursor: 2, y: 1, expectedCursor: 4,
text: []rune("foo\n\n")},
{description: "move up (from empty line to empty line 2)", cursor: 2, y: -1, expectedCursor: 1,
text: []rune("\n\n\n\n\n")},
{description: "move down (from empty line to empty line 2)", cursor: 2, y: 1, expectedCursor: 3,
text: []rune("\n\n\n\n\n")},
}

e := NewEditor()
e.Text = []rune("content\ntest")

for _, tc := range tests {
e.Cursor = tc.cursor
e.MoveCursor(tc.x, 0)
e.Text = tc.text
e.MoveCursor(tc.x, tc.y)

got := e.Cursor
expected := tc.expectedCursor
Expand Down
11 changes: 6 additions & 5 deletions client/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ func handleTermboxEvent(ev termbox.Event, conn *websocket.Conn) error {
e.MoveCursor(-1, 0)
case termbox.KeyArrowRight, termbox.KeyCtrlF:
e.MoveCursor(1, 0)
case termbox.KeyArrowUp, termbox.KeyCtrlP:
e.MoveCursor(0, -1)
case termbox.KeyArrowDown, termbox.KeyCtrlN:
e.MoveCursor(0, 1)
case termbox.KeyHome:
e.SetX(0)
case termbox.KeyEnd:
Expand Down Expand Up @@ -352,18 +356,15 @@ func performOperation(opType int, ev termbox.Event, conn *websocket.Conn) {
msg = message{Type: "operation", Operation: Operation{Type: "insert", Position: e.Cursor, Value: ch}}
case OperationDelete:
logger.Infof("LOCAL DELETE: cursor position %v\n", e.Cursor)
if e.Cursor-1 <= 0 {
e.Cursor = 1
if e.Cursor-1 < 0 {
e.Cursor = 0
}
text := doc.Delete(e.Cursor)
e.SetText(text)
msg = message{Type: "operation", Operation: Operation{Type: "delete", Position: e.Cursor}}
e.MoveCursor(-1, 0)
}

// Print document state to logs.
printDoc(doc)

err := conn.WriteJSON(msg)
if err != nil {
e.StatusMsg = "lost connection!"
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
)

require (
github.com/Pallinder/go-randomdata v1.2.0 // indirect
github.com/mattn/go-colorable v0.1.9 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
github.com/rivo/uniseg v0.2.0 // indirect
Expand Down

0 comments on commit aea18bb

Please sign in to comment.