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

Crash when attempting to bind two key sequences C-x C-c Quit and C-x C-s Save #3194

Closed
khaytsus opened this issue Mar 18, 2024 · 7 comments · Fixed by #3266
Closed

Crash when attempting to bind two key sequences C-x C-c Quit and C-x C-s Save #3194

khaytsus opened this issue Mar 18, 2024 · 7 comments · Fixed by #3266
Assignees
Labels
Milestone

Comments

@khaytsus
Copy link

khaytsus commented Mar 18, 2024

Description of the problem or steps to reproduce

I've used an editor for many, many years called microEmacs and I'm binding a few common keys I am used to use there into Micro to make it a little easier to transition to it. I was able to bind Control-x Control-s to Save just fine,

bind <Ctrl-x><Ctrl-s> "Save"

And it works. But I was trying to bind Control-X Control-c to quit and Micro crashes as soon as I hit enter.

bind <Ctrl-x><Ctrl-c> "Quit"

Micro encountered an error: runtime.errorString runtime error: comparing uncomparable type action.KeySequenceEvent
/usr/lib/golang/src/runtime/alg.go:266 (0x55e57f146d1a)
ifaceeq: if eq == nil {
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/bindings.go:269 (0x55e57f603514)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/command.go:663 (0x55e57f60c8c6)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/command.go:983 (0x55e57f60f1e4)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/actions.go:1470 (0x55e57f5ff892)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/info/infobuffer.go:147 (0x55e57f5d258e)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/infopane.go:190 (0x55e57f61310c)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/infopane.go:54 (0x55e57f61255d)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/infopane.go:125 (0x55e57f612a59)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/internal/action/infopane.go:93 (0x55e57f6127d0)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/cmd/micro/micro.go:443 (0x55e57f642c1c)
/builddir/build/BUILD/micro-2.0.11/_build/src/github.com/zyedidia/micro/cmd/micro/micro.go:382 (0x55e57f64268a)
/usr/lib/golang/src/runtime/internal/atomic/types.go:194 (0x55e57f17bf72)
(*Uint32).Load: return Load(&u.value)
/usr/lib/golang/src/runtime/asm_amd64.s:1598 (0x55e57f1ac481)
goexit: DATA shifts<>+0x00(SB)/8, $0x0000000000000000

After experimenting some more, it appears that I can't bind this sort of thing twice, unrelated to the command or control characters. If I remove the bindings.json I can bind whatever, but if I set one, then set another one, it crashes every time. If I remove bindings.json I can create Ctrl-x Ctrl-c and set it to Quit, works, but now I can't create Ctrl-x Ctrl-s for Save.

bindings.json after I've created a "Quit" command:

{
"\u003cCtrl-x\u003e\u003cCtrl-c\u003e": "Quit",
"Alt-/": "lua:comment.comment",
"CtrlUnderscore": "lua:comment.comment"
}

If I manually make a binding.json of the following, these two bindings do function in the editor:

{
"\u003cCtrl-x\u003e\u003cCtrl-c\u003e": "Quit",
"\u003cCtrl-x\u003e\u003cCtrl-s\u003e": "Save",
}

And I can even add simple bindings later, like bind Ctrl-s "Find". 
It's just when I try to do another <Ctrl-x><Ctrl-s> type binding it fails.

Basic reproduction steps:

1. Run micro
2. Hit control-e, type bind <Ctrl-x><Ctrl-c> "Quit"
3. Hit control-e, type bind <Ctrl-x><Ctrl-s> "Save"

Specifications

Commit hash: 225927b
OS: Fedora 38
Terminal: xfce-terminal

I have also tried on the latest Release version from the github, 68d88b5

Sorry for the weird formatting, github markdown (?) was eating some of the sequences.

@dustdfg
Copy link
Contributor

dustdfg commented Mar 19, 2024

I confirm it leads to crash on current master c64add2 with

Micro encountered an error: runtime.errorString runtime error: comparing uncomparable type action.KeySequenceEvent
runtime/alg.go:266 (0x40519a)
github.com/zyedidia/micro/v2/internal/action/bindings.go:282 (0x8ac385)
github.com/zyedidia/micro/v2/internal/action/command.go:673 (0x8b67df)
github.com/zyedidia/micro/v2/internal/action/command.go:989 (0x8b9144)
github.com/zyedidia/micro/v2/internal/action/actions.go:1545 (0x8a8092)
github.com/zyedidia/micro/v2/internal/info/infobuffer.go:152 (0x87d494)
github.com/zyedidia/micro/v2/internal/action/infopane.go:206 (0x8bd68c)
github.com/zyedidia/micro/v2/internal/action/infopane.go:54 (0x8bc656)
github.com/zyedidia/micro/v2/internal/action/infopane.go:129 (0x8bcea2)
github.com/zyedidia/micro/v2/internal/action/infopane.go:93 (0x8bcb5e)
github.com/zyedidia/micro/v2/cmd/micro/micro.go:476 (0x8f0287)
github.com/zyedidia/micro/v2/cmd/micro/micro.go:395 (0x8efc9e)
runtime/proc.go:250 (0x43afd2)
runtime/asm_amd64.s:1594 (0x46a9a1)

Manually setting values int the bindings.json file works

@khaytsus
Copy link
Author

Out of curiosity I tried to see if this was a regression and it seems like it maybe has always been this way, 2.0.7 was the first version that seemed to support this syntax and it crashes similarly.

@dustdfg
Copy link
Contributor

dustdfg commented Mar 19, 2024

By adding small print in the code before error, I understood why it reports uncomparable types

screen-1710855638

screen-1710855662

function findEvent internally use two functions: findSingleEvent for single events and findEvents for frequencies. First returns one event while another returns an array of such events wrapped to an object. So you can't compare. These two types are uncomparable but findEvent just a facade that returns one of the result of those two functions.

func findEvents(k string) (b KeySequenceEvent, ok bool, err error) {
var events []Event = nil
for len(k) > 0 {
groups := r.FindStringSubmatchIndex(k)
if len(groups) > 3 {
if events == nil {
events = make([]Event, 0, 3)
}
e, ok := findSingleEvent(k[groups[2]:groups[3]])
if !ok {
return KeySequenceEvent{}, false, errors.New("Invalid event " + k[groups[2]:groups[3]])
}
events = append(events, e)
k = k[groups[3]+1:]
} else {
return KeySequenceEvent{}, false, nil
}
}
return KeySequenceEvent{events}, true, nil
}

func findSingleEvent(k string) (b Event, ok bool) {

But I am curios why does type system doesn't report it as a problem? I findEvent returns Event as like findSingleEvent but findEvents returns KeySequenceEvent how it can fit there?

func findEvent(k string) (Event, error) {
var event Event
event, ok, err := findEvents(k)
if err != nil {
return nil, err
}
if !ok {
event, ok = findSingleEvent(k)
if !ok {
return nil, errors.New(k + " is not a bindable event")
}
}
return event, nil
}

@JoeKar
Copy link
Collaborator

JoeKar commented Mar 19, 2024

Interesting, before the comparison they should...at least from my understanding...type checked and in case of KeySequenceEvent iterated once again and within that iteration compared against the entry, which is a KeyEvent.

@dustdfg
Copy link
Contributor

dustdfg commented Mar 19, 2024

You iterate through all the bindings in the bindings.json And then compare them with event passed to TryBindKey. If we will represent it like array of events in the bindings.json we will get something like [Event,Event,KeySequenceEvent,Event,...]. When you call the TryBindKey you iterate trough this array and will check all the elemnts one by one if it is equal to the event you passed to TryBindKey so it leads to bug

@dustdfg
Copy link
Contributor

dustdfg commented Mar 19, 2024

Or maybe I am wrong. I am almost always wrong ¯\_(ツ)_/¯. I've just deleted from my bindings all the events in order to prevent check between Event and KeySequenceEvent and the problem still persist 😅

@dmaluka
Copy link
Collaborator

dmaluka commented Mar 19, 2024

https://go.dev/ref/spec#Comparison_operators

Interface types that are not type parameters are comparable. Two interface values are equal if they have identical dynamic types and equal dynamic values or if both have value nil.

Struct types are comparable if all their field types are comparable.

A comparison of two interface values with identical dynamic types causes a run-time panic if that type is not comparable.

KeyEvent is a comparable type, KeySequenceEvent is not.

So comparing KeyEvent with KeyEvent is legal, comparing KeyEvent with KeySequenceEvent is also legal (and always returns false), but comparing KeySequenceEvent with KeySequenceEvent triggers a panic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants