AddCharacter(std::string s, std::string utf8notify, addCharacterFlags)

[johnsonj]

needs instructions.

[johnsonj]

* 3 conditions to cotrol addCharUTF
=================================
1. inOverstrike
-. addCharUTF
2. notify
3. macro

struct addCharacterFlags {
    bool allowOverstrike
    bool notifyUtf32
    bool recordMacro
}

* direct control try
if inOverstrike && allowOverstrike

if notifyUtf32
   notify
if recordMacro
    record Macro

* 4 states of ime composition.
---------------------------------------------
1. inCompletion -> record
2. inComposition
3. lastInputAtCursor -> notify
4. inReconversion -> disable overstrike.

imeState { inCompletion, inComposition, inReconversion };
inReconversion { inCompletion, inComposition };
inCompletion { normalInput }
inComposition { normalInput, blockInput, otherInput };
normalInput { firstInput, midInput, lastInput inputAtCursor}

* direct control, all defaults true;
                  notifyUtf32   recordMacro  Overstrike
--------------------------------------------------------------------------
firstInput             X            X           
midInput               X            X
lastInputAtCursor      O            X           
block(other)Input      X            X           
inReconversion                                   O

* simplified try
                       notify     macro
==========================================
1. inCompletion                     O
2  inComposition                    X
5. lastInputAtCursor     O          X

addCharacterFlags { inCompletion, inComposition, lastInputAtCursor }

if inCompletion and lastInputAtCursor
   notify
if inCompletion
   record macro

* more simplified try
                       notify     macro
==========================================
1. inCompletion          O          O
2  inComposition         X          X

addCharacterFlag { inCompletion, inComposition }

if inCompletion and inComposition
   notify
if inCompletion
   record macro

* most simplified try
                      macro
==========================================
1. recordMacro          O

bool recordMacro

if recordMacro
   record macro

[johnsonj]

used StringEncode instead of the existing codes inScintillaWin::HandleCompositionInline
used std::string utf8 instead of utf32.
used s[0] instead of *s in ac.IsFillUpChar(s[0]);

[Neil Hodgson]

johnsonj:

> used StringEncode instead of the existing codes inScintillaWin::HandleCompositionInline

OK.

> used std::string utf8 instead of utf32.

Probably OK. I thought an int would be simpler than a structured type.

> used s[0] instead of *s in ac.IsFillUpChar(s[0]);

Fillups and stop characters are currently limited to a single byte. This should be extended at some point to multi-bye characters and this looks like it may be an opportunity to do so. With an int utf32 argument, I was thinking that a fast test for whether a character is in a set could be implemented. However, searching a 100K string for each character isn’t unreasonable on current hardware so maybe we change the search from fillUpChars.find(ch) to fillUpChars.find(utf8).

There are a couple of choices here - fillUpChars can either be in the document encoding (possibly DBCS) or in UTF-8. The problem with DBCS is that searching for a character in a string isn’t safe without using a DBCS-compliant search function (not std::string::find) since characters like may be found as part of other characters. In Shift-JIS, for example, ‘h’ may be either an actual ‘h’ or the trail byte of a Japanese character. UTF-8 is self-synchonizing with no character contained in any other character so using find is safe.

Bit-masks are more conventional for flags arguments than a struct. It may be clearer to invert the ‘recordEnabled’ and ‘overstrikeEnabled’ names since they are used to turn off the otherwise default behaviour rather than forcing it on. Or change ‘…Enabled’ to ‘allow…’ since it is for an individual act instead of an ongoing state. The ‘notifyEnabled’ flag is simply causing the notification.

In the notification branch, there should be no check for size or special casing of bytes: there must be exactly one character which can be extracted from the utf8Notify variable. There could be an assertion at the top that s and utf8Notify are non-empty and otherwise reasonable to cause errors in debug builds.

Neil

[johnsonj]

"""Fillups and stop characters are currently limited to a single byte."""

Yes, a very difficult force. too much for me.

"""fillUpChars can either be in the document encoding (possibly DBCS) or in UTF-8."""

so it is convenient to have a pair of std::string docChar and  std::string utf8Char.

""" Bit-masks are more conventional for flags arguments than a struct. """

I have changed parameters a lot. I find struct more flexible. I thank signature does not have to be changed.

"""It may be clearer to invert the ‘recordEnabled’ and ‘overstrikeEnabled’ names"""

changed xxxEnabled to allowXXX.

"""there must be exactly one character which can be extracted from the utf8Notify variable."""

I am thinking about 'std::vector<std::string> s' like this:

void Editor::AddCharacter(std::vector<std::string> docChar, std::vector<std::string> utf8Char, addCharacterFlags flags);

more flexible but burdensome.

* patch attached changes
- no need ClearBeforeTentativeStart, AddCharUTF() is restructured into AddCharacter();
- removed recording ime positions

[johnsonj]

Factors tested with addCharacterFlags
================================================
    factors             positive        reason
fillVirtualSpace           X      control level too deep
TentativeStart             X      control level too deep
imeBlockCaret              X      variable duration too long
inOverstrike               X      variable duration too long
notify                     X      It seems Ok to reflect all changes
macro                      O

Now addCharacterFlags have only one member left, allowRecord
struct addCharacterFlags {
    bool allowRecord
}

It is unlikely for addCharacter() to be changed no more for future.
so I suggest like this signature:

void AddCharacter(std::string docChar, std::string utf8Char="", bool allowMacro=true)

I will follow your instruction.

[johnsonj]

another signature following your instruction.
AddCharacter(const std::string docChar, int utf32, addCharacterFlags flags)

editor.h added 3 coditions
+    struct addCharacterFlags {
+       bool allowNotify;
+       bool allowRecord;
+       bool allowOverstrike;
+       addCharacterFlags(): allowNotify(true), allowRecord(true), allowOverstrike(true) { }
+    };
but only allowRecord being used.

Notify branch changed a lot.

*ClearTentaveStart fixed.

+    FilterSelections();
+    {
+        UndoGroup ug(pdoc, (sel.Count() > 1) || !sel.Empty() || inOverstrike);

ClearBeforeTentativeStart in ScintillWin moved under if (lParam & GCS_COMPSTR) following document comments.

I used utf8 std::string to get utf32 code point following your path.

[Neil Hodgson]

johnsonj:

> """there must be exactly one character which can be extracted from the utf8Notify variable."""
> I am thinking about 'std::vector<std::string> s' like this:
>
> void Editor::AddCharacter(std::vector<std::string> docChar, std::vector<std::string> utf8Char, addCharacterFlags flags);

The logic in AddCharUTF is very strongly character at a time. You don’t want to add several characters then call NotifyChar several times since the application may be expecting each change to have an immediate notification. A convenience method could go around AddCharacter to insert strings of characters. There are also some benefits to handling a string together such as minimising calls to (expensive) WrapOneLine and EnsrureCaretVisible but I don’t think that outweighs confused apps.

> void AddCharacter(std::string docChar, std::string utf8Char="", bool allowMacro=true)

It shouldn’t be too hard for callers to always provide Unicode data so I don’t think utf8Char (or alternatively int utf32) should be optional. Using a bool for the last argument isn’t easy to extend and is part of why we are here. At least make it an enum for now:
enum AddCharacterFlags { addDefault, addRecord }; or {addDefault, addNoRecord} if recording is the common case.

> + struct addCharacterFlags {

Its a class / struct so capital first letter AddCharacterFlags. Methods also start with a cap, and local variables start with lower case so NoTentativeUndo->noTentativeUndo.

If fewer flags are needed then that would be great.

> Notify branch changed a lot.

Join conditions together when they reduce the changes from before:

if (inOverstrike && flags.allowOverstrike) {

instead of

if (inOverstrike) {
if (flags.allowOverstrike) {

Same for recording.

There is still a lot of code that should not be needed. Much simpler:

if (flags.allowNotify) {
NotifyChar(utf32);
}

This should eventually become NotifyCharacter(utf32, docChar) so that the text will also be passed up in SCNotification::text.

> *ClearTentaveStart fixed.
>
> + FilterSelections();
> + {
> + UndoGroup ug(pdoc, (sel.Count() > 1) || !sel.Empty() || inOverstrike);
>
> ClearBeforeTentativeStart in ScintillWin moved under if (lParam & GCS_COMPSTR) following document comments.

noTentativeUndo is just remembering the state of tentative undo early in code so should be something like

const bool tentativeUndoWasActive = doc->TentativeActive();

or (with inverse meaning)

const bool initialCompose = !doc->TentativeActive();

> I used utf8 std::string to get utf32 code point following your path.

Encapsulating the conversion to UTF-32 in a function would avoid extra variables in the complex IME handling and could be reused so you can say:

AddCharacter(docChar, UTF32CharacterFromUTF8(utf8Char));

Neil

[johnsonj]

""" This should eventually become NotifyCharacter(utf32, docChar) so that the text will also be passed up in SCNotification::text. """

I am afraid your path will be very complicated.
It appears docChar will be of no use.
Will NotifyCharacter(utf8Char) be enough in future?

Here is my humble thought.
About 'int utf32(unicode)' vs. 'std::string utf8'

1. Scintilla uses text as is, not converting text into unicode or something.
2. Text should be compared in document string, not in unicode.
3. Unicode should be used for external interaction with Os or other applications.

Using only UTF8 rather than DBCS solves all the problems.

      current                  transient                    future
-------------------------------------------------------------------
If IsUnicodeMode() {           s = 'utf8'                  s = 'utf8'  
   s = 'utf8'                  If !IsUniCodeMode() {
} else {                          s = convert('dbcs')
   s = convert('dbcs')         }
}

current:  utf8 and dbcs are on same level.
transient : utf8 will be primary.
future: all std::string must be assumed as utf8.

It seems OK to remove supporting DBCS at least within Scintilla Control.
It should be set encoding to edit or view multiby characters correctly.
so encoding can be used to provide converting service between Scintilla and Applications.
and Scintilla can treat all charcters as in utf8 internally.

AddCharacter(std::string utf8Char, AddCharacterFlags flag).
AddCharacter(std::string docChar, AddCharacterFlags flag).

Converting utf32 to utf8 string will unpack a doument character.
If IsUnicodeMode(), its encoding is utf8, otherwise DBCS.

In conclusion, I have a future version in mind.
but I will follow your path.

Thank you for instructions!
* copied
---------------------------------------
AddCharacterFlags
bool initialCompose
inOverstrike && flags.allowOverstrike
UTF32CharacterFromUTF8(utf8Char)

[johnsonj]

Fixed according to your instructions.

Now it is not impossible calling AddCharacter(docChar).

changed docChar[0] to utf32 in ScintillaBase::AddCharacter()
    bool isFillUp = ac.Active() && ac.IsFillUpChar(utf32);
    AutoCompleteCharacterAdded(utf32);

[johnsonj]

Now free from maxLenInputIME,
get more C++ style of STL.

[Neil Hodgson]

johnsonj:

> Now free from maxLenInputIME,
> get more C++ style of STL.

This could be taken a little further, encapsulating the length discovery and text retrieval into a method on IMContext like:

std::wstring GetCompositionString(DWORD dwIndex) {
const LONG byteLen = ::ImmGetCompositionStringW(hIMC, dwIndex, NULL, 0);
std::wstring wcs(byteLen / 2, 0);
::ImmGetCompositionStringW(hIMC, dwIndex, &wcs[0], byteLen);
return wcs;
}

This is then used like

const std::wstring wcs = imc.GetCompositionString(GCS_COMPSTR);

Later code can use wcs.length() instead of unattached variables like wcsLen which might be updated separately.

The same thing could be done for the GCS_COMPATTR retrieval although is for a byte string and there is only one case retrieving a byte string so adding a method doesn’t reduce the amount of code.

The expression wcs.substr(0, wcs.length()) is the substring of was from its start to end so is equivalent to wcs in

int posAtLast = StringEncode(wcs.substr(0, wcs.length()), codePage).size();

> I am afraid your path will be very complicated.
> It appears docChar will be of no use.
> Will NotifyCharacter(utf8Char) be enough in future?
>
> Here is my humble thought.
> About 'int utf32(unicode)' vs. 'std::string utf8'
>
> 1. Scintilla uses text as is, not converting text into unicode or something.
> 2. Text should be compared in document string, not in unicode.
> 3. Unicode should be used for external interaction with Os or other applications.

Not sending through the UTF-32 character for Unicode files in SCNotification::ch would upset many applications without much benefit. It may have been a better approach if done initially but back then most applications were really only interested in ASCII and it works fine for that.

If (2) then there needs to be a way of correctly checking fillUpChars and stopChars for DBCS character strings taking lead bytes and tail bytes into account. This could be done by Document since it understands which bytes are lead and tail but that requires making the document or code page visible to AutoComplete.

For the more intensive cases like checking for word characters when searching, using a string search to discover if a character is a member of a set may be too slow. The CharacterSet class could be extended to efficiently hold large sets of characters as bit arrays for rapid look up by indexing.

> It seems OK to remove supporting DBCS at least within Scintilla Control.
> It should be set encoding to edit or view multiby characters correctly.

Its not just DBCS. This would also cause problems for very simple applications which only handle single-byte character sets.

Neil

[johnsonj]

""" const std::wstring wcs = imc.GetCompositionString(GCS_COMPSTR);"""

Thank you for guiding me!. more simple and more easy reading.

""" Not sending through the UTF-32 character for Unicode files in SCNotification::ch would upset many applications without much benefit. """

Thank you for instructions. I will have it in mind.

patch attached following your instructions.

[johnsonj]

removed wcsLen

removed checking GCS_COMPATTR and GCS_CURSORPOS

moved GCS_CURSORPOS near MoveImeCarets()

names changed to widthToLast, widthToCursor and inputStyle

added filteselection() to Editor::ClearBeforeTentativeStart()

[Neil Hodgson]

Most of the changes appear viable without AddCharacter so they could be separated out and made as a first stage. This would minimise differences between AddCharUTF and AddCharacter.

That could allow non-core platform layers to have some time to move from AddCharUTF to AddCharacter without too many inconsistencies. Taking 6 months or so to transition may be OK.

Using “const std::string” for method arguments is disliked by some of the static checkers. Making it a reference “const std::string &” keeps them happy.

Neil

[johnsonj]

Thank you for hard works.

Let me advance little by little.
While trial and error I find virtual spaces are not undoed in one unit.
so add FilterSelection() and UndoGroup ug() to ClearBeforeTentativeStart().

[Neil Hodgson]

johnsonj:

> Let me advance little by little.
> While trial and error I find virtual spaces are not undoed in one unit.
> so add FilterSelection() and UndoGroup ug() to ClearBeforeTentativeStart().

Committed without the extra scope:
http://sourceforge.net/p/scintilla/code/ci/b7d9df70211e3353ce2096888a1f29eec894f86f/

In AddCharUTF, there is a scope around the UndoGroup so that the group is completed before the post-insertion code is run. For ClearBeforeTentativeStart the UndoGroup is completed at the end of the method and an extra scope doesn’t do anything.

Neil

[johnsonj]

Changed ime codes for style of STL.

Next, I will change GCS_RESULTSTR, HandlecompositonWindowed and AddCharUTF16 by add AddWString().

[Neil Hodgson]

johnsonj:

> Changed ime codes for style of STL.
>
> Next, I will change GCS_RESULTSTR, HandlecompositonWindowed and AddCharUTF16 by add AddWString().

Yes. There is duplicated code in the loops for GCS_RESULTSTR and GCS_COMPSTR branches that could be combined.

Neil

[johnsonj]

changed duplicated codes with AddWString(std::wstrining wcs): GCS_RESULTSTRING, AddCharUTF16().
Next I will add SetCompositionWindowPos();

[Neil Hodgson]

johnsonj:

> changed duplicated codes with AddWString(std::wstrining wcs): GCS_RESULTSTRING, AddCharUTF16().

Why does AddWString return a length which is never used?

Neil

[johnsonj]

It was added by design for later use.

I have tested this patch scheme for gtk and qt, too.
ime codes, especially of Qt, are factored out to be slim.

Current my test signature for AddCharUTF():
win: AddWString(std::wstring ws, std::vector<int> indicator);
gtk: AddUTF8String(std::string s, std::vector<int> indicator);
win: AddQString(QString qs, std::vector<int> indicator);

Here is a contour patch for AddCharacter(), just for a disscusion.
addCharacterFlags is good for consistent function signature,
but it is too flexible, it might be dirty with this scheme I am afraid.

[Neil Hodgson]

johnsonj:

> It was added by design for later use.

Hmmm.

> Here is a contour patch for AddCharacter(), just for a disscusion.
> addCharacterFlags is good for consistent function signature,
> but it is too flexible, it might be dirty with this scheme I am afraid.

It does appear to be worthwhile to have AddCharacter perform the character marking with indicators. It implements protection for read-only text so may refuse to insert text for some selections. MoveImeCarets currently upsets this by then modifying each selection even if it included protected text. Protected text can be experimented with by adding the notchangeable attribute to styles in SciTE. Another issue to think about is how IME input should interact with applications that want to use SC_MOD_INSERTCHECK to convert entered text: possibly allow the application to change GCS_RESULTSTR but hide GCS_COMPSTR as it is just temporary.

The patch uses a vector to pass indicators to AddCharacter but it only appears necessary to pass a single value since it is just for one character.

Neil

[johnsonj]

Thank you for instructions.

"""   It does appear to be worthwhile to have AddCharacter perform the character marking with indicators. It implements protection for read-only text so may refuse to insert text for some selections. """

"""   The patch uses a vector to pass indicators to AddCharacter but it only appears necessary to pass a single value since it is just for one character. """

I have tried to draw indicator in AddCharacter, but in fail.
It appers indicator will revive due to UndoGroup.
but I do not know why exactly yet.
so I decide to put DrawImeIndicator() under AddWString(),
therefore I have to use the signature of AddWString like this:

AddWString(std::wstring ws, std::vector<int> indicator, addCharacterFlags flag);

"""MoveImeCarets currently upsets this by then modifying each selection even if it included protected text. Protected text can be experimented with by adding the notchangeable attribute to styles in SciTE. """
"""Another issue to think about is how IME input should interact with applications that want to use SC_MOD_INSERTCHECK to convert entered text: possibly allow the application to change GCS_RESULTSTR but hide GCS_COMPSTR as it is just temporary."""

The name 'MoveImeCaret' means it intends to work within ime context.
so I have never dreamed about SC_MOD_INSERTCHECK.
Although MoveImeCarets seems to need to be fixed for SC_MOD_INSERTCHECK.
but I do not know how to handle it.

I will use the returned length to guard DrawImeIndicator and MoveImeCarets.
Now AddCharacter signature have be like this:

int AddCharacter(std::wstring ws, int utf32, addCharacterFlags flag)

Here is patch attaced within my ability following your instruction.
DrawImeIndicator was left at Editor for testing.
Please just take a review.

[johnsonj]

I succeded moving DrawImeIndicator to Editor.
Putting it outside for loop works for drawing indicator correctly.

and about protecting ime indicator: Is it OK to guard like this?

        if (!RangeContainsProtected(start, end)) {
            pdoc->DecorationFillRange(start - len, 1, len);
        }

and about SC_MOD_INSERTCHECK: I could find nothing.

[johnsonj]

DrawImeIndicator moves into AddCharacter() like this.
This should solve all concerns about indicator.

                if (lengthInserted > 0) {
                    currentSel->caret.SetPosition(positionInsert + lengthInserted);
                    currentSel->anchor.SetPosition(positionInsert + lengthInserted);
                    if (flags.indicator != 0) {
                        if (lengthAdded == static_cast<int>(docChar.size())) {
                            if ((7 < flags.indicator) && (flags.indicator < INDIC_MAX)) {
                                pdoc->decorations.SetCurrentIndicator(flags.indicator);
                                pdoc->DecorationFillRange(positionInsert, 1, lengthInserted);
                            }
                        }
                    }
                }

[Neil Hodgson]

johnsonj:

> DrawImeIndicator moves into AddCharacter() like this.
> This should solve all concerns about indicator.
>
> if (lengthInserted > 0) {
> currentSel->caret.SetPosition(positionInsert + lengthInserted);
> currentSel->anchor.SetPosition(positionInsert + lengthInserted);
> if (flags.indicator != 0) {
> if (lengthAdded == static_cast<int>(docChar.size())) {

This fixes a current bug with read-only mode (Options | Read Only in SciTE) where spurious IME indicators could appear.

It may be more robust to simply accept the lengthInserted as the length for the indicator. This would allow for the insert check feature inserting a shorter or longer text. Not that we are committed to insert check for IME but its a possibility.

> if ((7 < flags.indicator) && (flags.indicator < INDIC_MAX)) {

No need to check the indicator here: if its non-zero assume its valid since anything else is a bug in the calling code inside Scintilla.

Neil

[johnsonj]

I somtimes feel like a dumb.
It is nice to solve ime bugs about indicator.
I have not noticed 'read only' menu in SciTE until you pointed up.
Thank you!

"""Another issue to think about is how IME input should interact with applications that want to use SC_MOD_INSERTCHECK to convert entered text: possibly allow the application to change GCS_RESULTSTR but hide GCS_COMPSTR as it is just temporary."""

I could have found no clue on how to protect GCS_COMPSTR by searching SC_MOD_INSERTCHECK.
How can I test it?

[johnsonj]

The existing ime codes are so fragile.
At last, if (pdoc->IsReadOnly()) handling is added.
 
sptr_t ScintillaWin::HandleCompositionInline(uptr_t, sptr_t lParam) {
     // Copy & paste by johnsonj with a lot of helps of Neil.
     // Great thanks for my foreruners, jiniya and BLUEnLIVE.
 
     IMContext imc(MainHWND());
-    if (!imc.hIMC) {
+    if (!imc.hIMC)
+        return 0;
+    if (pdoc->IsReadOnly()) {
+        ::ImmNotifyIME(imc.hIMC, NI_COMPOSITIONSTR, CPS_CANCEL, 0);
         return 0;
     }

Patch attached following your instruction.
Thanks for your hard works.

[Neil Hodgson]

johnsonj:

-    if (!imc.hIMC) {
+    if (!imc.hIMC)
+        return 0;
+    if (pdoc->IsReadOnly()) {
+        ::ImmNotifyIME(imc.hIMC, NI_COMPOSITIONSTR, CPS_CANCEL, 0);

   Committed.

http://sourceforge.net/p/scintilla/code/ci/61abfcb07e9fc255e908421b746e3151e390e149/

   Also changed some other IME code that sets up the font which only worked correctly for first 32 styles.

http://sourceforge.net/p/scintilla/code/ci/248325f930f2ca1a510d9f454f6c921e2ad9ee4e/

I could have found no clue on how to protect GCS_COMPSTR by searching SC_MOD_INSERTCHECK.
How can I test it?

   There’s no prepared code for insert check so you could add a temporary handler to SciTE for testing. See the original insert check post for some ideas:

https://groups.google.com/d/msg/scintilla-interest/eVi6bobA2tw/Tdq2UJfuqrYJ

   For determining whether the insertion is compstr or resultstr, there isn’t (currently) a direct method but I think TentativeActive() implies compstr.

   Neil

[johnsonj]

Thank your for the tip.

ime patch attached for lunux.

diff -r 61abfcb07e9f gtk/ScintillaGTK.cxx
--- a/gtk/ScintillaGTK.cxx    Mon Dec 07 11:13:48 2015 +1100
+++ b/gtk/ScintillaGTK.cxx    Mon Dec 07 12:54:30 2015 +0900
@@ -2434,6 +2434,11 @@

     // Copy & paste by johnsonj with a lot of helps of Neil

     // Great thanks for my foreruners, jiniya and BLUEnLIVE

     try {
+        if (pdoc->IsReadOnly()) {
+            gtk_im_context_reset(im_context);
+            return;
+        }
+
         view.imeCaretBlockOverride = false; // If backspace.
 
         if (pdoc->TentativeActive()) {

gtk_im_context_reset() intends for clearing preedit context.
it is equvalent to CompleteComposition on win ime.
commit string (if any) will return with "commit" event. but it will be rejected in pdoc->InsertString();
At any rate, IsReadOnly() check intends for blocking ime carets movent rather than ime input.

[johnsonj]

RangeContainsProtected() is one thing, IsReadOnly() is another.
I found out no way to block ime carets from moving when IsReadOnly().
I suggest, Is this change allowable?

- virtual void AddCharUTF(const char *s, unsigned int len, bool treatAsDBCS=false);
+ virtual int AddCharUTF(const char *s, unsigned int len, bool treatAsDBCS=false);

lengthAdded is in need for the existing ime.

[Neil Hodgson]

johnsonj:

I suggest, Is this change allowable?

- virtual void AddCharUTF(const char *s, unsigned int len, bool treatAsDBCS=false);
+ virtual int AddCharUTF(const char *s, unsigned int len, bool treatAsDBCS=false);

lengthAdded is in need for the existing ime.

   Possibly, but the implementation in the …1208.patch looks like a bad idea, specifically

return lengthAdded/sel.Count();

   which won’t deal well with allowing or disallowing some additions. 

   Another approach may be to simply disallow IME character addition if any selection is protected. Or drop any selections that are protected.

--- a/gtk/ScintillaGTK.cxx    Mon Dec 07 11:13:48 2015 +1100
+++ b/gtk/ScintillaGTK.cxx    Mon Dec 07 12:54:30 2015 +0900
@@ -2434,6 +2434,11 @@
     // Copy & paste by johnsonj with a lot of helps of Neil
     // Great thanks for my foreruners, jiniya and BLUEnLIVE
     try {
+        if (pdoc->IsReadOnly()) {
+            gtk_im_context_reset(im_context);
+            return;
+        }
+

   OK.

http://sourceforge.net/p/scintilla/code/ci/9a9ba1b746314d1065934994bc54f00e66dbcb64/

   While canceling the IME for read-only is an OK approach, it doesn’t allow a technique that was included in the read-only feature originally. When a modification attempt is made on a read-only document, a SCN_MODIFYATTEMPTRO notification is sent to the application which can make the document writeable if it wants. This allows the application to perform some external action such as locking the document in an (older-style) source-control system. For now, other issues are more important but we should revisit this issue later.

http://www.scintilla.org/ScintillaDoc.html#SCN_MODIFYATTEMPTRO

   Neil

[johnsonj]

"""Another approach may be to simply disallow IME character addition if any selection is protected. Or drop any selections that are protected."""

Would you please give some tips? I would try to.

"""When a modification attempt is made on a read-only document, a SCN_MODIFYATTEMPTRO notification is sent to the application which can make the document writeable if it wants."""

Thank you for the tip!
I will give it another try.

[Neil Hodgson]

johnsonj:

> Would you please give some tips? I would try to.

SelectionContainsProtected() for finding if any element of the selection is protected.

For filtering, it may be a little complex but essentially reverse iterate through the ranges and DropSelection each that is protected. However, it should behave sensibly for boundary cases such as when all selections are protected, possibly maintaining the main selection only and not allowing input.

Neil

[johnsonj]

Neil:

"""   For filtering, it may be a little complex but essentially reverse iterate through the ranges and DropSelection each that is protected. However, it should behave sensibly for boundary cases such as when all selections are protected, possibly maintaining the main selection only and not allowing input. """

I am trying to but I have no idea about filtering mechanism yet.

"""SelectionContainsProtected() for finding if any element of the selection is protected. """

Although I am not sure whether RangeContainsProtected() works or not,
I have no choice but to depend on RangeContainsProtected() to protect preedit string from external change.
Here is my try.

                positionInsert = InsertSpace(positionInsert, currentSel->caret.VirtualSpace());
                const int lengthInserted = pdoc->InsertString(positionInsert, docChar.c_str(), static_cast<int>(docChar.size()));

                if (lengthInserted > 0) {
                    currentSel->caret.SetPosition(positionInsert + lengthInserted);
                    currentSel->anchor.SetPosition(positionInsert + lengthInserted);
                    if (flags.indicator != 0) {

                        pdoc->decorations.SetCurrentIndicator(flags.indicator);
                        pdoc->DecorationFillRange(positionInsert, 1, lengthInserted);

                        // Unvisible style set to protect preedit string.
                        // style number 111 randomly chosen.

                        pdoc->StartStyling(positionInsert, 0);
                        pdoc->SetStyleFor(lengthInserted, 111);
                        vs.styles[111].changeable = false;
                    }
                }

[Neil Hodgson]

johnsonj:

> Although I am not sure whether RangeContainsProtected() works or not,

Have you tried setting a style to be protected with the notchangeable attribute in SciTE? For C++ identifiers:
style.cpp.11=notchangeable

> I have no choice but to depend on RangeContainsProtected() to protect preedit string from external change.

I don’t see why this is needed.

Neil

[johnsonj]

Neil:

"""style.cpp.11=notchangeable"""
I have tried 0 rathter than 'notchangeable'
Thank you. it works.

Caret can not go into protected regions.
so it can not be used to protect comp string,
which gives me disapointment.

Another try needs for me.
Thank you for indtructions.

[johnsonj]

removed ClearTentativeStart().
moved TentativeStart() into AddCharacter().
restructured AddCharacter() for efficiency.

[johnsonj]

just for reporting.
moved TentiveUndo() mechanism into AddCharacter().
I am not sure whether this is good or bad.

[johnsonj]

Sorry for AddCharacter-TentiveUndo.patch!
Backspace never gets to TentativeUndo control.

[Neil Hodgson]

johnsonj:

> <AddCharacter-tentativeStart.patch>

The second FilterSelections shouldn’t do anything so shouldn’t be needed.

flags.posInCharacters appears new and I can’t work out why it is needed.

> Sorry for AddCharacter-TentiveUndo.patch!
> Backspace never gets to TentativeUndo control.

OK. I’ll pretend it never happened.

Terminology note: flags is a set of booleans. When you add a non-boolean, …Flags should be renamed …Options.

Neil

[johnsonj]

Currently AddCharUTF performs a lot of redundant works.
It is enough for FillvirtualSpace to run only if the first charater.
It is enough for TentativeActive to run only if the first preedit character.

I have tried to move TentativeStart() from platforms into Editor, and to move TentativeUndo() into Editor or ScintillaEditBase.

TentativeStart() appears successfully moved into Editor
but I could not move TentativeUndo() into Editor due to Backpace (or unknown message or event).

It is preferable TentativeStart() be with TentativeUndo.

Consequently, Lots of options or flags has been removed.
Now I have one flag (isPreeditCharacter) and one option (imeIndicator) left.

I find I am back from the starting point.
I need seperator of undos.

[johnsonj]

Thank you for the tip!

I will keep in mind.