#2391 Move SetCandidateWindowPos() out of if (imeEndToImeCaretU16 != 0) block.

[johnsonj]

    if (imeEndToImeCaretU16 != 0) {
        // Move back IME caret from current last position to imeCaretPos.
        const Sci::Position currentPos = CurrentPosition();
        const Sci::Position imeCaretPosDoc = pdoc->GetRelativePositionUTF16(currentPos, imeEndToImeCaretU16);

        MoveImeCarets(-currentPos + imeCaretPosDoc);

        if (std::find(imeIndicator.begin(), imeIndicator.end(), IndicatorTarget) != imeIndicator.end()) {
            // set candidate window left aligned to beginning of target string.
            SetCandidateWindowPos();
        }
    }

    What does imeEndToImeCaretU16 relates with SetCandidateWindowPos()?
    What guarantees there is no target input  if (imeEndToImeCaretU16 != 0)?
    Move  SetCandidateWindowPos() out of if (imeEndToImeCaretU16 != 0) block.

Related discussion:
https://sourceforge.net/p/scintilla/bugs/2390/?limit=25#8af1
https://sourceforge.net/p/scintilla/feature-requests/1304/

[Zufu Liu]

The code come from [feature-requests:#1300]:
https://sourceforge.net/u/madpilot78/scintilla/ci/7761c6b46433aad59b05dfdafc445bd53499d296/

[johnsonj]

The above link has no discussion why 2nd SetCandidateWindowPos() should be inside "if (imeEndToImeCaretU16 != 0) block".

[Zufu Liu]

Because otherwise the caret is not moved?
seems the correct code to align candidate window with target string is finding offset between ime.GetImeCaretPos() and target start (first IndicatorTarget in imeIndicator) .

[johnsonj]

Candidate window should be located at ime caret.
2nd SetCandidateWindowPos follows ime caret.
You should not try to control the ime caret.
It breaks scintilla ime from syncronizing with the internal state of IMM32.

I have had a hard time catching up with the recent bugs with TSF.
On the while, I managed to figure out what the purpose of 3 if checks is.
I wll continue on reverting the rest if (!onlyTarget) block with causes.

[johnsonj]

Now 2nd SetCandidateWindowPos() moves ut of "if check"
Let us take a close look at "if check" inside.

What if imeEndToImeCaretU16 is 0 ?
Then pdoc->GetRelativePositionUTF16(currentPos, 0);
So currentPos equals to imeCaretPosDoc
And then MoveImeCarets(0);
There is no moving, just addition and subtraction!.

In addition,
Within void ScintillaWin::MoveImeCarets(Sci::Position offset)
if offset is 0, then there is still no caret moving.

I do not check if offset is 0.
since benifits of speed is negligible.

And
I think readabilty is more important rather than efficency for IO-bound functon.

"if (imeEndToImeCaretU16 != 0) check" compromises readibility.
so It is reasonable it should be removed.

[Neil Hodgson]

Superseded by [feature-requests:#1488] which completely removes the second SetCandidateWindowPos in HandleCompositionInline.

[johnsonj]

Candidate window position gets now related with user's preference.
I feel it looks not bad visually while playing with japannes input.
In addition, less code feels me comfortable.