awesome-gocui / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
350 stars 39 forks source link

Fix bug in utf8 editing & set cursor #104

Closed aynakeya closed 2 years ago

aynakeya commented 3 years ago
// delete this because MoveCursor() don't really need to update the gui.
// it has no effect.
v.gui.userEvents <- userEvent{func(g *Gui) error { return nil }}
dankox commented 2 years ago

Sorry for my late review. I was a bit swamped and didn't have time to look into it.

I've added some comments and some other suggestions (which can be resolved in other PR).

dankox commented 2 years ago

@aynakeya The latest changes are causing panics in _examples/demo.go program. They also don't support scrolling to next line when at the end of line or to the previous line if at the beginning.

Here is what I tried and what works for me:

// MoveCursor mores the cursor relative from it's current possition
func (v *View) MoveCursor(dx, dy int) {
    newY := v.cy + dy
    // do nothing when no line exists
    if len(v.lines) == 0 {
        v.cx, v.cy = 0, 0
        return
    }
    // If newY is more than all lines set it to the last line
    if newY >= len(v.lines) {
        newY = len(v.lines) - 1
    }
    if newY < 0 {
        newY = 0
    }
    line := v.lines[newY]
    // compute the rune delta
    if dx != 0 && len(line) > 0 {
        callNext := true
        idx := v.cx
        if dx < 0 {
            callNext = false
            dx = -dx // make dx positive ;)
        }
        for dx > 0 {
            if callNext {
                idx = v.nextRuneIdx(idx, newY)
            } else {
                idx = v.prevRuneIdx(idx, newY)
            }
            dx--
        }
        dx = idx - v.cx
    }
    v.moveCursor(dx, dy)
}

func (v *View) nextRuneIdx(x, y int) int {
    if len(v.lines[y]) < x {
        return x
    }
    x++
    for x < len(v.lines[y]) && v.lines[y][x].chr == '\x00' {
        x++
    }
    return x
}

func (v *View) prevRuneIdx(x, y int) int {
    if len(v.lines[y]) < x {
        return x
    }
    x--
    for x > 0 && v.lines[y][x].chr == '\x00' {
        x--
    }
    return x
}

Not sure if it's the best solution, but it works on assumption that all the "real" runes are specified in the cell and when there is \0 character in the cell it is because of wide rune before it.
The real problem with runewidth is that no matter what it returns, you need to move in the internal cell buffer which is slice and hence doesn't matter what's the size of the rune, rather it matters where the rune starts (or is placed) in the buffer.

Also, the v.prevRuneIdx can be used in EditDelete function to fix backspace deleting.

dankox commented 2 years ago

@aynakeya @mjarkk So I just couldn't stop thinking about this problem, and I think the issue is not with the way we use writeRune or MoveCursor.

The original idea from what I can see across the code is that internal cell buffer always represent one specific rune which is written in it.
Therefore there is no need to add the empty cells with \x00 rune for it to display correctly. This is already presented in the view.setRune function (although there is one small bug with empty cells created by SetWritePos) which propagate those cells into underlaying tcell and when the rune width is bigger it skip the next cell in tcell.

The real problem is in inconsistent usage of cell buffer and in certain places applying rune width to it and in other places not. This leads to behaviour which is confusing because sometimes the cursor is in one spot while in reality is in the other spot.

@aynakeya I will create a new PR for you to check if this would resolve your problems with the wide characters, to see if it's really "just" that. This PR will also address @coloursofnoise problem with table.go example and "empty" cells not displayed correctly.

dankox commented 2 years ago

@aynakeya can you please check the newest changes in the master? New PR was merged (#109) which should fix the wide runes problem. Hopefully it resolves any problem you had before too, which you were trying to fix with this PR.

Let us know if that helps and resolves your problem, or if there is still something to it.

aynakeya commented 2 years ago

@dankox it works!