Repository: zyedidia/micro
File: internal/action/bufpane.go (Go)

Code before:

			names = append(names, a)
		} else if f, ok := BufKeyActions[a]; ok {
			afn = f
			names = append(names, a)
		} else if f, ok := BufMouseActions[a]; ok {
			afn = f
			names = append(names, a)
		} else {
			screen.TermMessage("Error in bindings: action", a, "does not exist")
			continue
		}
		actionfns = append(actionfns, afn)
	}
	bufAction := func(h *BufPane, te *tcell.EventMouse) bool {
		cursors := h.Buf.GetCursors()
		for i, a := range actionfns {
			success := true
			for j, c := range cursors {
				if c == nil {
					continue
				}
				h.Buf.SetCurCursor(c.Num)
				h.Cursor = c
				if i == 0 || (success && types[i-1] == '&') || (!success && types[i-1] == '|') || (types[i-1] == ',') {
					success = success && h.execAction(a, names[i], j, te)
				} else {
					break
				}
			}
			// if the action changed the current pane, update the reference
			h = MainTab().CurPane()
			if h == nil {
				// stop, in case the current pane is not a BufPane
				break
			}
		}
		return true
	}

	switch e := k.(type) {
	case KeyEvent, KeySequenceEvent, RawEvent:
		BufBindings.RegisterKeyBinding(e, BufKeyActionGeneral(func(h *BufPane) bool {
			return bufAction(h, nil)
		}))
	case MouseEvent:
		BufBindings.RegisterMouseBinding(e, BufMouseActionGeneral(bufAction))
	}
}

// BufUnmap unmaps a key or mouse event from any action

Review comment (bug):
I don't think this is equivalent to the previous logic. Also this seems buggy.
1. In the previous logic, if the action failed for the given cursor, we still keep executing it for the remaining cursors, before stopping the action chain. In your new logic, we stop as soon as the action failed, so some cursors may remain unserved.
2. More importantly: you reset `success` to true at each iteration of the outer loop, i.e. for each next action in the chain. So, `&` and `|` are not gonna work correctly at all?

Moreover: just removing this `success := true` from line 155 wouldn't be enough to fix that: it would mean that once `success` became false, it never becomes true again, so e.g. with `foo|bar&baz` chain, if `foo` failed but `bar` succeeded, `baz` is not executed: it still thinks that the previous action failed.

BTW, FYI `innerSuccess` was introduced in https://github.com/zyedidia/micro/commit/037c3c993fe81a258ed273fccfdfcb9183d7fafb as a part of the fix for #1721. Though I haven't analyzed what was the issue #1721 about and how exactly it relates to `innerSuccess`. But it looks like `innerSuccess` is needed for correct logic anyway?

Code after:

			names = append(names, a)
		} else if f, ok := BufKeyActions[a]; ok {
			afn = f
			names = append(names, a)
		} else if f, ok := BufMouseActions[a]; ok {
			afn = f
			names = append(names, a)
		} else {
			screen.TermMessage("Error in bindings: action", a, "does not exist")
			continue
		}
		actionfns = append(actionfns, afn)
	}
	bufAction := func(h *BufPane, te *tcell.EventMouse) bool {
		cursors := h.Buf.GetCursors()
		success := true
		for i, a := range actionfns {
			innerSuccess := true
			for j, c := range cursors {
				if c == nil {
					continue
				}
				h.Buf.SetCurCursor(c.Num)
				h.Cursor = c
				if i == 0 || (success && types[i-1] == '&') || (!success && types[i-1] == '|') || (types[i-1] == ',') {
					innerSuccess = innerSuccess && h.execAction(a, names[i], j, te)
				} else {
					break
				}
			}
			// if the action changed the current pane, update the reference
			h = MainTab().CurPane()
			success = innerSuccess
			if h == nil {
				// stop, in case the current pane is not a BufPane
				break
			}
		}
		return true
	}

	switch e := k.(type) {
	case KeyEvent, KeySequenceEvent, RawEvent:
		BufBindings.RegisterKeyBinding(e, BufKeyActionGeneral(func(h *BufPane) bool {
			return bufAction(h, nil)
		}))
	case MouseEvent:
		BufBindings.RegisterMouseBinding(e, BufMouseActionGeneral(bufAction))
	}
}