#2516 GetColumn() and FindColumn() doesn't handle Unicode line endings

[Zufu Liu]

Not sure whether is a real bug but Document::GetColumn() and Document::FindColumn() doesn't handle Unicode line endings. the upper for loop should be cb.LineEnd(line) or cb.LineEnd(line) + 1 instead of LengthNoExcept() + "\r\n".

[Zufu Liu]

The two pdoc->GetColumn() inside void Editor::DelCharBack(bool allowLineStartDeletion) can be extracted.

if (pdoc->GetColumn(sel.Range(r).caret.Position()) <= pdoc->GetLineIndentation(lineCurrentPos) &&
        pdoc->GetColumn(sel.Range(r).caret.Position()) > 0 && pdoc->backspaceUnindents) {

The GetColumn() inside void Editor::Indent(bool forwards, bool lineIndent) seems can be replaced with some form of pdoc->FindColumn(newColumn)?

const Sci::Position newColumn = std::max<Sci::Position>(0,
    ((column - 1) / pdoc->tabInChars) * pdoc->tabInChars);
Sci::Position newPos = caretPosition;
while (pdoc->GetColumn(newPos) > newColumn)
    newPos--;
sel.Range(r) = SelectionRange(newPos);

[Zufu Liu]

The two pdoc->GetColumn() inside void Editor::DelCharBack(bool allowLineStartDeletion) can be extracted.

both pdoc->GetColumn() and pdoc->GetLineIndentation(lineCurrentPos) can be extracted.

[Neil Hodgson]

GetColumn is also bounded by pos whch must be on the line. It doesn't appear to matter for the uses that that unicode line ends return +1 compared to common line ends. A cb.LineEnd can be added but it adds work.

Many of the GetColumn are >0 so really asking 'is this position after line start' which is cheaper to calculate with IsLineStartPosition.

FindColumn will loop too far with Unicode line ends but there will be no visual effect since the calculated position is outside the line so is benign for beyond edge colouring. The edge discovery calling code knows the line length so could pass that in.

The extra calculations in DelCharBack can be hoisted although the condition could also be reordered to avoid or minimizing calculations. Its also a candidate for moving to Document except for updating the selection.

GetColumn ... seems can be replaced with some form of pdoc->FindColumn

OK.

I did previously try to move some of Indent into Document since it only really interacts with Editor by updating one selection range but didn't commit as it also added complexity. Attached the work done on this.

[Zufu Liu]

for edge discovery, something like following might be enough (edgePosition >= posLineStart is always true):

- Sci::Position edgePosition = model.pdoc->FindColumn(line, vstyle.theEdge.column)
- if (edgePosition >= posLineStart) {
-   edgePosition -= posLineStart;
- }
+ const Sci::Position edgePosition = model.pdoc->FindColumn(line, vstyle.theEdge.column) - posLineStart;
+ ll->edgeColumn = std::min(ll->numCharsBeforeEOL, static_cast<int>(edgePosition));

void SciTEBase::GoMessage(int dir) (inside SciTEBuffers.cxx) could apply similar limit.

startSourceLine = wEditor.FindColumn(sourceLine, column);

+ startSourceLine = std::min(startSourceLine, endSourceline);

edit: previous code for limiting column parameter is incorrect due to tab expanding.

[Neil Hodgson]

Here's some unit tests for a changed GetColumn. ForceLineEndTypesAllowed is a testing only API that can turn on Unicode line ends with no check, The commented out REQUIRE calls are the result from the current GetColumn and the next line is from an updated version which changes the results for positions inside a character and thus inside a Unicode line end. It changes result for beyond end from 1 to 0 which seems reasonable to me.

SECTION("GetColumn") {
    DocPlus doc("ab\tc\xCE\x93\r\nd\xe2\x80\xa8z", CpUtf8);
    doc.document.ForceLineEndTypesAllowed(LineEndType::Unicode);

    // Outside document
    REQUIRE(0 == doc.document.GetColumn(-1));
    //REQUIRE(1 == doc.document.GetColumn(1000));
    REQUIRE(0 == doc.document.GetColumn(1000));

    REQUIRE(0 == doc.document.GetColumn(0));    // a
    REQUIRE(1 == doc.document.GetColumn(1));    // b
    REQUIRE(2 == doc.document.GetColumn(2));    // \t
    REQUIRE(8 == doc.document.GetColumn(3));    // c
    REQUIRE(9 == doc.document.GetColumn(4));    // gamma[0]
    //REQUIRE(10 == doc.document.GetColumn(5)); // gamma[1]
    REQUIRE(9 == doc.document.GetColumn(5));    // gamma[1]
    REQUIRE(10 == doc.document.GetColumn(6));   // \r
    REQUIRE(10 == doc.document.GetColumn(7));   // \n

    REQUIRE(0 == doc.document.GetColumn(8));    // d
    REQUIRE(1 == doc.document.GetColumn(9));    // LS[0]
    //REQUIRE(2 == doc.document.GetColumn(10)); // LS[1]
    REQUIRE(1 == doc.document.GetColumn(10));   // LS[1]
    //REQUIRE(2 == doc.document.GetColumn(11)); // LS[2]
    REQUIRE(1 == doc.document.GetColumn(11));   // LS[2]

    REQUIRE(0 == doc.document.GetColumn(12));   // z
    REQUIRE(1 == doc.document.GetColumn(13));   // end of document

    //REQUIRE(1 == doc.document.GetColumn(14)); // 1 after end of document
    REQUIRE(0 == doc.document.GetColumn(14));   // 1 after end of document
}

Sci::Position Document::GetColumn(Sci::Position pos) const noexcept {
if ((pos < 0) || (pos > LengthNoExcept())) {
    return 0;
}
Sci::Position column = 0;
const Sci::Line line = SciLineFromPosition(pos);
for (Sci::Position i = cb.LineStart(line); i < pos;) {
    const char ch = cb.CharAt(i);
    if (ch == '\t') {
        column = NextTab(column, tabInChars);
        i++;
    } else if ((ch == '\r') || (ch == '\n')) {
        return column;
    } else if (UTF8IsAscii(ch)) {
        column++;
        i++;
    } else {
        i = NextPosition(i, 1);
        if (i > pos) {
            // For pos inside multibyte character report column before
            return column;
        }
        column++;
    }
}
return column;

}

[Zufu Liu]

I'm prefer to retain current truncation behavior:

Sci::Position Document::GetColumn(Sci::Position pos) const noexcept {
Sci::Position column = 0;
const Sci::Line line = SciLineFromPosition(pos);
//if ((line >= 0) && (line < LinesTotal())) {
    const Sci::Position length = LengthNoExcept();
    pos = std::min(pos, length);
    for (Sci::Position i = cb.LineStart(line); i < pos;) {
        const char ch = cb.CharAt(i);
        if (ch == '\t') {
            column = NextTab(column, tabInChars);
            i++;
        } else if (ch == '\n' || ch == '\r') {
            return column;
        } else if (UTF8IsAscii(ch)) {
            column++;
            i++;
        } else {
            column++;
            i = NextPosition(i, 1);
        }
    }
//}
return column;
}

[Neil Hodgson]

That retains the original problem with the unexpected treatment of Unicode EOLs with different results from the positions inside the Unicode EOL.