Change bar feature in scintilla

[gryphon]

I stumbled across this link (after a LOT of searching). It is a patched version of scintilla that provides a changebar feature in the margin like in Visual Studio.
http://groups.google.com/group/scintilla-interest/browse_thread/thread/2e5e1b82350886cc/adea77152522a434?lnk=gst&q=change#adea77152522a434

- maybe it's old news however I think it would be nice to try to build against this patched version to see the effect. It could be a very nice addition to CodeBlocks. I had planned to post this after I'd tried that but I haven't found the time yet. I know a couple of you have been playing with building against the latest scintilla code so I thought at least some of you may be interested in trying this.

gryphon

[gryphon]

I finally got a chance to look at this again tonight. I've been able to build codeblocks using this modified scintilla source and the change bars work quite well (from my limited testing). I'm trying to do a bit more testing and collate a patch of the necessary changes to to enable this (assuming it is stable enough for use). In the meantime however I've attached two images showing the change bars in action. A yellow bar for unsaved changes and a green bar for saved changes.

gryphon

[attachment deleted by admin]

[Biplab]

Looks cool. Can you post a patch to Berlios?

[Ceniza]

That's a pretty nice feature of other IDEs, and it's lovely you're working on implementing it for Code::Blocks. I have to use Visual C++ at the office, and, everytime I see that bar, I wish Code::Blocks had it too.

Something I've noticed in Visual C++ is that if I try to compile something that produces compiling errors, I can modify the code, and when I double click on the error, it takes into account modified lines to take me to the right new line in the source where the error really happens. In other words, if I have an error in line 1234, but I delete 10 lines before that one, when I double click on the error in the errors window, which still points to 1234, it'll actually take me to line 1224. However, that's something that would have to be implemented in a different place, not in this plugin.

Have you considered telling Scintilla guys about it? It'd be nice to have that patch applied upstream too.

[killerbot]

this is great. Bring the patch on :-)

[Jenna]

I just created a patch against the new-scintilla-branch.

I think patching the (older) scintilla version in trunk is much more difficult, because the patch gryphon has found is against scintilla 1.7.5 and we use 1.7.7 in scintilla branch, but 1.6.x in trunk.

I uploaded it to my server ( http://apt.jenslody.de/patches/scintilla_changebar.patch ).
I'm not sure if I should commit it to the scintilla-branch, because I did (could) not test it much.

At the moment it can not be turned on or off, and the colours are not configurable.

It's tested under linux (debian 64bit) and windows.

Unfortunately the patch changes the colour of the second margin.
I don't know why, because I did not go deeper into the code, but I hope I will find the time to do so (maybe this evening/night).

[gryphon]

Ok, I've uploaded a patch here:
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=2596&group_id=5358

Hopefully the patch will work for everyone.

A couple of points about the patch:

• The scintilla changebar patch itself was not written by me, but by Neil Hodgson as you can see from the first post in this thread.
• The changebar markers are placed in their own margin to prevent overlapping of markers.
• Currently the changebar is placed as the last margin, after the folding margin. This is a little different than how it appears in C++Builder and Visual C++. I did it this way as it also solved another problem I commonly have - I often want to select a line of text by draggingat the left of the line. With the folding margin there I often click inside it and fold the text rather than selecting it. With the changebar margin I now have a 4 pixel wide strip I can select lines from. Of course if folding was only activated when clicking on a '+' or '-' this would not be so much of an issue. I haven't investigated if that behaviour can be achieved for not.
• Making the margin orders configurable would not be a problem and currently in the patch the ordering of the changebar margin and the folding margin is controlled by two constants making swapping their order at compile time easy.
• The comments associated with the changebar patch indicated that the code is a memory hog and it is unlikely to be integrated into scintilla anytime soon. As such I think it would be sensible to further enhance the changebar patch to make it optional. I'd hope that would make it more likely for this to make it into scintilla. Once optional there would be opportunities to improve the changebar feature as part of scintilla itself. Separating the changebar code out further would also allow us to patch scintilla as appropriate with the changebar code even if it was not going to become part of scintilla. Or at least until it was efficient enough to become so.

gryphon

[gryphon]

@Jens: Thanks for trying this! (my previous post crossed your last one as I was busy doing other things in-between writing it)

Yes I had thought about the differing versions of scintilla. My intention was to provide a patch against 1.7.7 of scintilla after I had cleaned it up. Please have a look at the patch I added to berlios to see what you think. It might be a straightforward merge.

Of course my patch in berlios is against trunk which means most of the changebar patch is actually a patch to 1.7.5 from the scintilla trunk version. With my limited testing I have not encountered any big issues but I am sure those more familiar with scintilla will be able to identify problems. Otherwise my patch against trunk seems ok.

About the margin colours (and other related issues you'll find): Have a look at how I patched the cbeditor.cpp and other codeblocks files (I think that was the main one). I think enabling the feature in codeblocks works ok and I have not had any issues so far.

Configuring, turning on and off and colour customisation, are definitely worth exploring. I didn't add that to the patch I put up to keep it smaller, I'd say though we'd really want to have that (see my comments in the previous post also).

Finally (and OT) - something I've been meaning to do for a long time: Thank you very much for you debian repository, it's been incredibly useful for me ever since you first started it 

gryphon

[Jenna]

Finally (and OT) - something I've been meaning to do for a long time: Thank you very much for you debian repository, it's been incredibly useful for me ever since you first started it 

Many thanks 

You did not change the value of wxSCI_MASK_FOLDERS to the changed value of SC_MASK_FOLDERS, otherwise you would have two changebars, one in its own margin and one inside the folder-margin.
I have to subtract the changebar-masks from wxSCI_MASK_FOLDERS for the folding mask.

But I guess the own margin is better. And it's a good idea to place it at last, because I sometimes have the same problem then you (folding instead of marking text).
If the colurs are configurable, there should be no problems with the standard-colour for "Highlight line under caret" (lightyellow).

I think the order of the margins can be hardcoded.

If it would be possible to switch the changebar off and on at runtime and to clear the history of the actual editor from the context-menu it would be really great.
I think that it should be possible, but needs some investigation, to do it "the right way".

[gryphon]

You did not change the value of wxSCI_MASK_FOLDERS to the changed value of SC_MASK_FOLDERS, otherwise you would have two changebars, one in its own margin and one inside the folder-margin.
I have to subtract the changebar-masks from wxSCI_MASK_FOLDERS for the folding mask.

Good catch! I missed that, though I had to try this to see what you mean. What is the preferred approach?

1. Leave wxSCI_MASK_FOLDERS as 0xFE000000 and no need to negate the change-bar masks (as the current patch does), or,
2. Update wxSCI_MASK_FOLDERS to the new value of 0xFF800000 and then negate the change-bar masks when setting the folding masks? I guess the original scintilla patch thought mimicking VS by default was the best approach.

My feeling is we should use the value of SC_MASK_FOLDERS from 1.7.7 (whatever that is) and perhaps add a SC_MASK_CHANGEBAR which ORs the two changebar masks together.

If you think I need to make a change to the patch regarding this let me know.

But I guess the own margin is better. And it's a good idea to place it at last, because I sometimes have the same problem then you (folding instead of marking text).
If the colurs are configurable, there should be no problems with the standard-colour for "Highlight line under caret" (lightyellow).

Glad I'm not the only one who hits that problem Making the colours configurable should be straightforward. If you don't beat me to it I'll have a go at adding that.

If it would be possible to switch the changebar off and on at runtime and to clear the history of the actual editor from the context-menu it would be really great.
I think that it should be possible, but needs some investigation, to do it "the right way".

Yeah, I think that's true. I've only begun digging deeper into the scintilla code today so haven't really got a feel yet of how to do that. I guess I should start by diff'ing against 1.7.7 to get a feel for how invasive the changebar patch actually is.

[Jenna]

Yeah, I think that's true. I've only begun digging deeper into the scintilla code today so haven't really got a feel yet of how to do that. I guess I should start by diff'ing against 1.7.7 to get a feel for how invasive the changebar patch actually is.

You can either do that or download the patch from my server, it's a patch against 1.7.7 (used for C::B's new-scintilla-branch).
Your patch is against trunk and the scintilla used there is an 1.6.x-version as far as I know, so there are many, many differences, for example to the lexers, that should not be needed for the change bar.

You can download C::B with scintilla 1.7.7, which is not (yet) in trunk with svn from 

Code

svn://svn.berlios.de/codeblocks/branches/scintilla

it should work on linux and windows and will soon (?) be merged with trunk. The scintilla-branch is uptodate, and contains some smal improvements, due to newer (better) indicatiors (used for "Highlight occurrences" and "Incremental Search").

One cause I worked with the newer scintilla is because it will most likely be committed to trunk the next time (no exact timeline, but no problems so far).

[MortenMacFly]

I'm not sure if I should commit it to the scintilla-branch, because I did (could) not test it much.

Please don't atm. We can do it in another branch later. :-)

[mariocup]

But I guess the own margin is better. And it's a good idea to place it at last, because I sometimes have the same problem then you (folding instead of marking text).

I know that problem folding instead of marking text. Therefore I use the following workaround: If you enable the display of line numbers you can select lines by clicking with the mouse button in the left margin next to the displayed line numbers and then it won't happen that you select a folding point.

[MortenMacFly]

I uploaded it to my server ( http://apt.jenslody.de/patches/scintilla_changebar.patch ).

This does not work for me at all. Either "patch" crashes "unexpectedly" or it fails patching ~80% of the hunks.
Can you try to create the patch again, please?!

[Jenna]

I uploaded it to my server ( http://apt.jenslody.de/patches/scintilla_changebar.patch ).

This does not work for me at all. Either "patch" crashes "unexpectedly" or it fails patching ~80% of the hunks.
Can you try to create the patch again, please?!

Hi Martin,

try http://apt.jenslody.de/patches/scintilla_changebar_new.patch, that should work.
The other patch only applies if you ignore changes to whitespace. I don't know why. It's created with "svn diff", and the newer one with the diff-tool from commandline.

The new patch is slightly modified compared to the first one (using an own margin for the changebar).

[gryphon]

@Jens: I uploaded a patch against the scintilla branch here:
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=2597&group_id=5358

I am not putting this forward as a patch similar to yours - I only uploaded it so you could see a change I made to scintilla that allows any margin to have the folding background. I noticed when I used the newer version of scintilla with the changebar in a separate margin it used the normal margin colour. I guess 1.7.5 had a bug in it which meant the margin colour matched the folding margin even when it shouldn't have.

I wanted it to look the same as the folding margin so made this patch quickly to try out the idea. I thought you may find that aspect of the patch useful and worth incorporating into your own patch.

gryphon

[Jenna]

@Jens: I uploaded a patch against the scintilla branch here:
https://developer.berlios.de/patch/index.php?func=detailpatch&patch_id=2597&group_id=5358

I am not putting this forward as a patch similar to yours - I only uploaded it so you could see a change I made to scintilla that allows any margin to have the folding background. I noticed when I used the newer version of scintilla with the changebar in a separate margin it used the normal margin colour. I guess 1.7.5 had a bug in it which meant the margin colour matched the folding margin even when it shouldn't have.

I wanted it to look the same as the folding margin so made this patch quickly to try out the idea. I thought you may find that aspect of the patch useful and worth incorporating into your own patch.

gryphon

If I see it right (after a short look over your patch without replying it), the main difference to my patch is, apart from making it easy to swap the folding- and changebar-margin at compile-time, that you set the margins background explicitely and created a function and an event for this purpose.
This makes the background-chosing more flexible, but it is not needed if we use scintillas standard-behaviour.

You need to do that, because you did not change "SC_MASK_FOLDERS" and "wxSCI_MASK_FOLDERS" to "0xFF800000". The unpatched scintilla uses this mask to decide which background colour is chosen:

Code

If (mask & wxSCI_MASK_FOLDERS)==0, the margin background colour is controlled by style 33 (wxSCI_STYLE_LINENUMBER). 

Quote from the scintilla api-reference (you find it in "src/sdk/wxscintilla/website/reference.html" inside the C::B source-tree).

That's the cause, why I have to explicitely change the mask for the folder-margin, to make it not show the changebar.

The main problem with the whole changebar-patch, is that it seems not to be easy to switch it on and off (not only to make the changebar-visible or not).

[MortenMacFly]

try http://apt.jenslody.de/patches/scintilla_changebar_new.patch, that should work.

It does. Nice! :-)
BTW: I believe the patch of gryphon concerning cbeditor.cpp is slightly better in certain areas. In addition I think you missed one point in cbeditor.cpp concerning calculation of the margin, namely this part of gryphons patch:

Code

@@ -2269,7 +2286,8 @@
         wxPoint clientpos(ScreenToClient(position));
         const int margin = m_pControl->GetMarginWidth(0) + // numbers, if present
                            m_pControl->GetMarginWidth(1) + // breakpoints, bookmarks... if present
-                           m_pControl->GetMarginWidth(2);  // folding, if present
+                           m_pControl->GetMarginWidth(changebarMargin) +
+                           m_pControl->GetMarginWidth(foldingMargin);  // folding, if present
         wxRect r = m_pControl->GetRect();
 
         bool inside1 = r.Contains(clientpos);

Was there a special reason *not* to apply this section?

What I like especially is that there are static constants named "changebarMargin" and "foldingMargin" which makes reading the code a lot easier to understand. What do you think?!

[Jenna]

Was there a special reason *not* to apply this section?

No reason, I had it on my working copy, but it got lost, while merging, don't know why, maybe too late.

What I like especially is that there are static constants named "changebarMargin" and "foldingMargin" which makes reading the code a lot easier to understand. What do you think?!

It's a good idea, but should be changed for linenumber-, and error/bookmark/debugmark-margin too.
Might make sense in other parts of the sourcecode too. Even if it it is not a must, but more or less a matter of taste (or coding style).

[MortenMacFly]

It's a good idea, but should be changed for linenumber-, and error/bookmark/debugmark-margin too.

I have done that already (for all!)... including the adjustment you missed. So hopefully it does not get lost.

My proposal of what to do next:
1.) Merge scintilla into trunk (it should be safe now, but we need to talk to Yiannis before)
2.) Apply marker changes to scintilla branch (so we re-use that one... ;-)) -> I can do it, I should have all we need.
3.) Do dome more testing and probably apply it after some more feedback of the other devs.

I personally like the feature quite much. But I am also aware that we break scintilla compatibility a little. Hence it gives more than it takes IMHO... what about the others?!

[Jenna]

It's a good idea, but should be changed for linenumber-, and error/bookmark/debugmark-margin too.

I have done that already (for all!)... including the adjustment you missed. So hopefully it does not get lost.

My proposal of what to do next:
1.) Merge scintilla into trunk (it should be safe now, but we need to talk to Yiannis before)
2.) Apply marker changes to scintilla branch (so we re-use that one... ;-)) -> I can do it, I should have all we need.
3.) Do dome more testing and probably apply it after some more feedback of the other devs.

I personally like the feature quite much. But I am also aware that we break scintilla compatibility a little. Hence it gives more than it takes IMHO... what about the others?!

I also think, that we can merge (or better you can, I have to work outside the today and tomorrow, to tune some parameters of a control, and have no access to my linux at the moment, because I have to use windows for programming  :cry: ).

About the changebars: as written before it would be nice if the user can switch them off and to clear the history. It should not be to difficult I hope. Maybe I find some time this evening at the hotel.

[mariocup]

Hi Martin,

I will try the patch in the end of the week under windows and linux and will give you some feedback. I think these is a really nice feature and if it is stable enough there should be the interest of the scintilla team to integrate it. Are you in contact with some scintilla devs and are they aware of this patch?

Bye,

Mario

[MortenMacFly]

Are you in contact with some scintilla devs and are they aware of this patch?

I don't know. But read further in the thread. I think gryphon (the author) said something they won't apply it... better you ask him.

[Jenna]

I just updated my patch and improved it a little bit:

• the user can select from the editor configuration dialog, whether to use changebar or not
• the history of the active editor can be deleted via main- or context-menu

If the changebar will be switched off globally, the undo-history will not be touched, but if it will be switched on, the undo-history of all editors will be deleted, because we otherwise get inconsistencies between both collections.
Because of the problem with the inconsistent collections, I decided to delete the whole history at once (undo- and changebar).

The patch can be downloaded from http://apt.jenslody.de/patches/scintilla_changebar_20081120-2.patch .

It's created against the scintilla-branch r5318.

I did not (yet) test it under windows.

[mariocup]

Hi Jens,

I did not try the changebar feature, therefore I have some questions:
1. Where is the changebar history stored in CB? Will this history be cleared when I close the file?
2. Is every changebar saved in a local history?
3. What would be cool to have same navigation feature like in a diff program (e.g. kdiff3) to jump to the next and previous difference/changebar.

[Jenna]

Hi Jens,

I did not try the changebar feature, therefore I have some questions:
1. Where is the changebar history stored in CB? Will this history be cleared when I close the file?
2. Is every changebar saved in a local history?
3. What would be cool to have same navigation feature like in a diff program (e.g. kdiff3) to jump to the next and previous difference/changebar.

1. it's stored inside the active editor, and will therefore be cleared if editor is closed,
2. yes
3. you mean a shortcut to find the next changed line, without stepping through the history ? I think, this should be possible.

To make it clear, I am not the author of the changebar-feature (http://groups.google.com/group/scintilla-interest/browse_thread/thread/2e5e1b82350886cc/adea77152522a434?lnk=gst&q=change#adea77152522a434), I just merged it with scintilla 1.7.5 and modified it, to make it optional, to switch it on and off and to delete history.  (and of course changed C::B to use this)

[MortenMacFly]

I did not (yet) test it under windows.

I'll do it.
BTW: I believe you still missed that part (unless you have a reason not to apply it):

Code

        const int margin = m_pControl->GetMarginWidth(lineMargin) +     // numbers, if present
                           m_pControl->GetMarginWidth(markerMargin) +   // breakpoints, bookmarks... if present
                           m_pControl->GetMarginWidth(foldingMargin) +  // folding, if present
                           m_pControl->GetMarginWidth(changebarMargin); // changebar

[mmkider]

Hi Jens,

I did not try the changebar feature, therefore I have some questions:
1. Where is the changebar history stored in CB? Will this history be cleared when I close the file?
2. Is every changebar saved in a local history?
3. What would be cool to have same navigation feature like in a diff program (e.g. kdiff3) to jump to the next and previous difference/changebar.

Hi, changebar  is  exciting.
If you want store traces, you can reference Bookmark plugin.
Because Bookmark is stored inside the active editor,too.
I implement record historys for Bookmarks.
Next, I will implement Navigate Backward and forward.

http://forums.codeblocks.org/index.php/topic,9531.0.html

[Jenna]

I did not (yet) test it under windows.

I'll do it.
BTW: I believe you still missed that part (unless you have a reason not to apply it):

Code

        const int margin = m_pControl->GetMarginWidth(lineMargin) +     // numbers, if present
                           m_pControl->GetMarginWidth(markerMargin) +   // breakpoints, bookmarks... if present
                           m_pControl->GetMarginWidth(foldingMargin) +  // folding, if present
                           m_pControl->GetMarginWidth(changebarMargin); // changebar

Shame on me ! :oops:

Too less sleep, too old, whatever.

Here's a patched patch: https://apt.jenslody.de/patches/scintilla_changebar_20081120-3.patch .

[MortenMacFly]

I'll do it.

Looks good, besides on objection:
We have under editor settings the category "margins and caret".
I believe because the changebar *is* a margin the settings option should go there...?!

[MortenMacFly]

https://apt.jenslody.de/patches/scintilla_changebar_20081120-3.patch .

I feel #4 is probably coming soon, too... ;-) We wrote literally the same second.

[Jenna]

I'll do it.

Looks good, besides on objection:
We have under editor settings the category "margins and caret".
I believe because the changebar *is* a margin the settings option should go there...?!

You are right.

Here's a patched patched patch: https://apt.jenslody.de/patches/scintilla_changebar_20081120-4.patch.

EDIT: Seems not to work correctly, if checkbox switched (I guess the id got lost while copying), will fix later.
Family needs some time now.

[Jenna]

And another one (https://apt.jenslody.de/patches/scintilla_changebar_20081120-5.patch).

This works as expected (as far as I see).

Checkbox is in "Margins and caret", and the margin disapeears, if changebar is disabled.

I swapped changebar- and folding-margin, because after using it a while, I think it looks better this way.