A patch for korean IME on the spot on win32

[johnsonj]

hi, korean hangul users.
I have solved UNDO problem at last.
I am pleased to give you this patch.
This is a patch for korean IME on the spot on win32.
Try it.

[Neil Hodgson]

Post by johnsonj
I have solved UNDO problem at last.

It isn't clear to me what the issue with undo is. The video seems to concentrate on multiple selection which could present issues with IMEs.

Turning undo on and off is dangerous. It is very easy to lose synchronisation and damage the document's integrity, particularly if not all entries and exits from IME composition mode are handled. Using a global static variable (compstrExist) may cause problems when there are multiple visible Scintilla windows. It may be better for this to be a variable on the ScintillaWin object to better handle applications like SciTE with edit and output panes.

Always returning to CARETSTYLE_LINE discards the application setting. There could be an 'imeCaretBlockOverride' field on Editor just for the use of IME code so that the inOverstrike and vs.caretStyle variables are not changed by the IME code and will stay as the application desires.

An assertion (undoSequenceDepth > 0) failure was shown when the overstrike mode was toggled (with the Ins key): empty UTF-8 document, Korean IME mode Han, enter "k", Ins, "k".

The variables collectUndo and dropUndo are actually constants and should be marked as such since, if these may change, then the behaviour will be much more complex. The UndoGroup scope should be indented - I misunderstood the behaviour at first.

HangulToHanja looks like it will break if the character at pos isn't 3 bytes for UTF-8 or 2 bytes for 949. My experience is more with Japanese IMEs where you may see in UTF-8 a 4 byte emoji or historical character or there may be ASCII after the caret when the Hanja Convert button is pressed. Document::LenChar can be used to see how wide the character at pos is. Maybe a failure here is unimportant.

Neil

[Neil Hodgson]

For compatibility, the indicator used for IME should be INDIC_MAX-1 since this is used for the IME on Cocoa where the definition in cocoa/ScintillaView.mm is

// The scintilla indicator used for keyboard input.
#define INPUT_INDICATOR INDIC_MAX - 1

This should be harmonised with a global definition in Scintilla.iface/Scintilla.h so that it can be used on different platforms. Qt uses #define INDIC_INPUTMETHOD 24.

Neil

[johnsonj]

Note that all of the patches are packed into one function:
HandleComposition()
It is desirible to split up into relevant places.
but for now I have no way. it works perfect. That's the point.

I can never understand scintilla's UNDO system, but I have experienced it
strong and robust.
my undo problem means my IME undo cannot adapt to scintilla's UNDO system.
Not to affect other places, there exist minimal codes between dropUndo and
collectUndo.
but in the first place, my IME undo does not work with virtual spaces mode
on.
I trust your code AddCharUTF(), try and study, finally I decded to split
for statement into two parts.
One handles virtual spaces, another handles display chars.
It solved my IME undo to adapt scintilla's UNDO system at last.

Is there a safe way to freeze UNDO?

HANJA is a virtual key. it belongs to Korean IME, neither Japanes nor
Chinese.
hanja, hangul and special chars all have three bytes.

Take a look at "void ScintillaWin::AddCharBytes(char b0, char b1)"
char utfval[4] = "\0\0\0";
char ansiChars[3];

As you defined, just as I do.

Thanks.

[Neil Hodgson]

I can never understand scintilla's UNDO system, but I have experienced it strong and robust.

Scintilla’s undo functionality is only robust because its rules are followed to avoid inconsistent states.

my undo problem means my IME undo cannot adapt to scintilla's UNDO system.
Not to affect other places, there exist minimal codes between dropUndo and collectUndo.

A problem here is that there is an open UndoGroup at the time that undo collection is turned off, characters inserted and then undo is turned on. The UndoGroup exists to write grouping information into the undo history which will fail if undo is turned off. One way to see a failure is to have a file with two lines, with the caret on the first line. Now add a second caret (Ctrl+Click) on the second line and turn on Hangul, type a character and you’ll see a failure in the UndoGroup destructor. The lifespan of an UndoGroup must not overlap turning undo on or off.

The patch is placing IME text into the document that is unknown to the undo system and has moved the text that is referenced in the undo history. Hence, undo can not be called when this IME text is present, since the undo history will be replayed in the wrong positions. The only thing that is safe to do when there is IME text present is to remove it all so that the undo system will be healed and is again synchronised with the document.

The patch removes the IME text in the first block that calls SetUndoCollection (if (compstrExist) … ). However, this code looks fragile to me since it is assuming that the selections are exactly the same as when the IME text was entered and that there is exactly one character of IME text at each selection. On Cocoa, there is similar use of SetUndoCollection but with more explicit retention of the IME text range.

It would be better to use the undo system for the IME text instead of turning undo off since that would retain synchronisation of the history with the document and could also make it easy to remove the IME text since that would just be undoing to the start point. When last I was working on the IME code for Cocoa I was looking at implementing something like that but I haven’t had the time to progress it.

Is there a safe way to freeze UNDO?

Not really. The SetUndoCollection call was never really meant for IMEs - it was meant for loading documents or when a document became read-only.

HANJA is a virtual key. it belongs to Korean IME, neither Japanes nor Chinese.
hanja, hangul and special chars all have three bytes.

The document does not necessarily have a three byte Korean character after the caret when HangulToHanja is called. For example, I have the ASCII text “The” in the document then place the caret at the document start and press Hanja Convert. The HangulToHanja function scoops up my ASCII characters and displays a menu - maybe it has something to do with “The”.

Attached is some of the work I did on adding ‘tentative text’ to Scintilla as a way of using the undo history to store IME text so that it could easily be removed. I can’t recall exactly what I meant by ‘sealed’ in this patch - it may just have been an extra check on the sequence of calls to ensure that there was no possibility of multiple additions occurring.

Neil

[johnsonj]

Thank you for your kind explanation.
freezing undo is very dangerous.
Your are absolutely right.
I am sorry for tentative.patch is above myself.

Instead of droping undos, I have reimplemented it by coalescing undos.
To coalesce undo steps between WM_IME_STARTCOMPOSITION and
WM_IME_ENDCOMPOSITION,
I have touched the variable mayCoalesce under BeginUndoAction and
EndUndoAction in CellBuffer.cxx.
I changed only the variable mayCoalesce to be true.

For flexible testing, I changed the names:
the methods BeginUndoCoalesce, EndUndoCoalesce and the variable
mayContinuing respectively.

This is the patch.
I think my patched methods may help implementing IME on the spot.
Please take a look.

====================================================================
Replace this function. This the main part.
file : ScintillaWin.cxx
----------------------------------------------------------------------------------------------------------------------------
sptr_t ScintillaWin::HandleComposition(uptr_t wParam, sptr_t lParam) {
bool koreanIME = (InputCodePage() == 949 || InputCodePage() == 1361);
if (koreanIME) {
// copy & paste by johnsonj
// Great thanks to
// jiniya from http://www.jiniya.net/tt/494 for DBCS input with
AddCharUTF()
// BLUEnLIVE from http://zockr.tistory.com/1118 for UNDO and
inOverstrike

HIMC hIMC = ::ImmGetContext(MainHWND());
if (!hIMC) {
return 0;
}

const int maxLenInputIME = 4;
wchar_t wcs[maxLenInputIME];
int wides = 0;
char hanval[maxLenInputIME];
unsigned int hanlen = 0;

bool tmpOverstrike = inOverstrike; // for avoiding inOverstrike
condition in AddCharUTF()

if (compstrExist) {
pdoc->BeginUndoCoalesce(true);
for (size_t r = 0; r < sel.Count(); r++) {
pdoc->DelChar(sel.Range(r).caret.Position());
}
pdoc->EndUndoCoalesce(true);
}

if (lParam & GCS_COMPSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_COMPSTR, wcs, maxLenInputIME);
wides = bytes / 2;
inOverstrike = inOverstrike && (!compstrExist);
compstrExist = (wides != 0);
} else if (lParam & GCS_RESULTSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_RESULTSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides == 0);
inOverstrike = inOverstrike && (wides >= 2);
}

if (wides >= 1) {
if (IsUnicodeMode()) {
hanlen = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, hanval, hanlen);
hanval[hanlen] = '\0';
} else {
hanlen = ::WideCharToMultiByte(InputCodePage() , 0
, wcs , wides , hanval ,
sizeof(hanval) - 1 , 0 , 0);
hanval[hanlen] = '\0';
}
if (compstrExist) {
vs.caretStyle = CARETSTYLE_BLOCK;

bool tmpRecordingMacro = recordingMacro;
recordingMacro = false;
pdoc->BeginUndoCoalesce(true);
AddCharUTF(hanval, hanlen, true);
pdoc->BeginUndoCoalesce(true);
recordingMacro = tmpRecordingMacro;

AutoCompleteCharacterDeleted();
Editor::NotifyChar((static_cast<unsigned char>(hanval[0])
<< 8) |
static_cast<unsigned char>(hanval[1]));

for (size_t r = 0; r < sel.Count(); r++) {
int positionInsert = sel.Range(r).Start().Position();
sel.Range(r).caret.SetPosition(positionInsert - hanlen);
sel.Range(r).anchor.SetPosition(positionInsert -
hanlen);
}
} else {
pdoc->BeginUndoCoalesce(true);
AddCharUTF(hanval, hanlen, true);
pdoc->BeginUndoCoalesce(true);
}
}

// set the candidate window position for HANJA
// have to be moved to WM_IME_NOTIFY
Point pos = PointMainCaret();
CANDIDATEFORM CandForm;
CandForm.dwIndex = 0;
CandForm.dwStyle = CFS_CANDIDATEPOS;
CandForm.ptCurrentPos.x = static_cast<int>(pos.x);
CandForm.ptCurrentPos.y = static_cast<int>(pos.y + vs.lineHeight);
::ImmSetCandidateWindow(hIMC, &CandForm);

ShowCaretAtCurrentPosition();
inOverstrike = tmpOverstrike;
::ImmReleaseContext(MainHWND(), hIMC);
return 0;
} else { // original code follows for other IMEs
if (lParam & GCS_RESULTSTR) {
HIMC hIMC = ::ImmGetContext(MainHWND());
if (hIMC) {
const int maxLenInputIME = 200;
wchar_t wcs[maxLenInputIME];
LONG bytes = ::ImmGetCompositionStringW(hIMC,
GCS_RESULTSTR, wcs, (maxLenInputIME-1)*2);
int wides = bytes / 2;
if (IsUnicodeMode()) {
char utfval[maxLenInputIME * 3];
unsigned int len = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, utfval, len);
utfval[len] = '\0';
AddCharUTF(utfval, len);
} else {
char dbcsval[maxLenInputIME * 2];
int size = ::WideCharToMultiByte(InputCodePage(),
0, wcs, wides, dbcsval, sizeof(dbcsval) - 1, 0, 0);
for (int i=0; i<size; i++) {
AddChar(dbcsval[i]);
}
}
// Set new position after converted
Point pos = PointMainCaret();
COMPOSITIONFORM CompForm;
CompForm.dwStyle = CFS_POINT;
CompForm.ptCurrentPos.x = static_cast<int>(pos.x);
CompForm.ptCurrentPos.y = static_cast<int>(pos.y);
::ImmSetCompositionWindow(hIMC, &CompForm);
::ImmReleaseContext(MainHWND(), hIMC);
}
return 0;
}
return ::DefWindowProc(MainHWND(), WM_IME_COMPOSITION, wParam,
lParam);
}
}

Replace these conditions.
--------------------------------------------------------------------------------------------------------------------
case WM_IME_STARTCOMPOSITION: { // dbcs
bool koreanIME = (InputCodePage() == 949 || InputCodePage() ==
1361);
if (koreanIME) { //by johnsonj
tmpCaretStyle = vs.caretStyle;
if (vs.caretStyle > CARETSTYLE_BLOCK) {
vs.caretStyle= CARETSTYLE_LINE;
}
return ::DefWindowProc(MainHWND(), iMessage, wParam,
lParam);
}
ImeStartComposition();
return ::DefWindowProc(MainHWND(), iMessage, wParam, lParam);
}

case WM_IME_ENDCOMPOSITION: { // dbcs
bool koreanIME = (InputCodePage() == 949 || InputCodePage() ==
1361);
if (koreanIME) { // by johnsonj
vs.caretStyle = tmpCaretStyle;
}
ImeEndComposition();
return ::DefWindowProc(MainHWND(), iMessage, wParam, lParam);
}


====================================================================
file : ScintillaWin.cxx
under : class ScintillaWin :
int tmpCaretStyle; // by johnsonj
bool compstrExist; //by johnsonj

under : ScintillaWin::ScintillaWin(HWND hwnd) {
tmpCaretStyle = 0; // by johnsonj
compstrExist = false; //by johnsonj

=====================================================================
file : CellBuffer.cxx
under : class CellBuffer {
void BeginUndoCoalesce(bool mayContinuing); //by johnsonj
void EndUndoCoalesce(bool mayContinuing); //by johnsonj
under : class UndoHistory {
void BeginUndoCoalesce(bool mayContinuing); //by johnsonj
void EndUndoCoalesce(bool mayContinuing); //by johnsonj

====================================================================
file : CellBuffer.h
void CellBuffer::BeginUndoCoalesce(bool mayContinuing) { //by johnsonj
uh.BeginUndoCoalesce(mayContinuing);
}
void CellBuffer::EndUndoCoalesce(bool mayContinuing) { //by johnsonj
uh.EndUndoCoalesce(mayContinuing);
}
=====================================================================
file : Document.h
under : class Document : PerLine, public IDocumentWithLineEnd, public
ILoader
void BeginUndoCoalesce(bool mayContinuing) {
cb.BeginUndoCoalesce(mayContinuing); } //by johnsonj
void EndUndoCoalesce(bool mayContinuing)
{cb.EndUndoCoalesce(mayContinuing); } //by johnsonj

[johnsonj]

I found This patch coalesce even Selection.
I squizeed my brain out but could not solved the problem.

Some rather dull compared to Ascii mode,
but It is possible to coalesce undos by character.
I think it works character by character for sure.

I hope there may not be a problem.
I reposted the patch.

file : ScintillaWin.cxx
=======================================================================
sptr_t ScintillaWin::HandleComposition(uptr_t wParam, sptr_t lParam) {
bool koreanIME = (InputCodePage() == 949 || InputCodePage() == 1361);
if (koreanIME) {
// copy & paste by johnsonj
// Great thanks to
// jiniya from http://www.jiniya.net/tt/494 for DBCS input with
AddCharUTF()
// BLUEnLIVE from http://zockr.tistory.com/1118 for UNDO and
inOverstrike

HIMC hIMC = ::ImmGetContext(MainHWND());
if (!hIMC) {
return 0;
}

const int maxLenInputIME = 4;
wchar_t wcs[maxLenInputIME];
int wides = 0;
char hanval[maxLenInputIME];
unsigned int hanlen = 0;

bool tmpOverstrike = inOverstrike; // for avoiding inOverstrike
condition in AddCharUTF()

if (compstrExist) {
pdoc->BeginUndoCoalesce(true);
for (size_t r = 0; r < sel.Count(); r++) {
pdoc->DelChar(sel.Range(r).caret.Position());
}
pdoc->EndUndoCoalesce(true);
}

if (lParam & GCS_COMPSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_COMPSTR, wcs, maxLenInputIME);
wides = bytes / 2;
inOverstrike = inOverstrike && (!compstrExist);
compstrExist = (wides != 0);
} else if (lParam & GCS_RESULTSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_RESULTSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides == 0);
inOverstrike = inOverstrike && (wides >= 2);
}

if (wides >= 1) {
if (IsUnicodeMode()) {
hanlen = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, hanval, hanlen);
hanval[hanlen] = '\0';
} else {
hanlen = ::WideCharToMultiByte(InputCodePage() , 0
, wcs , wides , hanval ,
sizeof(hanval) - 1 , 0 , 0);
hanval[hanlen] = '\0';
}
if (compstrExist) {
vs.caretStyle = CARETSTYLE_BLOCK;

bool tmpRecordingMacro = recordingMacro;
recordingMacro = false;
pdoc->BeginUndoCoalesce(true);
AddCharUTF(hanval, hanlen, true);
pdoc->EndUndoCoalesce(true);
recordingMacro = tmpRecordingMacro;

AutoCompleteCharacterDeleted();
Editor::NotifyChar((static_cast<unsigned char>(hanval[0])
<< 8) |
static_cast<unsigned char>(hanval[1]));

for (size_t r = 0; r < sel.Count(); r++) {
int positionInsert = sel.Range(r).Start().Position();
sel.Range(r).caret.SetPosition(positionInsert - hanlen);
sel.Range(r).anchor.SetPosition(positionInsert -
hanlen);
}
} else {
//pdoc->BeginUndoCoalesce(true);
AddCharUTF(hanval, hanlen, true);
//pdoc->EndUndoCoalesce(true);
}
}

// set the candidate window position for HANJA
// have to be moved to WM_IME_NOTIFY
Point pos = PointMainCaret();
CANDIDATEFORM CandForm;
CandForm.dwIndex = 0;
CandForm.dwStyle = CFS_CANDIDATEPOS;
CandForm.ptCurrentPos.x = static_cast<int>(pos.x);
CandForm.ptCurrentPos.y = static_cast<int>(pos.y + vs.lineHeight);
::ImmSetCandidateWindow(hIMC, &CandForm);

ShowCaretAtCurrentPosition();
inOverstrike = tmpOverstrike;
::ImmReleaseContext(MainHWND(), hIMC);
return 0;
} else { // original code follows for other IMEs
if (lParam & GCS_RESULTSTR) {
HIMC hIMC = ::ImmGetContext(MainHWND());
if (hIMC) {
const int maxLenInputIME = 200;
wchar_t wcs[maxLenInputIME];
LONG bytes = ::ImmGetCompositionStringW(hIMC,
GCS_RESULTSTR, wcs, (maxLenInputIME-1)*2);
int wides = bytes / 2;
if (IsUnicodeMode()) {
char utfval[maxLenInputIME * 3];
unsigned int len = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, utfval, len);
utfval[len] = '\0';
AddCharUTF(utfval, len);
} else {
char dbcsval[maxLenInputIME * 2];
int size = ::WideCharToMultiByte(InputCodePage(),
0, wcs, wides, dbcsval, sizeof(dbcsval) - 1, 0, 0);
for (int i=0; i<size; i++) {
AddChar(dbcsval[i]);
}
}
// Set new position after converted
Point pos = PointMainCaret();
COMPOSITIONFORM CompForm;
CompForm.dwStyle = CFS_POINT;
CompForm.ptCurrentPos.x = static_cast<int>(pos.x);
CompForm.ptCurrentPos.y = static_cast<int>(pos.y);
::ImmSetCompositionWindow(hIMC, &CompForm);
::ImmReleaseContext(MainHWND(), hIMC);
}
return 0;
}
return ::DefWindowProc(MainHWND(), WM_IME_COMPOSITION, wParam,
lParam);
}
}

[Neil Hodgson]

This post might be inappropriate. Click to display it.

[johnsonj]

Sorry I made a big mistake.
but this patch has also a problem with selection.
I think I should go back in the first place.
"Difficult come, Easy go."
I need more study.
Thank you very much for your kind comments.

Here is the ommitted part.
changed just one variable mayCoalesce to be true.
its affect is to link undos splitted by delchar into One unit.
Practically it works. Theoritically I don't know why.

Sorry again I troubled you.

CellBuffer.cxx
------------------------------------------------------------------------------------------
void UndoHistory::BeginUndoCoalesce(bool mayContinuing) { //by johnsonj
EnsureUndoRoom();
if (undoSequenceDepth == 0) {
if (actions[currentAction].at != startAction) {
currentAction++;
actions[currentAction].Create(startAction);
maxAction = currentAction;
}
actions[currentAction].mayCoalesce = mayContinuing;
}
undoSequenceDepth++;
}

void UndoHistory::EndUndoCoalesce(bool mayContinuing) { //by johnsonj
PLATFORM_ASSERT(undoSequenceDepth > 0);
EnsureUndoRoom();
undoSequenceDepth--;
if (0 == undoSequenceDepth) {
if (actions[currentAction].at != startAction) {
currentAction++;
actions[currentAction].Create(startAction);
maxAction = currentAction;
}
actions[currentAction].mayCoalesce = mayContinuing;
}
}

[Neil Hodgson]

While I'm probably missing a lot of behaviour, here is an attempt at using a simplified version of the "tentative" change merged into your changes. The important parts are that the Undo system is asked to remove the previous composition text at the top of the IME code and then, when it is inserting the new composition text, it first calls TentativeStart so the Undo system knows that this is temporary and may be removed.

Neil

[johnsonj]

I understand your intention.
I have ever used Undo and SetSavePoint to implement Korean IME.
It works, but runs too heavily. So I gave up the method.

If your Tentative undo works, I think it must be light.
But Your KoreanTentativ.patch does not work.
It has no effect.
I can not find out where uh.TentativeActive() is.

I am looking forward to your lightweight Tentative Undo works.

What do you think about my BeginUndoCoalesce and EndUndoCoalesce.
Is there any problem?

Thanks.

[Neil Hodgson]

Post by johnsonj
If your Tentative undo works, I think it must be light.
But Your KoreanTentativ.patch does not work.
It has no effect.
I can not find out where uh.TentativeActive() is.

UndoHistory::TentativeActive is in the CellBuffer.h header. You might need to do a complete recompile. Its defined as:

bool TentativeActive() const { return tentativePoint >= 0; }

Post by johnsonj
What do you think about my BeginUndoCoalesce and EndUndoCoalesce.
Is there any problem?

I'm not sure. I've been trying to work out what you are trying to achieve and it looks like the point of the BeginUndoCoalesce and EndUndoCoalesce is to wrap the insertion and subsequent deletion of the composition string into a single undo action so that if you use the Undo command, you don't see the intermediate states with/without the composition string. It seems to work but I haven't analysed it deeply so there may still be problems.

For this to work the steps always have to occur in the same order: (insertion of composition string; deletion of composition string) * n; insertion of result string. Other IMEs I have worked with have had different possible sequences, for example abandoning the character being composed but that does not appear to be possible with the Korean IME. I don't really know how to drive it, though.

What I don't like about Begin/EndUndoCoalesce is that it is still storing a lot of uninteresting states in undo that the user will never want to retrieve. It also doesn't allow coalescing the typing of a complete string as is done for ASCII so you have to undo Korean text character by character which can be slow.

The tentative undo code was initially developed for the OS X IME for Japanese which is quite complex allowing a multiple character composition string with one character being active and various underlines to show modes. It was never properly finished as I was uncertain about various aspects.

Neil

[johnsonj]

Thank you very much for your great works.
TentativKorean.patch works perfect,
but it does not go well with virtual spaces.

I think pdoc->TentativeStart() has to be put somewhere in
Editor::AddCharUTF().
I have been trying but have no result.

[Neil Hodgson]

Post by johnsonj
TentativKorean.patch works perfect,
but it does not go well with virtual spaces.
I think pdoc->TentativeStart() has to be put somewhere in Editor::AddCharUTF().

Extracting the code that fills in the virtual space would be one technique, or providing an argument to AddCharUTF so that it only runs its virtual space code. However, it appears that if AddCharUTF is called with a length of 0 then it will fill in the virtual spaces. There may be some other side-effects but this seems to work: towards the top of the Korean composition handling detect that this is the first call for this composition and call AddCharUTF("", 0) before entering tentative mode. Like this change over KoreanTentative.patch:

if (pdoc->TentativeActive()) {
pdoc->TentativeUndo();
} else {
// No tentative undo means start of this composition so
// fill in any virtual spaces.
AddCharUTF("", 0, false);
}

Neil

[johnsonj]

How beautiful it is!
I think it works perfect.
I feel quite relieved.
I do really thank you for your hard works.

A patch by winmerge attathed

[Neil Hodgson]

Post by johnsonj
A patch by winmerge attathed

That isn't cleanly applying since ScintillaWin.cxx has changed since the version you are using. With patches, unified format is better than normal format since it can handle some changes in the target file.

In WinMerge, inside the Patch Generator dialog, look for the Style pull-down inside the Format section and choose Unified. Or send me your ScintillaWin.cxx and I'll use WinMerge to merge it.

Neil

[johnsonj]

sorry I have troubled you.

I have lots of new things from your kind instructions.
thank you.

Scite is the best editor. I love it.

scintillaWin.cxx has been too littered to be diffed.
I did clean it all my best according to scintilla 344.

Here is my patch and scintiliaWin.cxx

[Neil Hodgson]

Post by johnsonj
scintillaWin.cxx has been too littered to be diffed.
I did clean it all my best according to scintilla 344.

Yes, I recently made several changes to ScintillaWin.cxx for the fine grained timer feature and some of those changes are adjacent to the Korean changes which makes it more difficult to merge.

Post by johnsonj
Here is my patch and scintiliaWin.cxx

Committed as
https://sourceforge.net/p/scivntilla/code/ci/5f234d4bb4e0642eacd33afb030498c5b5e6b2e2/

Made some minor changes to scopes and formatting.

Neil

[johnsonj]

I find out my inOverstrike condition is unnecessary.
TentativeUndo throws even my inOverstrike condition away.
But I appreciate that.
HandleCompositionKoreanIme get simpler.

my scintillaWin.cxx has got very littered.
It is difficult to post diffed patch.
Sorry for that.

Have a look "inOverstrike"
I added some codes around AddCharUTF("", 0, false);
My inOverstrike conditon was deleted.
How clean and clear Anybody can understand !
Here follows my patch
========================================================================

sptr_t ScintillaWin::HandleCompositionKoreanIME(uptr_t wParam, sptr_t
lParam) {

// copy & paste by johnsonj
// Great thanks to
// jiniya from http://www.jiniya.net/tt/494 for DBCS input with
AddCharUTF()
// BLUEnLIVE from http://zockr.tistory.com/1118 for UNDO and
inOverstrike

HIMC hIMC = ::ImmGetContext(MainHWND());
if (!hIMC) {
return 0;
}

const int maxLenInputIME = 4;
wchar_t wcs[maxLenInputIME];
int wides = 0;
char hanval[maxLenInputIME];
unsigned int hanlen = 0;

if (pdoc->TentativeActive()) {
pdoc->TentativeUndo();
} else {
// No tentative undo means start of this composition so
// fill in any virtual spaces.
bool tmpOverstrike = inOverstrike;
inOverstrike = false; // not allow to be deleted twice.
AddCharUTF("", 0, false);
inOverstrike = tmpOverstrike;
}

if (lParam & GCS_COMPSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_COMPSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides != 0);
} else if (lParam & GCS_RESULTSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_RESULTSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides == 0);
}

if (wides >= 1) {

if (IsUnicodeMode()) {
hanlen = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, hanval, hanlen);
hanval[hanlen] = '\0';
} else {
hanlen = ::WideCharToMultiByte(InputCodePage() , 0
, wcs , wides , hanval , sizeof(hanval) -
1 , 0 , 0);
hanval[hanlen] = '\0';
}

if (compstrExist) {
vs.caretStyle = CARETSTYLE_BLOCK;
bool tmpRecordingMacro = recordingMacro;
recordingMacro = false;
pdoc->TentativeStart();
AddCharUTF(hanval, hanlen, true);
recordingMacro = tmpRecordingMacro;

//NotifyChar() in AddCharUTF() may not know comprStr is deleted
by undo.
Editor::NotifyChar((static_cast<unsigned char>(hanval[0]) << 8)
|
static_cast<unsigned char>(hanval[1]));

for (size_t r = 0; r < sel.Count(); r++) { // for block caret
int positionInsert = sel.Range(r).Start().Position();
sel.Range(r).caret.SetPosition(positionInsert - hanlen);
sel.Range(r).anchor.SetPosition(positionInsert - hanlen);
}
} else {
AddCharUTF(hanval, hanlen, true);
}
}

// set the candidate window position for HANJA while composing.
Point pos = PointMainCaret();
CANDIDATEFORM CandForm;
CandForm.dwIndex = 0;
CandForm.dwStyle = CFS_CANDIDATEPOS;
CandForm.ptCurrentPos.x = static_cast<int>(pos.x);
CandForm.ptCurrentPos.y = static_cast<int>(pos.y + vs.lineHeight);
::ImmSetCandidateWindow(hIMC, &CandForm);

ShowCaretAtCurrentPosition();
::ImmReleaseContext(MainHWND(), hIMC);
return 0;
}
===========================================================================

[Neil Hodgson]

Post by johnsonj
I find out my inOverstrike condition is unnecessary.
TentativeUndo throws even my inOverstrike condition away.
But I appreciate that.
HandleCompositionKoreanIme get simpler.

Is there any need to remember compstrExist between calls to HandleCompositionKoreanIME? Shouldn't it be local to that method and removed from the class?

Neil

[johnsonj]

With undo implementation, It is not necessary in IME routine
But I have seen lots of Korean IMEs have a flag indicating "while
composing".
compstrExist means that. Visually it looks block caret.

I am not sure but it may be used other places for future.
for example, use it to block the block caret to follow mouse.
if you type a compstr in while selecting,
composition window(block caret) is parted away from the compstr, following
mouse.

It is better to be ready for on the spot IME.
So I have compstrExist to be alive.

But it's ok to make it removed for clean code.
You have keen eyes.

TentativeUndo() beats allmost all my hard works.
but I get simple, intuitive and beautiful Korean IME.
Thank you very much.

[Neil Hodgson]

Post by johnsonj
With undo implementation, It is not necessary in IME routine
But I have seen lots of Korean IMEs have a flag indicating "while composing".
compstrExist means that. Visually it looks block caret.

OK. I wasn't sure of the precise lifetime since it isn't reset (for example) in WM_IME_ENDCOMPOSITION. Platform APIs are more likely to be accurate if what is wanted is "Is there an active IME composition string?".

I would still like to avoid changing vs.caretStyle for the IME by adding another variable just for IME use like imeCaretBlockOverride which would be set to true in HandleCompositionKoreanIME and false in WM_IME_ENDCOMPOSITION. Then there would be no need for tmpCaretStyle and the possibility of it being wrong if, for example, the application changed caret style while the IME was active.

Neil

[johnsonj]

My sources get very dirty. I touched a lot of things.
It is very difficult to make diff .
sorry for that.

Would you please just take a look?

I removed global variables, compstrExist and tmpCaretStyle.
now comptstrExist is local.
I added a global variable imeCaretBlockOverride instead of tmpCaretStyle.
I put it under docwatcher in editor.h and init it under editor() in
editor.cxx.
I patch a line to add imeCaretBlockOverride as follows.
Editor.cxx
--------------------------------------------------------------------------------------------------------
void Editor::DrawCarets(Surface *surface, const ViewStyle &vsDraw, int
lineDoc, int xStart,
PRectangle rcLine, LineLayout *ll, int subLine) {
..............
..............
} else if ((vsDraw.caretStyle == CARETSTYLE_BLOCK) ||
imeCaretBlockOverride) {
..............
-------------------------------------------------------------------------------------------------------
I am not sure this is correct!
I do not know where to put and how to handle it.
If you show me a good solution, I would appreciate that.

I wonder what you say.
I hope you like it!!!

===============================================================
case WM_IME_STARTCOMPOSITION: { // dbcs
if (KoreanIME()) {
return 0;
}
ImeStartComposition();
return ::DefWindowProc(MainHWND(), iMessage, wParam, lParam);
}

case WM_IME_ENDCOMPOSITION: { // dbcs
ImeEndComposition();
return ::DefWindowProc(MainHWND(), iMessage, wParam, lParam);
}

case WM_IME_COMPOSITION:
if (KoreanIME()) {
return HandleCompositionKoreanIME(wParam, lParam);
} else {
return HandleComposition(wParam, lParam);
}

===============================================================

sptr_t ScintillaWin::HandleCompositionKoreanIME(uptr_t wParam, sptr_t
lParam) {

// copy & paste by johnsonj
// Great thanks to
// jiniya from http://www.jiniya.net/tt/494 for DBCS input with
AddCharUTF()
// BLUEnLIVE from http://zockr.tistory.com/1118 for UNDO and
inOverstrike

HIMC hIMC = ::ImmGetContext(MainHWND());
if (!hIMC) {
return 0;
}

const int maxLenInputIME = 200;
wchar_t wcs[maxLenInputIME];
int wides = 0;
char hanval[maxLenInputIME];
unsigned int hanlen = 0;
bool compstrExist = false;

if (pdoc->TentativeActive()) {
pdoc->TentativeUndo();
} else {
// No tentative undo means start of this composition so
// fill in any virtual spaces.
bool tmpOverstrike = inOverstrike;
inOverstrike = false; // not allow to be deleted twice.
AddCharUTF("", 0, false);
inOverstrike = tmpOverstrike;
}

imeCaretBlockOverride = false;
if (lParam & GCS_COMPSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_COMPSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides != 0);
} else if (lParam & GCS_RESULTSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_RESULTSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides == 0);
}

if (wides >= 1) {

if (IsUnicodeMode()) {
hanlen = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, hanval, hanlen);
hanval[hanlen] = '\0';
} else {
hanlen = ::WideCharToMultiByte(InputCodePage() , 0
, wcs , wides , hanval , sizeof(hanval) -
1 , 0 , 0);
hanval[hanlen] = '\0';
}

if (compstrExist) {
imeCaretBlockOverride = true;
bool tmpRecordingMacro = recordingMacro;
recordingMacro = false;
pdoc->TentativeStart();
AddCharUTF(hanval, hanlen, true);
recordingMacro = tmpRecordingMacro;

//NotifyChar() in AddCharUTF() may not know comprStr is deleted
by undo.
Editor::NotifyChar((static_cast<unsigned char>(hanval[0]) << 8)
|
static_cast<unsigned char>(hanval[1]));

for (size_t r = 0; r < sel.Count(); r++) { // for block caret
int positionInsert = sel.Range(r).Start().Position();
sel.Range(r).caret.SetPosition(positionInsert - hanlen);
sel.Range(r).anchor.SetPosition(positionInsert - hanlen);
}
} else {
AddCharUTF(hanval, hanlen, true);
}
}

// set the candidate window position for HANJA while composing.
Point pos = PointMainCaret();
CANDIDATEFORM CandForm;
CandForm.dwIndex = 0;
CandForm.dwStyle = CFS_CANDIDATEPOS;
CandForm.ptCurrentPos.x = static_cast<int>(pos.x);
CandForm.ptCurrentPos.y = static_cast<int>(pos.y + vs.lineHeight);
::ImmSetCandidateWindow(hIMC, &CandForm);

ShowCaretAtCurrentPosition();
::ImmReleaseContext(MainHWND(), hIMC);
return 0;
}
============================================================================

[Neil Hodgson]

Post by johnsonj
const int maxLenInputIME = 200;

That was 4 before. Is there any reason to expect long strings?

Post by johnsonj
I removed global variables, compstrExist and tmpCaretStyle.
now comptstrExist is local.

I think that is good.

Post by johnsonj
I added a global variable imeCaretBlockOverride instead of tmpCaretStyle.
I put it under docwatcher in editor.h and init it under editor() in editor.cxx.

OK, you are still using the 3.4.4 release whereas I am using the current code in the Hg repository (https://sourceforge.net/p/scintilla/code/ci/default/tree/) and Editor has recently been broken up with some code going into EditView. For me, imeCaretBlockOverride goes into EditView. My patch is attached.

Post by johnsonj
I patch a line to add imeCaretBlockOverride as follows.
Editor.cxx
--------------------------------------------------------------------------------------------------------
void Editor::DrawCarets(Surface *surface, const ViewStyle &vsDraw, int lineDoc, int xStart,
PRectangle rcLine, LineLayout *ll, int subLine) {
..............
..............
} else if ((vsDraw.caretStyle == CARETSTYLE_BLOCK) || imeCaretBlockOverride) {
..............
-------------------------------------------------------------------------------------------------------
I am not sure this is correct!

That change is good.

Scintilla tries to keep the caret visible. When there is a block caret, it tries to keep the right side of that character visible. There should also be another change for CARETSTYLE_BLOCK:

@@ -1246,7 +1246,7 @@
newXY.xOffset = static_cast<int>(pt.x + xOffset - rcClient.left) - 2;
} else if (pt.x + xOffset >= rcClient.right + newXY.xOffset) {
newXY.xOffset = static_cast<int>(pt.x + xOffset - rcClient.right) + 2;
- if (vs.caretStyle == CARETSTYLE_BLOCK) {
+ if ((vs.caretStyle == CARETSTYLE_BLOCK) || view.imeCaretBlockOverride) {
// Ensure we can see a good portion of the block caret
newXY.xOffset += static_cast<int>(vs.aveCharWidth);
}

This is a minor issue, it may just show a little more of the right hand context when composing and the window is narrow so horizontal scrolling is active.

Post by johnsonj
I wonder what you say.
I hope you like it!!!

I like simpler code and this is simpler. It should also allow the GTK+ code to be simpler.

Neil

[johnsonj]

this is cut away.
It is impossible to edit.
so reposted.

===============================================================

sptr_t ScintillaWin::HandleCompositionKoreanIME(uptr_t wParam, sptr_t
lParam) {

// copy & paste by johnsonj
// Great thanks to
// jiniya from http://www.jiniya.net/tt/494 for DBCS input with
AddCharUTF()
// BLUEnLIVE from http://zockr.tistory.com/1118 for UNDO and
inOverstrike

HIMC hIMC = ::ImmGetContext(MainHWND());
if (!hIMC) {
return 0;
}

const int maxLenInputIME = 200;
wchar_t wcs[maxLenInputIME];
int wides = 0;
char hanval[maxLenInputIME];
unsigned int hanlen = 0;
bool compstrExist = false;

if (pdoc->TentativeActive()) {
pdoc->TentativeUndo();
} else {
// No tentative undo means start of this composition so
// fill in any virtual spaces.
bool tmpOverstrike = inOverstrike;
inOverstrike = false; // not allow to be deleted twice.
AddCharUTF("", 0, false);
inOverstrike = tmpOverstrike;
}

imeCaretBlockOverride = false;
if (lParam & GCS_COMPSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_COMPSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides != 0);
} else if (lParam & GCS_RESULTSTR) {
long bytes = ::ImmGetCompositionStringW
(hIMC, GCS_RESULTSTR, wcs, maxLenInputIME);
wides = bytes / 2;
compstrExist = (wides == 0);
}

if (wides >= 1) {

if (IsUnicodeMode()) {
hanlen = UTF8Length(wcs, wides);
UTF8FromUTF16(wcs, wides, hanval, hanlen);
hanval[hanlen] = '\0';
} else {
hanlen = ::WideCharToMultiByte(InputCodePage() , 0
, wcs , wides , hanval , sizeof(hanval) -
1 , 0 , 0);
hanval[hanlen] = '\0';
}

if (compstrExist) {
imeCaretBlockOverride = true;
bool tmpRecordingMacro = recordingMacro;
recordingMacro = false;
pdoc->TentativeStart();
AddCharUTF(hanval, hanlen, true);
recordingMacro = tmpRecordingMacro;

//NotifyChar() in AddCharUTF() may not know comprStr is deleted
by undo.
Editor::NotifyChar((static_cast<unsigned char>(hanval[0]) << 8)
|
static_cast<unsigned char>(hanval[1]));

for (size_t r = 0; r < sel.Count(); r++) { // for block caret
int positionInsert = sel.Range(r).Start().Position();
sel.Range(r).caret.SetPosition(positionInsert - hanlen);
sel.Range(r).anchor.SetPosition(positionInsert - hanlen);
}
} else {
AddCharUTF(hanval, hanlen, true);
}
}

// set the candidate window position for HANJA while composing.
Point pos = PointMainCaret();
CANDIDATEFORM CandForm;
CandForm.dwIndex = 0;
CandForm.dwStyle = CFS_CANDIDATEPOS;
CandForm.ptCurrentPos.x = static_cast<int>(pos.x);
CandForm.ptCurrentPos.y = static_cast<int>(pos.y + vs.lineHeight);
::ImmSetCandidateWindow(hIMC, &CandForm);

ShowCaretAtCurrentPosition();
::ImmReleaseContext(MainHWND(), hIMC);
return 0;
}
============================================================================

[johnsonj]

Post by johnsonj
const int maxLenInputIME = 200;

That was 4 before. Is there any reason to expect long strings?

When I first touch Korean IME, I only think it is OK Korean IME works.
From now on I would like to take other IMEs into consideration.
I will follow scintilla's rule as possible as I can.

I can not catch current source tree.
Especially in recent days it has changed too much.
My scintilla source also has been littered two much.
Unitl when 3.4.5 release, I am going to concentrate on Korean IME on gtk
2.0.
No more patch!

Thanks for all your hard work.

[Neil Hodgson]

Post by johnsonj
When I first touch Korean IME, I only think it is OK Korean IME works.
From now on I would like to take other IMEs into consideration.
I will follow scintilla's rule as possible as I can.

OK. The recent changes were committed as
https://sourceforge.net/p/scintilla/code/ci/f72c4145b64d80e40802b448e1be24c70b6b2feb/

Post by johnsonj
I can not catch current source tree.
Especially in recent days it has changed too much.
My scintilla source also has been littered two much.
Unitl when 3.4.5 release, I am going to concentrate on Korean IME on gtk 2.0.

3.4.5 will be in about 3 weeks. I don't want to release early because there have been some large changes like the refactoring and more efficient timers so want everyone to have a good opportunity to try it and report problems before release.

Neil