Fix bug in utf8 editing & set cursor

[aynakeya]

// 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 }}

[mjarkk]

One small thing

[dankox]

I'm not sure if this part is correct. I feel like it kinda kills the option where user would like to use dx higher than 1.

Basically if user do something like this

v.MoveCursor(4, 0)

it wouldn't move it by 4 characters, but instead it would move it by 1. It doesn't take into account the original dx value and override it with rune width.

[dankox]

Another thing is, is it really necessary to count columns by adding rune width? The reason I ask is, there is a fix in writeRune() function which adds for each rune width higher than 1 another column in to the line. Meaning you can just check the index instead of trying to figure out the real column, because you already have that information in the line length kinda.

Not sure if it's clear what I mean, but feel free to correct me. I just felt like this wouldn't be that necessary considering the fix in writeRune.

[aynakeya]

For this one, the function of moveCursor didn't change. so you can still use moveCursor to move exactly what you enter. Also, for usage outside the package, maybe I could change MoveCursor to MoveCursorWithRuneWidth and make MoveCursor same as 'moveCursor'?

The fix in writeRune works well, if you enter a wide rune, it the move cursor correctly.
However, if you have a wide rune character already, it will not move cursor correct. For example "烫|aaa", if you press left arrow, it will still at the same position "烫|aaa".

[dankox]

Oh yeah, moveCursor is ok, but that one is private. If you use this package in your application, you can only use the public version which is this one.

When using it in your application (for example when you need to overwrite default editor), and you would like to move cursor more than 1 character(rune) on X axis (left/right on line), it will always change dx to rune width only (which is in most cases 1).

To be honest, I don't think we need separate function because in my opinion MoveCursor should always move by runes, not by cells. Because you don't want to write in the middle of the rune. @mjarkk what do you think?

And with the fix in writeRune, this would be more consistent if the move would be by number of runes instead of cells.
However, for that to work correctly the dx counting should be done differently. Something like this:

	if dx != 0 {
		line := v.lines[newY]
		if dx < 0 {
			line = line[newX:v.cx] // not sure if the ranges are correct
		} else {
			line = line[v.cx:newX]
		}
		dx = 0
		for _, ch := range line {
			dx += runewidth.RuneWidth(ch.chr)
		}
	}

I guess the code needs to be fine-tuned to work correctly, but wanted to just show what I meant why the dx is not correct and how it could be fixed.

[aynakeya]

fixed

[dankox]

Doesn't really work :/ If you run _examples/demo.go you will see bunch of panics happening when scrolling in the text.

[dankox]

The view.MoveCursor function should be fixed in my opinion. I guess it will cause problems when used as described in my other comment.

[dankox]

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]

@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]

@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]

@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]

@dankox it works!