Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Change bar feature in scintilla

<< < (4/7) > >>

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:

--- Quote from: gryphon on November 18, 2008, 03:12:26 am ---@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

--- End quote ---

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).
--- End code ---
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:

--- Quote from: jens on November 17, 2008, 07:23:42 pm ---try http://apt.jenslody.de/patches/scintilla_changebar_new.patch, that should work.

--- End quote ---
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);

--- End code ---
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:

--- Quote from: MortenMacFly on November 18, 2008, 09:43:49 am ---Was there a special reason *not* to apply this section?

--- End quote ---

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


--- Quote from: MortenMacFly on November 18, 2008, 09:43:49 am ---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?!

--- End quote ---

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:

--- Quote from: jens on November 18, 2008, 12:30:42 pm ---It's a good idea, but should be changed for linenumber-, and error/bookmark/debugmark-margin too.

--- End quote ---
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?!

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version