Add Entry undo/redo functionality #4251

xypwn · 2023-09-14T18:16:46Z

Description:

Implements #436

Checklist:

Tests included.
Lint and formatter run with no errors.
Tests all pass.

Notes:

I wrote this independently of #2539.

My implementation is fairly extensible, although currently tied to Entry by name.
It works by keeping track of the changes made by the user, NOT by making snapshots. This makes it more efficient and flexible than the other implementation.

Basic Architecture:

interface entryUndoAction: Represents a single user interaction. It contains the functions Undo and Redo, which modify the current Entry content to undo/redo that user interaction.

interface entryMergeableUndoAction: Extends entryUndoAction. Contains the TryMerge method, which allows for merging two actions into one. This is useful because, for example when typing a word, we want to undo that whole word at once, and not have to spam Ctrl+Z for every character.

struct entryInsertAction: Implements entryMergeableUndoAction and represents a string insertion or deletion. It only saves the position and string of the edit, i.e. no snapshots, as mentioned previously. It can merge with itself, but doesn't merge across word boundaries.

struct entryUndoStack: This is the main interface for dealing with the undo/redo system. It has the following methods:

Undo/Redo: Executes the current undo/redo operation on the current string and returns the new string as well as the executed operation.
Add/MergeOrAdd: Records the given action, allowing for Undo/Redo to be called afterwards. MergeOrAdd also tries to merge the given action with the undo action already on top of the stack; if it can't merge, it just reverts to call Add.
Clear clears all records in the undo stack. This is done after SetText() is called, for example, also meaning that SetText() modifications aren't recorded.

Feel free to drop in any suggestions :)

coveralls · 2023-09-14T20:39:52Z

coverage: 65.382% (+0.08%) from 65.301% when pulling ddb1bf8 on xypwn:develop into 3ee5a3e on fyne-io:develop.

andydotxyz

This is really excellent work, thanks so much.
A couple of items about naming inline but it is clear and well laid out, thanks.

widget/entry.go

andydotxyz · 2023-09-19T10:29:07Z

One possible bug to be improved - if I "undo" and there are no items left it seems to put the cursor back to the top left, scrolling content if appropriate. I don't think that should happen?

xypwn · 2023-09-19T11:23:39Z

Hi, glad you like it! I'm not home right now, but when I am I'll be able address these issues and make the necessary changes.

Regarding the naming, insert was essentially vim terminology. But I think I'll change it to something like "modifyAction" instead, since that makes a lot more sense.

Regarding the bug, that's because the new cursor position is default initialized to 0 and if it isn't an insertAction, it uses that default value instead of just returning. Should be an easy fix.

Also, I think it would make sense to expose Undo and Redo as public functions, since you might want that for text editors, for example.

When the undo stack was empty, the cursor would just jump to position 0 on undo. With this change, undo/redo only changes the cursor position if anything was actually undone or redone.

xypwn · 2023-09-20T19:46:31Z

Alright, I made all the changes you requested + I made Undo/Redo public functions

andydotxyz

This looks excellent, thanks for all the fixes.
Please annotate the new APIs with the version number as commented inline

widget/entry.go

Jacalz

Thanks for working on this. I have a few comments inline.

widget/entry.go

Jacalz · 2023-09-21T19:38:25Z

widget/entry.go

+	undoItem := fyne.NewMenuItem("Undo", e.Undo)
+	redoItem := fyne.NewMenuItem("Redo", e.Redo)


How common is it to have undo and redo in the context menu? It seems to me like most applications don't expose it there.

QT has undo/redo in the context menu in every text box by default. Also all popular web browsers. GTK apps are the only counter example I can think of. I don't think it's unreasonable to expose it in the context menu.

A possible improvement though would be to only show undo/redo if there is actually anything to undo/redo

A possible improvement though would be to only show undo/redo if there is actually anything to undo/redo

I just implemented that, it was pretty simple. Looks a bit cleaner now.

Normally it would be enable/disable rather than add/remove - having menus be consistent in size and layout makes it easier to navigate them at speed.

I mean, it is less cluttered. Also that's how Chromium does it. And if you want to operate at speed, you'll want to use keyboard shortcuts anyway. But your project, your design decisions :)

widget/entry.go

xypwn · 2023-09-23T21:39:53Z

What's it with the CI checks not passing? Is that my fault?

andydotxyz · 2023-09-24T10:30:52Z

There seems to be a bug with the windows / go 1.17 that has impacted all builds in the last 3 days. Multiple projects have noted it - we may need to downgrade the patch version for that build until it's fixed.

Not your fault at all!

andydotxyz

Just 2 types missing the Since line sorry I missed that before.

Otherwise I think it's great - @Jacalz ?

shortcut.go

andydotxyz

Amazing thanks

Jacalz · 2023-09-30T15:06:32Z

I'll try to review tomorrow or on Monday. Been a bit busy lately

Jacalz

Thanks @xypwn for a well written PR both in terms of code and the extensive description. Your work is much appreciated. Looks good to me.

Also, congratulations on landing your first PR for Fyne 🥳

xypwn · 2023-10-02T22:29:21Z

Let's gooo! Thank you all!

andydotxyz · 2023-10-02T22:56:56Z

So excited this is in - thanks a lot :) v2.5.0 looks great already and it's still 3-4 months away!

aneeskA · 2024-05-27T11:17:13Z

@andydotxyz Can you please share the release plan for these changes? I cannot find them in the master branch.

xypwn · 2024-05-27T12:33:27Z

@andydotxyz Can you please share the release plan for these changes? I cannot find them in the master branch.

It will be added in v2.5.0

andydotxyz · 2024-05-27T12:47:58Z

@andydotxyz Can you please share the release plan for these changes? I cannot find them in the master branch.

It will be added in v2.5.0

Exactly right. As you can see we are working towards feature freeze now, but you can use it on @develop branch before then.
https://github.com/fyne-io/fyne/wiki/Releases

xypwn added 2 commits September 14, 2023 19:54

Add Entry undo/redo functionality

1043da4

go fmt

ddb1bf8

andydotxyz requested changes Sep 19, 2023

View reviewed changes

widget/entry.go Outdated Show resolved Hide resolved

widget/entry.go Outdated Show resolved Hide resolved

widget/entry.go Outdated Show resolved Hide resolved

xypwn added 5 commits September 20, 2023 21:32

Make Undo/Redo a public API; remove NewMenuItem wrapper func

f3be986

Fix func call

0cdb446

Add separator to right click menu between undo/redo and other items

d9d4996

Rename entryInsertAction to entryModifyAction

bddb10a

Fix Undo/Redo cursor position bug

b0b97dc

When the undo stack was empty, the cursor would just jump to position 0 on undo. With this change, undo/redo only changes the cursor position if anything was actually undone or redone.

Fix testdata

a0eab73

andydotxyz requested changes Sep 20, 2023

View reviewed changes

widget/entry.go Outdated Show resolved Hide resolved

xypwn added 3 commits September 21, 2023 00:19

Fyne-conform comments for public Undo/Redo methods

065d250

Order Undo/Redo methods alphabetically

a1891e4

Comment formatting

1f9cbaa

Jacalz requested changes Sep 21, 2023

View reviewed changes

widget/entry.go Outdated Show resolved Hide resolved

xypwn added 4 commits September 22, 2023 03:07

Only show undo/redo menu item if undo/redo available

7e7093e

Avoid making copy of provider string

f628e81

Properly lock Text before accessing

e9730ed

Fix deadlock

a102aa3

Fix undo/redo for newline in multiline entry

18829f7

andydotxyz requested changes Sep 29, 2023

View reviewed changes

shortcut.go Show resolved Hide resolved

Since label for undo/redo shortcuts

0abac16

andydotxyz approved these changes Sep 29, 2023

View reviewed changes

Jacalz approved these changes Oct 2, 2023

View reviewed changes

Jacalz merged commit df6e029 into fyne-io:develop Oct 2, 2023

xypwn deleted the develop branch October 2, 2023 22:37

xypwn restored the develop branch October 2, 2023 22:37

Jacalz mentioned this pull request Jan 7, 2024

Add basic Undo/Redo functionality to Entry widget #2539

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Entry undo/redo functionality #4251

Add Entry undo/redo functionality #4251

xypwn commented Sep 14, 2023 •

edited

Loading

coveralls commented Sep 14, 2023

andydotxyz left a comment

andydotxyz commented Sep 19, 2023

xypwn commented Sep 19, 2023

xypwn commented Sep 20, 2023

andydotxyz left a comment

Jacalz left a comment

Jacalz Sep 21, 2023

xypwn Sep 22, 2023

xypwn Sep 22, 2023

xypwn Sep 22, 2023 •

edited

Loading

andydotxyz Sep 22, 2023

xypwn Sep 23, 2023

xypwn commented Sep 23, 2023 •

edited

Loading

andydotxyz commented Sep 24, 2023 •

edited

Loading

andydotxyz left a comment

andydotxyz left a comment

Jacalz commented Sep 30, 2023

Jacalz left a comment

xypwn commented Oct 2, 2023

andydotxyz commented Oct 2, 2023

aneeskA commented May 27, 2024

xypwn commented May 27, 2024

andydotxyz commented May 27, 2024

		undoItem := fyne.NewMenuItem("Undo", e.Undo)
		redoItem := fyne.NewMenuItem("Redo", e.Redo)

Add Entry undo/redo functionality #4251

Add Entry undo/redo functionality #4251

Conversation

xypwn commented Sep 14, 2023 • edited Loading

Description:

Checklist:

Notes:

Basic Architecture:

coveralls commented Sep 14, 2023

andydotxyz left a comment

Choose a reason for hiding this comment

andydotxyz commented Sep 19, 2023

xypwn commented Sep 19, 2023

xypwn commented Sep 20, 2023

andydotxyz left a comment

Choose a reason for hiding this comment

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz Sep 21, 2023

Choose a reason for hiding this comment

xypwn Sep 22, 2023

Choose a reason for hiding this comment

xypwn Sep 22, 2023

Choose a reason for hiding this comment

xypwn Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

andydotxyz Sep 22, 2023

Choose a reason for hiding this comment

xypwn Sep 23, 2023

Choose a reason for hiding this comment

xypwn commented Sep 23, 2023 • edited Loading

andydotxyz commented Sep 24, 2023 • edited Loading

andydotxyz left a comment

Choose a reason for hiding this comment

andydotxyz left a comment

Choose a reason for hiding this comment

Jacalz commented Sep 30, 2023

Jacalz left a comment

Choose a reason for hiding this comment

xypwn commented Oct 2, 2023

andydotxyz commented Oct 2, 2023

aneeskA commented May 27, 2024

xypwn commented May 27, 2024

andydotxyz commented May 27, 2024

xypwn commented Sep 14, 2023 •

edited

Loading

xypwn Sep 22, 2023 •

edited

Loading

xypwn commented Sep 23, 2023 •

edited

Loading

andydotxyz commented Sep 24, 2023 •

edited

Loading