Fix -startpos flag being ignored (#1129)

* Refactored cursor location login into a function. Fixed buffer overflow when line position is 1 more than file lines

* Fixed crash when -startpos has an invalid argument

* Adapted tests to new interface

* Fixed bug where -startpos with lines 0 and 1 would both be on the first line

* Changed Fatalf format back to digits

* Fixed issues with buffer cursor location. Added tests for new function

* ParseCursorLocation will now return an error when path doesnt contain line/col

* Fixed off-by-one line error

* Fixed tests to account for subtracting 1 from the line index
This commit is contained in:
Dimitar Borislavov Tasev 2018-06-04 17:27:27 +01:00 committed by Zachary Yedidia
parent 61baa73d70
commit aa74b1233c
4 changed files with 190 additions and 73 deletions

View file

@ -106,13 +106,12 @@ func NewBufferFromFile(path string) (*Buffer, error) {
// NewBufferFromString creates a new buffer containing the given string
func NewBufferFromString(text, path string) *Buffer {
return NewBuffer(strings.NewReader(text), int64(len(text)), path, []string{"0", "0"})
return NewBuffer(strings.NewReader(text), int64(len(text)), path, nil)
}
// NewBuffer creates a new buffer from a given reader with a given path
func NewBuffer(reader io.Reader, size int64, path string, cursorPosition []string) *Buffer {
cursorLocation, cursorLocationError := ParseCursorLocation(cursorPosition)
// check if the file is already open in a tab. If it's open return the buffer to that tab
if path != "" {
for _, tab := range tabs {
for _, view := range tab.Views {
@ -156,45 +155,9 @@ func NewBuffer(reader io.Reader, size int64, path string, cursorPosition []strin
os.Mkdir(configDir+"/buffers/", os.ModePerm)
}
// Put the cursor at the first spot
cursorStartX := 0
cursorStartY := 0
// If -cursorLocation LINE,COL was passed, use start position LINE,COL
if len(*flagStartPos) > 0 || cursorLocationError == nil {
positions := strings.Split(*flagStartPos, ",")
if len(positions) == 2 || cursorLocationError == nil {
var lineNum, colNum int
var errPos1, errPos2 error
if cursorLocationError == nil {
lineNum = cursorLocation.Y
colNum = cursorLocation.X
} else {
lineNum, errPos1 = strconv.Atoi(positions[0])
colNum, errPos2 = strconv.Atoi(positions[1])
}
if errPos1 == nil && errPos2 == nil {
cursorStartX = colNum
cursorStartY = lineNum - 1
// Check to avoid line overflow
if cursorStartY > b.NumLines {
cursorStartY = b.NumLines - 1
} else if cursorStartY < 0 {
cursorStartY = 0
}
// Check to avoid column overflow
if cursorStartX > len(b.Line(cursorStartY)) {
cursorStartX = len(b.Line(cursorStartY))
} else if cursorStartX < 0 {
cursorStartX = 0
}
}
}
}
cursorLocation, cursorLocationError := GetBufferCursorLocation(cursorPosition, b)
b.Cursor = Cursor{
Loc: Loc{
X: cursorStartX,
Y: cursorStartY,
},
Loc: cursorLocation,
buf: b,
}
@ -243,6 +206,52 @@ func NewBuffer(reader io.Reader, size int64, path string, cursorPosition []strin
return b
}
func GetBufferCursorLocation(cursorPosition []string, b *Buffer) (Loc, error) {
// parse the cursor position. The cursor location is ALWAYS initialised to 0, 0 even when
// an error occurs due to lack of arguments or because the arguments are not numbers
cursorLocation, cursorLocationError := ParseCursorLocation(cursorPosition)
// Put the cursor at the first spot. In the logic for cursor position the -startpos
// flag is processed first and will overwrite any line/col parameters with colons after the filename
if len(*flagStartPos) > 0 || cursorLocationError == nil {
var lineNum, colNum int
var errPos1, errPos2 error
positions := strings.Split(*flagStartPos, ",")
// if the -startpos flag contains enough args use them for the cursor location.
// In this case args passed at the end of the filename will be ignored
if len(positions) == 2 {
lineNum, errPos1 = strconv.Atoi(positions[0])
colNum, errPos2 = strconv.Atoi(positions[1])
}
// if -startpos has invalid arguments, use the arguments from filename.
// This will have a default value (0, 0) even when the filename arguments are invalid
if errPos1 != nil || errPos2 != nil || len(*flagStartPos) == 0 {
// otherwise check if there are any arguments after the filename and use them
lineNum = cursorLocation.Y
colNum = cursorLocation.X
}
// if some arguments were found make sure they don't go outside the file and cause overflows
cursorLocation.Y = lineNum - 1
cursorLocation.X = colNum
// Check to avoid line overflow
if cursorLocation.Y > b.NumLines-1 {
cursorLocation.Y = b.NumLines - 1
} else if cursorLocation.Y < 0 {
cursorLocation.Y = 0
}
// Check to avoid column overflow
if cursorLocation.X > len(b.Line(cursorLocation.Y)) {
cursorLocation.X = len(b.Line(cursorLocation.Y))
} else if cursorLocation.X < 0 {
cursorLocation.X = 0
}
}
return cursorLocation, cursorLocationError
}
// GetName returns the name that should be displayed in the statusline
// for this buffer
func (b *Buffer) GetName() string {

117
cmd/micro/buffer_test.go Normal file
View file

@ -0,0 +1,117 @@
package main
import (
"testing"
)
func TestGetBufferCursorLocationEmptyArgs(t *testing.T) {
buf := NewBufferFromString("this is my\nbuffer\nfile\nhello", "")
location, err := GetBufferCursorLocation(nil, buf)
assertEqual(t, 0, location.Y)
assertEqual(t, 0, location.X)
// an error is present due to the cursorLocation being nil
assertTrue(t, err != nil)
}
func TestGetBufferCursorLocationStartposFlag(t *testing.T) {
buf := NewBufferFromString("this is my\nbuffer\nfile\nhello", "")
*flagStartPos = "1,2"
location, err := GetBufferCursorLocation(nil, buf)
// note: 1 is subtracted from the line to get the correct index in the buffer
assertTrue(t, 0 == location.Y)
assertTrue(t, 2 == location.X)
// an error is present due to the cursorLocation being nil
assertTrue(t, err != nil)
}
func TestGetBufferCursorLocationInvalidStartposFlag(t *testing.T) {
buf := NewBufferFromString("this is my\nbuffer\nfile\nhello", "")
*flagStartPos = "apples,2"
location, err := GetBufferCursorLocation(nil, buf)
// expect to default to the start of the file, which is 0,0
assertEqual(t, 0, location.Y)
assertEqual(t, 0, location.X)
// an error is present due to the cursorLocation being nil
assertTrue(t, err != nil)
}
func TestGetBufferCursorLocationStartposFlagAndCursorPosition(t *testing.T) {
text := "this is my\nbuffer\nfile\nhello"
cursorPosition := []string{"3", "1"}
buf := NewBufferFromString(text, "")
*flagStartPos = "1,2"
location, err := GetBufferCursorLocation(cursorPosition, buf)
// expect to have the flag positions, not the cursor position
// note: 1 is subtracted from the line to get the correct index in the buffer
assertEqual(t, 0, location.Y)
assertEqual(t, 2, location.X)
assertTrue(t, err == nil)
}
func TestGetBufferCursorLocationCursorPositionAndInvalidStartposFlag(t *testing.T) {
text := "this is my\nbuffer\nfile\nhello"
cursorPosition := []string{"3", "1"}
buf := NewBufferFromString(text, "")
*flagStartPos = "apples,2"
location, err := GetBufferCursorLocation(cursorPosition, buf)
// expect to have the flag positions, not the cursor position
// note: 1 is subtracted from the line to get the correct index in the buffer
assertEqual(t, 2, location.Y)
assertEqual(t, 1, location.X)
// no errors this time as cursorPosition is not nil
assertTrue(t, err == nil)
}
func TestGetBufferCursorLocationNoErrorWhenOverflowWithStartpos(t *testing.T) {
text := "this is my\nbuffer\nfile\nhello"
buf := NewBufferFromString(text, "")
*flagStartPos = "50,50"
location, err := GetBufferCursorLocation(nil, buf)
// expect to have the flag positions, not the cursor position
assertEqual(t, buf.NumLines-1, location.Y)
assertEqual(t, 5, location.X)
// error is expected as cursorPosition is nil
assertTrue(t, err != nil)
}
func TestGetBufferCursorLocationNoErrorWhenOverflowWithCursorPosition(t *testing.T) {
text := "this is my\nbuffer\nfile\nhello"
cursorPosition := []string{"50", "2"}
*flagStartPos = ""
buf := NewBufferFromString(text, "")
location, err := GetBufferCursorLocation(cursorPosition, buf)
// expect to have the flag positions, not the cursor position
assertEqual(t, buf.NumLines-1, location.Y)
assertEqual(t, 2, location.X)
// error is expected as cursorPosition is nil
assertTrue(t, err == nil)
}
//func TestGetBufferCursorLocationColonArgs(t *testing.T) {
// buf := new(Buffer)
//}

View file

@ -13,6 +13,7 @@ import (
"github.com/mattn/go-runewidth"
"regexp"
"github.com/go-errors/errors"
)
// Util.go is a collection of utility functions that are used throughout
@ -363,9 +364,9 @@ func ReplaceHome(path string) string {
func GetPathAndCursorPosition(path string) (string, []string) {
re := regexp.MustCompile(`([\s\S]+?)(?::(\d+))(?::(\d+))?`)
match := re.FindStringSubmatch(path)
// no lines/columns were specified in the path, return just the path with cursor at 0, 0
// no lines/columns were specified in the path, return just the path with no cursor location
if len(match) == 0 {
return path, []string{"0", "0"}
return path, nil
} else if match[len(match)-1] != "" {
// if the last capture group match isn't empty then both line and column were provided
return match[1], match[2:]
@ -379,8 +380,8 @@ func ParseCursorLocation(cursorPositions []string) (Loc, error) {
var err error
// if no positions are available exit early
if len(cursorPositions) == 0 {
return startpos, err
if cursorPositions == nil {
return startpos, errors.New("No cursor positions were provided.")
}
startpos.Y, err = strconv.Atoi(cursorPositions[0])

View file

@ -150,64 +150,56 @@ func TestGetPathAbsoluteWindows(t *testing.T) {
assertEqual(t, path, "C:/myfile")
assertEqual(t, "10", cursorPosition[0])
assertEqual(t, cursorPosition[1], "5")
assertEqual(t, "5", cursorPosition[1])
}
func TestGetPathAbsoluteUnix(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("/home/user/myfile:10:5")
assertEqual(t, path, "/home/user/myfile")
assertEqual(t, "10", cursorPosition[0])
assertEqual(t, cursorPosition[1], "5")
assertEqual(t, "5", cursorPosition[1])
}
func TestGetPathRelativeWithDotWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("./myfile")
assertEqual(t, path, "./myfile")
assertEqual(t, "0", cursorPosition[0])
assertEqual(t, "0", cursorPosition[1])
// no cursor position in filename, nil should be returned
assertTrue(t, cursorPosition == nil)
}
func TestGetPathRelativeWithDotWindowsWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition(".\\myfile")
assertEqual(t, path, ".\\myfile")
assertEqual(t, "0", cursorPosition[0])
assertTrue(t, cursorPosition == nil)
assertEqual(t, "0", cursorPosition[1])
}
func TestGetPathRelativeNoDotWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("myfile")
assertEqual(t, path, "myfile")
assertEqual(t, "0", cursorPosition[0])
assertTrue(t, cursorPosition == nil)
assertEqual(t, "0", cursorPosition[1])
}
func TestGetPathAbsoluteWindowsWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("C:\\myfile")
assertEqual(t, path, "C:\\myfile")
assertEqual(t, "0", cursorPosition[0])
assertEqual(t, "0", cursorPosition[1])
assertTrue(t, cursorPosition == nil)
path, cursorPosition = GetPathAndCursorPosition("C:/myfile")
assertEqual(t, path, "C:/myfile")
assertEqual(t, "0", cursorPosition[0])
assertTrue(t, cursorPosition == nil)
assertEqual(t, "0", cursorPosition[1])
}
func TestGetPathAbsoluteUnixWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("/home/user/myfile")
assertEqual(t, path, "/home/user/myfile")
assertEqual(t, "0", cursorPosition[0])
assertTrue(t, cursorPosition == nil)
assertEqual(t, "0", cursorPosition[1])
}
func TestGetPathSingleLetterFileRelativePath(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("a:5:6")
@ -240,25 +232,22 @@ func TestGetPathSingleLetterFileAbsolutePathWindowsWithoutLineAndColumn(t *testi
path, cursorPosition := GetPathAndCursorPosition("C:\\a")
assertEqual(t, path, "C:\\a")
assertEqual(t, "0", cursorPosition[0])
assertEqual(t, "0", cursorPosition[1])
assertTrue(t, cursorPosition == nil)
path, cursorPosition = GetPathAndCursorPosition("C:/a")
assertEqual(t, path, "C:/a")
assertEqual(t, "0", cursorPosition[0])
assertEqual(t, "0", cursorPosition[1])
assertTrue(t, cursorPosition == nil)
}
func TestGetPathSingleLetterFileAbsolutePathUnixWithoutLineAndColumn(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("/home/user/a")
assertEqual(t, path, "/home/user/a")
assertEqual(t, "0", cursorPosition[0])
assertEqual(t, "0", cursorPosition[1])
assertTrue(t, cursorPosition == nil)
}
// TODO test for only line without a column
func TestGetPathRelativeWithDotOnlyLine(t *testing.T) {
path, cursorPosition := GetPathAndCursorPosition("./myfile:10")
@ -315,11 +304,12 @@ func TestParseCursorLocationTwoArgs(t *testing.T) {
assertEqual(t, nil, err)
}
func TestParseCursorLocationNoArgs(t *testing.T) {
location, err := ParseCursorLocation([]string{})
location, err := ParseCursorLocation(nil)
// the expected result is the start position - 0, 0
assertEqual(t, 0, location.Y)
assertEqual(t, 0, location.X)
assertEqual(t, nil, err)
// an error will be present here as the positions we're parsing are a nil
assertTrue(t, err != nil)
}
func TestParseCursorLocationFirstArgNotValidNumber(t *testing.T) {
// the messenger is necessary as ParseCursorLocation