Change bar feature in scintilla

[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...?!