Add new feature: Vim mode #368

steewbsd · 2020-11-16T22:44:33Z

This feature adds a working vim mode, some bugs are already known and will try to fix them. For example, getting inside shortcuts menu inside vim mode will render focus unusable and require an app reset (quit with q and open again).

steewbsd

Please test whether the ESC key works on a binding. Change the VimNormalMode from "-" to tcell.KeyEscape.

Bios-Marcel · 2020-11-17T09:50:49Z

The method addVimKey doesn't exist, you probably forgot to commit the file. You also added multiple unused dependencies to the go.mod.

steewbsd · 2020-11-17T11:01:00Z

Cannonical imports are preventing the package from building. I need some help managing them.

Bios-Marcel · 2020-11-17T11:11:40Z

go.mod

@@ -1,3 +1,5 @@
+replace github.com/Bios-Marcel/cordless => github.com/0xSteeW/cordless v0.0.0-20201117104521-a2cc7de4b6dd


This should not be upstream, it shouldn't be necessary at all.

Will not build either way. It was a quick test to try to get appveyor to recognize my files

… vim

Bios-Marcel · 2020-11-21T12:13:50Z

I hit alt+dot and this happened:

marcel@marcel-pc /m/d/c/cordless (0xSteeW-vim)> go run . --account=forkwatch
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x850ed9]

goroutine 1 [running]:
github.com/Bios-Marcel/cordless/tview.(*Application).Run.func1(0xc0000c17c0)
	/mnt/data/code/cordless/tview/application.go:168 +0x82
panic(0xd088c0, 0x165e470)
	/snap/go/6728/src/runtime/panic.go:969 +0x166
github.com/Bios-Marcel/cordless/shortcuts.(*Shortcut).Equals(0xc0000b7720, 0xc000c9b800, 0xc000c99e00)
	/mnt/data/code/cordless/shortcuts/shortcuts.go:440 +0x9
github.com/Bios-Marcel/cordless/ui.(*Window).handleGlobalShortcuts(0xc000130c60, 0xc000c9b800, 0xc0000c17c0)
	/mnt/data/code/cordless/ui/window.go:2145 +0xe4
github.com/Bios-Marcel/cordless/tview.(*Application).Run(0xc0000c17c0, 0x0, 0x0)
	/mnt/data/code/cordless/tview/application.go:256 +0xa4e
main.main()
	/mnt/data/code/cordless/main.go:74 +0x4cd
exit status 2

Bios-Marcel · 2020-11-21T12:14:11Z

I can't hit a single key with the app crashing actually ^^

steewbsd · 2020-11-21T13:10:59Z

Noticed it yesterday, only happening after merging your last commit. Can you try building my last commit (before merging)?

This reverts commit b2bd175, reversing changes made to 9e8bd4c.

Bios-Marcel · 2020-11-22T14:38:45Z

commands/commandimpls/vim.go

+)
+
+type VimHelp struct {
+	vimMode	*vim.Vim


This is doubly tracked, the config already knows about this, right?

Config only knows about VimEnabled, which is a bool only used when instantiating app.VimMode. This is needed to directly enable or disable vim interactively in the command line for this session.

Bios-Marcel · 2020-11-22T14:38:59Z

tview/application.go

 }

 // NewApplication creates and returns a new application.
-func NewApplication() *Application {
+func NewApplication(vimEnabled bool) *Application {


Why does tview need to know about vim mode?

Application holds the pointer to vim.Vim that will be shared to the entire program, with app.vimMode.

But this is a cordless thing, not a tview thing, right?

It is a cordless thing, but the best place as it seemed to me to share the global pointer to vim.Vim was in window.app and derived. So NewApplication needs to know if vim is enabled (as it does not have access to the config)

Bios-Marcel · 2020-11-22T14:39:19Z

tview/application.go

@@ -67,17 +68,17 @@ type Application struct {
 	// stop the application.
 	screenReplacement chan tcell.Screen

-	// Defines whether a bracketed paste is currently ongoing.
-	pasteActive bool


Still needs a merge with master, this shouldn't be an outgoing deletion!

Forgot to re-merge after reverting commits.

Bios-Marcel · 2020-11-22T14:40:01Z

shortcuts/shortcuts_win.go

@@ -5,5 +5,5 @@ package shortcuts
 import tcell "github.com/gdamore/tcell/v2"

 func addDeleteLeftShortcut() *Shortcut {
-	return addShortcut("delete_left", "Delete left", multilineTextInput, tcell.NewEventKey(tcell.KeyBackspace, rune(tcell.KeyBackspace), tcell.ModNone))
+	return addShortcut("delete_left", "Delete left", multilineTextInput, tcell.NewEventKey(tcell.KeyBackspace, rune(tcell.KeyBackspace), tcell.ModNone),nil)


Why is the last parameter nil? Does this need explanation or do we need to change it?

Windows build for some reason wont compile, it does not find anything declared inside shortcuts for some reason, so I can not use shortcuts.addVim or NullVimEvent. It's pretty much a HACK to make it pass the build.

Bios-Marcel · 2020-11-22T14:40:23Z

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/Bios-Marcel/shortnotforlong v1.1.1
 	github.com/alecthomas/chroma v0.7.3
 	github.com/atotto/clipboard v0.1.2
+	github.com/bunyk/gokeybr v0.0.0-20201019133936-f9e4ed3fdc5d // indirect


What's this?

Installed some things while working on this, thought I had reverted go.sum and go.mod. Will fix

Bios-Marcel · 2020-11-22T14:53:15Z

ui/window.go

+			window.vimStatus.Content = fmt.Sprintf("Vim: %s",window.app.VimMode.CurrentModeString())
+			return nil
+		} else if shortcuts.VimVisualMode.Equals(event) {
+			window.app.VimMode.Visual()


It's not obvious what the methods do, as there's no verb in the names.

Bios-Marcel · 2020-11-22T14:54:20Z

ui/window.go

@@ -2117,6 +2141,35 @@ func (window *Window) handleGlobalShortcuts(event *tcell.EventKey) *tcell.EventK
 		return nil
 	}

+	if window.app.VimMode.CurrentMode != vim.Disabled {


What about the login view, I assume that's not handled with this?

Login is not handled.

Bios-Marcel · 2020-11-22T14:54:43Z

util/vim/vim.go

+	// InsertMode : every key is sent to the program as normal,
+	// such as input boxes.
+	InsertMode				// 1
+	// TODO


Bios-Marcel · 2020-11-22T14:55:20Z

util/vim/vim.go

+	NormalMode int = iota	// 0
+	// InsertMode : every key is sent to the program as normal,
+	// such as input boxes.
+	InsertMode				// 1


The //0 and //1 comments are kinda unnecessary.

Bios-Marcel · 2020-11-22T14:55:41Z

util/vim/vim.go

+	VisualMode				// 2
+
+	// Default: disabled
+	Disabled = -1


Why do we need disabled? Isn't this what VimEnabled: false in the config is for?
And if I missunderstand, why isn't this 0? You might also want to create a type alias type VimMode int.

Config's VimEnabled is only used when instantiating vim.Vim. This struct then holds a value, CurrentMode, that will be checked everywhere vim mode needs to replace normal bindings.

bobbbay · 2021-01-02T00:14:34Z

+1 this would make and break for myself, and many others. I can finally moderate my vim server with vim keybindings!

Daniel Pérez added 12 commits November 14, 2020 00:21

Started working on vim event modifiers

10de420

Added most of the new vim keys (needs rework)

f0515f4

Partially working vim movements. Cannot get ESC key to work.

a15d76c

Removed some useless lines

26d6a15

Vim mode working. Added simulation keys inside some lists.

714e23b

Dynamic status bar and shortcuts menu

ce00566

Dynamic status bar and shortcuts menu

56fcff0

Fixed some keybindings

2016462

Merged from upstream

2c86b71

Fixed little issues

7219309

Edited changelog

155c160

Fixed go.sum

ad82e0e

steewbsd closed this Nov 16, 2020

steewbsd reopened this Nov 16, 2020

steewbsd commented Nov 16, 2020

View reviewed changes

Vim directional keys working everywhere (only in visual mode)

9e0b15d

steewbsd force-pushed the vim branch from de5606c to 9e0b15d Compare November 17, 2020 10:04

Daniel Perez added 4 commits November 17, 2020 11:39

Added brief manpage of vim mode

1b1c0c1

Fixed normal mode binding

86df4fd

Removed useless go sum lines

a2cc7de

Added replace instruction in go.mod

5050954

Bios-Marcel reviewed Nov 17, 2020

View reviewed changes

Daniel Perez added 4 commits November 17, 2020 12:13

Removed upstream fork from go.mod

422d825

Fixed escape binding

87a8946

Fixed shortcuts dialog inconsistency

b55c343

Fixed weird behaviour in text input

b38cbd9

steewbsd mentioned this pull request Nov 17, 2020

Vim mode #140

Closed

Changed expand selection shortcuts, conflicted with movement

6de4e41

Daniel Perez added 2 commits November 18, 2020 17:28

Vim can now be toggled in command mode

eea5b6b

Added vim command synopsis toggle line

1cc4e20

steewbsd force-pushed the vim branch from 5c5034d to 1cc4e20 Compare November 18, 2020 19:07

Daniel Perez added 3 commits November 19, 2020 11:26

Moved VimMode to app instead of config

9e8bd4c

Merged with upstream, fixed conflicts

b2bd175

Merge branch 'master' of https://github.com/Bios-Marcel/cordless into…

9d79a7f

… vim

Daniel Perez added 5 commits November 21, 2020 15:12

"Reverted merge

076d73b

This reverts commit b2bd175, reversing changes made to 9e8bd4c.

Fixed windows build

8774ec7

Fixed windows build

1e22b40

Fixed SIGSEGV

e7e4512

Attempt to fix appveyor compilation

dfdc8c6

Bios-Marcel suggested changes Nov 22, 2020

View reviewed changes

Daniel Perez added 2 commits November 22, 2020 16:41

Fixed go.mod and go.sum

30b8b96

Fixed format and qol

68d0832

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new feature: Vim mode #368

Add new feature: Vim mode #368

steewbsd commented Nov 16, 2020

steewbsd left a comment

Bios-Marcel commented Nov 17, 2020

steewbsd commented Nov 17, 2020

Bios-Marcel Nov 17, 2020

steewbsd Nov 17, 2020

Bios-Marcel commented Nov 21, 2020

Bios-Marcel commented Nov 21, 2020

steewbsd commented Nov 21, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

Bios-Marcel Nov 22, 2020

Bios-Marcel Nov 22, 2020

steewbsd Nov 22, 2020

bobbbay commented Jan 2, 2021

		@@ -1,3 +1,5 @@
		replace github.com/Bios-Marcel/cordless => github.com/0xSteeW/cordless v0.0.0-20201117104521-a2cc7de4b6dd

Add new feature: Vim mode #368

Are you sure you want to change the base?

Add new feature: Vim mode #368

Conversation

steewbsd commented Nov 16, 2020

steewbsd left a comment

Choose a reason for hiding this comment

Bios-Marcel commented Nov 17, 2020

steewbsd commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bios-Marcel commented Nov 21, 2020

Bios-Marcel commented Nov 21, 2020

steewbsd commented Nov 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobbbay commented Jan 2, 2021