Hi,
as promised, here is a patch to scintilla which makes it send new
notifications SC_START_ACTION each time that a new undo/redo transaction
(which is the result of user action) begins.
Why is SC_START_ACTION a new notficiation rather than a bit flag on
existing notifications such as the SC_MOD_DELETETEXT |
SC_PERFORMED_USER notification in DeleteChars? Why do you get the
notification when calling BeginUndoAction? I would expect that to be
harder to deal with as you would have to count it with the
notification for the actual change.
the SC_START_ACTION was not put as a bit inside other notification... hum
because of nothing ;-) they could for insert and delete, as long as we would
state in doc that the transaction was started before the given action.
I need to send the notification from BeginUndoAction because first it is an
homogenous behaviour (all transaction starts get their notification), second
because Scintilla opens its own transactions (for example when calling
SetEol or such).
Passing around a reference to implicitBegin is ugly and makes a
reader wonder if there are more side effects: returning a value when
possible avoids needing to wonder about side effects and aliasing.
hum, the reason for an argument return rather than simple return is a bit
theoretical: a return value must be 'natural' else the spec will surely
evolve. I mean that calling DeleteChars clearly has nothing to do with
opening undo/redo transaction. My rule is not 'return because I can' but
'return because I should'. I used to do the first in my early coding years
and it never brings good things. However, I need the information from
outside so I give it back that's all. On the other hand, returning a bool in
BeginUndoAction is somewhat natural, if 'true' the action had an effect, if
'false' it did not.
For sure, the 'implicitBegin' should be documented as 'out only' as well.
Can't you predict that an action will cause a begin by testing whether
the depth==0 and the current action is a startAction? Recovering the
information after performing a change is more complex and can't be
extracted into a separate "WillChangeStartUndoStep" method.
As a rule, I never try to predict anything if simply logging what is done is
enough, prediction is double coding, hence double bugging (unless I use same
function at both places to decide to act or not). It creates unexpected
dependencies, whereas here the dependency is explicit: someone coming after
me knows that those variables needs to be initialized, in this way and at
this moment. So, if we could add a function predicting if a new transaction
should be open, it's OK (however its execution would repeated).
I'll remove the redundant notifications in InsertString/DeleteChars and send
the patch.
Armel
_______________________________________________
Scintilla-interest mailing list
[email protected]
http://mailman.lyra.org/mailman/listinfo/scintilla-interest
