Any support planed for a Change Margin ?

[Link]

I stumbled on a modified scintilla version (Code::Blocks) that have
implemented change margin support.

I was wondering if any official support for this was planed ?

Can this be done without modifying scintilla and still have good
performance and accuracy ?

How would you the scintilla developer do this ?

[Neil Hodgson]

Link:

> I stumbled on a modified scintilla version (Code::Blocks) that have
> implemented change margin support.
>
> I was wondering if any official support for this was planed ?

The implementations I have seen are inaccurate and use excessive
storage. A good implementation would be useful.

Neil

[Jérôme LAFORGE]

Hi,
Couple months ago, I begun to work on this kind of feature. By lack of
time, I had to stop to work on it.

So, you can find the 2 drafts. There's still work.
-For example for managing, the removed line with undo redo. I want to
store the LineChange for each removed line into Action of
UndoHistory.
-For redraw margin when line has changed.
-Surely, need to accuracy for some cases :-)

I hope this draft is good and cleaned enough to serve for base for
future work on this feature.
Neil, I'm sure that you have some clue or requirements for helping
me ;)

http://pastebin.com/jLk5UPZR
http://pastebin.com/N5JD2SFa

Jérôme

[Philippe Lhoste]

On 20/10/2011 15:58, Link wrote:
> I stumbled on a modified scintilla version (Code::Blocks) that have
> implemented change margin support.

Is there a good description of this feature somewhere?
What it looks like, what is the logic behind the rendering...

--
Philippe Lhoste
-- (near) Paris -- France
-- http://Phi.Lho.free.fr
-- -- -- -- -- -- -- -- -- -- -- -- -- --

[Jérôme LAFORGE]

Please find this new version of patch :
http://pastebin.com/qGde60U1

It's cleaner, use less memory.
It's also redraw margin when change has occurred.

For description of this feature, this patch is want to implement this
feature like Qt Creator (it seems that Eclipse also implements this
feature)

Jérôme

[Neil Hodgson]

Jérôme LAFORGE:

> Please find this new version of patch :
> http://pastebin.com/qGde60U1

Looks like it doesn't restore when undoing.

Neil

[Jérôme LAFORGE]

Please find this new patch :
http://pastebin.com/svCrq6h2

In this patch, I store the information into Action of UndoHistory.
There are some unaccurencies with the managment of undo/redo.
For example :
- if you insert (via copy&paste) a block of text.
- you modify some line of this block of text.
- you save the file
- you undo until remove this block of text.
- you redo insert (all block of text is considered like dirty, that's
wrong but difficult to manage...)
- you redo until savepoint (all block of text is cleaned).

Jérôme


Neil Hodgson a écrit :

[Neil Hodgson]

Jérôme LAFORGE:

> In this patch, I store the information into Action of UndoHistory.
> There are some unaccurencies with the managment of undo/redo.
> For example :
> - if you insert (via copy&paste) a block of text.
> - you modify some line of this block of text.
> - you save the file
> - you undo until remove this block of text.
> - you redo insert (all block of text is considered like dirty, that's
> wrong but difficult to manage...)
> - you redo until savepoint (all block of text is cleaned).

I've been down similar paths and they have ended up too complex and heavy.

Expanding the size of undo steps is bad for clients that do not use
this feature so it may be better to implement this as a parallel data
structure that can be completely off.

Neil

[Jérôme LAFORGE]

>    Expanding the size of undo steps is bad for clients that do not use> this feature so it may be better to implement this as a parallel data> structure that can be completely off.

With the current patch, the size of undo step is always increased by
one pointer (4 bytes or 8 bytes according to architecture), if the
user doesn't use this feature, then this pointer can be initialise to
NULL. Do you think that expanding is too important?
Put data into undo step, seems to me more comfortable because, it
there is no synchronization with undo stack and this future parallel
data structure.

>    I've been down similar paths and they have ended up too complex and heavy.

I maybe found one solution to manage this case with acceptable design.
But the current patch use only one byte (unsigned char) per line to
manage this feature. If we want manage this kind of case, then I think
that I have to use 2 bytes (2 unsigned char) per line (1 byte to store
the status at save point and 1 byte to store current status) and
don't reset the status to "maskHasBeenSaved | noChange" when save
point is reached.

Jérôme

[Neil Hodgson]

Jérôme LAFORGE:

> With the current patch, the size of undo step is always increased by
> one pointer (4 bytes or 8 bytes according to architecture), if the
> user doesn't use this feature, then this pointer can be initialise to
> NULL. Do you think that expanding is too important?

Code tends to bloat out if you don't check all changes for size
increases and it can be a lot of work to undo this as I did earlier to
reduce per-line data for optional features. I've been meaning to
reduce the memory used for undo since it really isn't optimal and this
makes Scintilla less viable for some applications.

> Put data into undo step, seems to me more comfortable because, it
> there is no synchronization with undo stack and this future parallel
> data structure.

There wouldn't be any complex mapping. They would both be arrays
with element n in the undo stack corresponding to element n in the
change bar undo.

> I maybe found one solution to manage this case with acceptable design.
> But the current patch use only one byte (unsigned char) per line to
> manage this feature. If we want manage this kind of case, then I think
> that I have to use 2 bytes (2 unsigned char) per line (1 byte to store
> the status at save point and 1 byte to store  current status) and
> don't reset the status to "maskHasBeenSaved | noChange" when save
> point is reached.

A single byte may not have enough values to be safe: Typing a
single 80 byte line will use 80 values up.

Visual Studio uses 4 states: original, changed, saved change and
saved change that has been undone.

Neil

[Jérôme LAFORGE]

Sorry for my previsous message, there is one problem :
Please find a update of my patch.

http://pastebin.com/BCeCaY9y

I have completly refactor the code. I don't use UndoHistory anymore.
With this patch, I use 2 unsigned short int for each line to store the
data (one for current status and for status at the save point)
The previous example case with insert by block, modification, save and
undo is normally managed.
There are still some inaccurancies (I'm going to try to fix it). If
someone can try to test this patch and give me test case to help me to
reproduce it and fix it. Feel free :)

TODO :
- Fix the remaining inaccurancies.
- Drawn the change margin.
- Add a new message to enable/disable this feature eg:
SCI_CHANGEMARGIN(bool).

I hope that this new design is more suitable for you.

On 11 nov, 23:25, Jérôme LAFORGE <jerome.lafo...@gmail.com> wrote:
> Please find a update of my patch.http://pastebin.com/BCeCaY9yIhave
> completly refactor the code. I don't use UndoHistory anymore.With this
> patch, I use 2 unsigned short int for each line to store the data (one
> for current status and for status at the save point)The previous
> example case with insert by block, modification, save and undo is
> normally managed.There are still some inaccurancies (I'm going to try
> to fix it). If someone can try to test this patch and give me test
> case to help me to reproduce it and fix it. Feel free :)
> TODO :- Fix the remaining inaccurancies.- Drawn the change margin.-
> Add a new message to enable/disable this feature eg:
> SCI_CHANGEMARGIN(bool).
> Neil, this new design, is it more suitable for you?
> Jérôme

[Neil Hodgson]

While I haven't examined it very closely, it appears to be trying
to detect modifications of the change margin in many pieces of
document mutation code inside Editor.cxx. Its unlikely you will find
all the places where the document changes at this level. It is also
unlikely to work when there is more than one view. This could behave
more like fold structure changes with a CM change bit in the
modification notifications which are distributed to all views.

Neil

[Jérôme LAFORGE]

Please find another version :
http://pastebin.com/xGMj0PSu

With this new version of patch :
I unfreeze ChangeUndo when reach save point.
The test case below work with the unfreeze :
1 - if you insert (via copy&paste) a block of text.
2 - you modify some line of this block of text.
3 - you save the file
4 - you undo until remove this block of text.
5 - you redo insert
6 - you redo until savepoint.
7 - again you modify some line of this block of text.
8 - you save the file (like step 3)
9 - you undo until remove this block of text (like step 4)
10 - you redo until savepoint (like step 6)
11 - redo from step 7 to step 10

I modify how I change margin. the modification performs only if
startSequence=true. It's more accurate than mechanism of my previous
patch.

I fix some inaccuracies during the document modification, undo and
redo.

Identify remaining inaccurancies :
- Insert at several lines (rectangular selection).
- Insert CR just after espace char. For ex : "insert end line after
this point. the_previous_space is deleted."

I also notice that sometime scite crashes due to segfault. I try to
fix that...

> This could behave more like fold structure changes
> with a CM change bit in the modification notifications
> which are distributed to all views.

Ok, thx for this clue. I going to release this notification into my
next patch.

Jérôme

[Jérôme LAFORGE]

Please find this new patch : http://pastebin.com/LjBdZimH

- Where I use document notification for redraw select margin.

- I fix somes inaccuracies (it's pretty tricky to fix those
inaccuracies without regression...).



Jérôme

[Neil Hodgson]

Jérôme LAFORGE:

> Please find this new patch : http://pastebin.com/LjBdZimH
>
> - Where I use document notification for redraw select margin.

I can't see any notification code in Document.cxx there. You could
use the approach from my earlier version in
http://scintilla.sourceforge.net/scitecb.zip

> - I fix somes inaccuracies (it's pretty tricky to fix those
> inaccuracies without regression...).

I'm not sure this approach is strong enough to cover all the cases.
The change margin does not have to be completely accurate but there
should be a solid story to tell like: "A line is coloured if and only
if it has been changed. The colour may be wrong in these circumstances
in this way". Otherwise its going to be difficult answering bug
reports.

There is no need for changing copyright years or including change
logs inside code files. I'm going to stop accepting changes to the
year ranges as they are just noise. Change logs only work for files
that see few changes and become an incomplete mess for other files
unless there are strong rules about when and what to add. Changes are
much better documented inside source code control as that also
includes the set of files involved in a change.

Neil

[Neil Hodgson]

Me:

>   I can't see any notification code in Document.cxx there. You could
> use the approach from my earlier version in
> http://scintilla.sourceforge.net/scitecb.zip

As a patch:
http://scintilla.org/changebar.patch

Neil