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.

Sign up for GitHub

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

Jump to bottom

Export Undo/Redo functions #100

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

Export Undo/Redo functions #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8eed6f9
Export Undo/Redo functions
FabienJansem Sep 4, 2022
9c40570
Export Undo/Redo functions
FabienJansem Sep 4, 2022
57e183d
Merge branch 'gioui:main' into main
FabienJansem Sep 17, 2022
ebde3ac
Merge branch 'gioui:main' into main
FabienJansem Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 35 additions & 23 deletions widget/editor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,7 @@ type Editor struct {
locale system.Locale

// history contains undo history.
history []modification
// nextHistoryIdx is the index within the history of the next modification. This
// is only not len(history) immediately after undo operations occur. It is framed as the "next" value
// to make the zero value consistent.
nextHistoryIdx int
History *History
}

type offEntry struct {
Expand Down Expand Up @@ -503,9 +499,9 @@ func (e *Editor) command(gtx layout.Context, k key.Event) {
e.caret.start = e.Len()
case "Z":
if k.Modifiers.Contain(key.ModShift) {
e.redo()
e.Redo()
} else {
e.undo()
e.Undo()
}
}
}
Expand Down Expand Up @@ -892,15 +888,18 @@ func (e *Editor) Text() string {
}

// SetText replaces the contents of the editor, clearing any selection first.
// History is cleared before and after replacement
func (e *Editor) SetText(s string) {
e.rr = editBuffer{}
e.caret.start = 0
e.caret.end = 0
e.History = nil // avoid alteration of previous text history
if e.SingleLine {
s = strings.ReplaceAll(s, "\n", " ")
}
e.replace(e.caret.start, e.caret.end, s, true)
e.caret.xoff = 0
e.History = nil // initial text is not a modification
}

func (e *Editor) scrollBounds() image.Rectangle {
Expand Down Expand Up @@ -1274,32 +1273,42 @@ type modification struct {
ReverseContent string
}

// undo applies the modification at e.history[e.historyIdx] and decrements
// e.historyIdx.
func (e *Editor) undo() {
if len(e.history) < 1 || e.nextHistoryIdx == 0 {
// History of modifications may be retrived/restored
type History struct {
// data contains undo/redo history.
data []modification
// nextIdx is the index within the history data of the next modification.
// This is only not len(data) immediately after undo operations occur.
// It is framed as the "next" value to make the zero value consistent.
nextIdx int
Copy link
Contributor

Choose a reason for hiding this comment

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

We're still stuck with nextIdx in your proposed change. Why can't modification be self-contained? I imagine calling it Edit and defined something like:

type Edit struct {
    // rng is the range to be replaced with text.
    rng key.Range
    text string
}

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry, but I don't understand your request.
nextIdx is the nextHistoryIdx that existed before my PR. I simply moved it to the History structure, which allows it to be saved along with the modifications (I changed the name because History was becoming redundant).
I don't see how it can be deleted without losing the boundary between undo and redo when undo was the last action performed by the user in the editor. If you don't save it, then when you restore modifications editor will consider that the redo's are undo's, which doesn't correspond to what's in the text. (that's my understanding from the existing comment associated with nextHistoryIdx which I also moved into the history).

Maybe I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

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

@eliasnaur we need a way to track the boundary between changes that have been applied and changes that can be applied. If you undo three changes, the final three modifications are what should happen when you "redo" three times. That's what the index is for. We could eliminate it by creating two slices (one for undo and one for redo), but we'd be copying elements between them pretty often. Other than that, I don't know how to efficiently eliminate the index.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, we may have different views of what makes History useful. I've posted on the issue tracker to clarify my understanding of your proposal.

}

// Undo applies the modification at e.history.data[e.history.nextIdx]
// and decrements e.history.nextIdx.
func (e *Editor) Undo() {
if e.History == nil || len(e.History.data) < 1 || e.History.nextIdx == 0 {
return
}
mod := e.history[e.nextHistoryIdx-1]
mod := e.History.data[e.History.nextIdx-1]
replaceEnd := mod.StartRune + utf8.RuneCountInString(mod.ApplyContent)
e.replace(mod.StartRune, replaceEnd, mod.ReverseContent, false)
caretEnd := mod.StartRune + utf8.RuneCountInString(mod.ReverseContent)
e.SetCaret(caretEnd, mod.StartRune)
e.nextHistoryIdx--
e.History.nextIdx--
}

// redo applies the modification at e.history[e.historyIdx] and increments
// e.historyIdx.
func (e *Editor) redo() {
if len(e.history) < 1 || e.nextHistoryIdx == len(e.history) {
// Redo applies the modification at e.history.data[e.history.nextIdx]
// and increments e.history.nextIdx.
func (e *Editor) Redo() {
if e.History == nil || len(e.History.data) < 1 || e.History.nextIdx == len(e.History.data) {
return
}
mod := e.history[e.nextHistoryIdx]
mod := e.History.data[e.History.nextIdx]
end := mod.StartRune + utf8.RuneCountInString(mod.ReverseContent)
e.replace(mod.StartRune, end, mod.ApplyContent, false)
caretEnd := mod.StartRune + utf8.RuneCountInString(mod.ApplyContent)
e.SetCaret(caretEnd, mod.StartRune)
e.nextHistoryIdx++
e.History.nextIdx++
}

// replace the text between start and end with s. Indices are in runes.
Expand Down Expand Up @@ -1339,15 +1348,18 @@ func (e *Editor) replace(start, end int, s string, addHistory bool) int {
ru, _, _ := e.rr.ReadRune()
deleted = append(deleted, ru)
}
if e.nextHistoryIdx < len(e.history) {
e.history = e.history[:e.nextHistoryIdx]
if e.History == nil {
e.History = &History{}
}
if e.History.nextIdx < len(e.History.data) {
e.History.data = e.History.data[:e.History.nextIdx]
}
e.history = append(e.history, modification{
e.History.data = append(e.History.data, modification{
StartRune: startPos.runes,
ApplyContent: s,
ReverseContent: string(deleted),
})
e.nextHistoryIdx++
e.History.nextIdx++
}

e.rr.deleteRunes(startOff, replaceSize)
Expand Down
63 changes: 58 additions & 5 deletions widget/editor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func TestEditorHistory(t *testing.T) {
e.Insert("")
assertContents(t, e, "", 0, 0)
// Ensure that undoing the overwrite succeeds.
e.undo()
e.Undo()
assertContents(t, e, "안П你 hello 안П你", 13, 0)
// Ensure that redoing the overwrite succeeds.
e.redo()
e.Redo()
assertContents(t, e, "", 0, 0)
// Insert some smaller text.
e.Insert("안П你 hello")
Expand All @@ -64,9 +64,9 @@ func TestEditorHistory(t *testing.T) {
e.Insert("П")
assertContents(t, e, "안ПeПlo", 4, 4)
// Ensure both operations undo successfully.
e.undo()
e.Undo()
assertContents(t, e, "안Пello", 4, 3)
e.undo()
e.Undo()
assertContents(t, e, "안П你 hello", 5, 1)
// Make a new modification.
e.Insert("Something New")
Expand All @@ -75,10 +75,63 @@ func TestEditorHistory(t *testing.T) {
// This redo() call should do nothing.
text := e.Text()
start, end := e.Selection()
e.redo()
e.Redo()
assertContents(t, e, text, start, end)
}

// TestEditorHistoryExpose ensures that History and SetHistory behave correctly.
func TestEditorHistoryExpose(t *testing.T) {
e := new(Editor)
// Insert some text and do modifications.
e.Insert("안П你 hello")
e.SetCaret(1, 5)
e.Insert("П")
e.SetCaret(3, 4)
e.Insert("П")
// Save text and history
savedText := e.Text()
savedHistory := e.History
// Clear history
e.History = nil
// Ensure no more Undo/Redo available
e.Undo()
assertContents(t, e, "안ПeПlo", 4, 4)
e.Redo()
assertContents(t, e, "안ПeПlo", 4, 4)
// restore history
e.History = savedHistory
// Ensure all Undos are back
e.Undo()
assertContents(t, e, "안Пello", 4, 3)
e.Undo()
assertContents(t, e, "안П你 hello", 5, 1)
e.Undo()
assertContents(t, e, "", 0, 0)
// Ensure Redo also works
e.Redo()
assertContents(t, e, "안П你 hello", 9, 0)
e.Redo()
assertContents(t, e, "안Пello", 2, 1)
e.Redo()
assertContents(t, e, "안ПeПlo", 4, 3)
// Init a new text
e.SetText("New text")
// Ensure history has been cleared
e.Undo()
assertContents(t, e, "New text", 0, 0)
// Put back previous text and history
e.SetText(savedText)
assertContents(t, e, "안ПeПlo", 0, 0)
e.History = savedHistory
// Ensure all Undos are back
e.Undo()
assertContents(t, e, "안Пello", 4, 3)
e.Undo()
assertContents(t, e, "안П你 hello", 5, 1)
e.Undo()
assertContents(t, e, "", 0, 0)
}

func assertContents(t *testing.T, e *Editor, contents string, selectionStart, selectionEnd int) {
t.Helper()
actualContents := e.Text()
Expand Down