new folding flag

[Stefan Küng]

The current folding flags only allow to draw lines above/below of contracted (or expanded) lines.
But I would like a new style that's changing the contracted line itself, something like this:

        if ((!expanded && (model.foldFlags & SC_FOLDFLAG_RECTANGLE_OVER_CONTRACTED))) {
            PRectangle rcFoldLine = rcLine;
            surface->AlphaRectangle(rcFoldLine, 1, vsDraw.styles[STYLE_DEFAULT].fore, 50, vsDraw.styles[STYLE_DEFAULT].fore, 255, 0);
        }

I've used fixed alpha values for this example, maybe that could be made configurable with a new style? And that new style could then also be used for the current folding lines - now they use the STYLE_DEFAULT foreground color, which is also something I would like to have configurable separately.

Of course, this could maybe be extended so that the line text has "..." appended, indicating that the line is folded.

And while we're at it: would it be possible to have all folded lines deleted if I click on a folded line margin and hit the Del key? Now it only deletes the first line of the fold, not all of them. Unless I Shift-Right extend the selection, then it deletes the hole fold.

[Neil Hodgson]

Stefan Küng:

> But I would like a new style that's changing the contracted line itself, something like this:
>
> if ((!expanded && (model.foldFlags & SC_FOLDFLAG_RECTANGLE_OVER_CONTRACTED))) {
> PRectangle rcFoldLine = rcLine;
> surface->AlphaRectangle(rcFoldLine, 1, vsDraw.styles[STYLE_DEFAULT].fore, 50, vsDraw.styles[STYLE_DEFAULT].fore, 255, 0);
> }

This looks like: http://www.scintilla.org/BlockHeader.png

> Of course, this could maybe be extended so that the line text has "..." appended, indicating that the line is folded.

The […] blobs that some editors use are more complex: they may indicate the type of the fold [ /* … */ ] or [<table>…</table>] and may support navigation and selection of the block. A good implementation here would be useful but I don’t think a minimal one is worth including.

Neil

[Stefan Küng]

On Sunday, September 11, 2016 at 12:23:03 PM UTC+2, Neil Hodgson wrote:

Stefan Küng:

> But I would like a new style that's changing the contracted line itself, something like this:
>
>         if ((!expanded && (model.foldFlags & SC_FOLDFLAG_RECTANGLE_OVER_CONTRACTED))) {
>             PRectangle rcFoldLine = rcLine;
>             surface->AlphaRectangle(rcFoldLine, 1, vsDraw.styles[STYLE_DEFAULT].fore, 50, vsDraw.styles[STYLE_DEFAULT].fore, 255, 0);
>         }

   This looks like: http://www.scintilla.org/BlockHeader.png

Can you tell me how you did that?
I can't see an SC_FOLDFLAG_ for this, or a margin style that could do that?
 

> Of course, this could maybe be extended so that the line text has "..." appended, indicating that the line is folded.

   The […] blobs that some editors use are more complex: they may indicate the type of the fold [ /* … */ ] or [<table>…</table>] and may support navigation and selection of the block. A good implementation here would be useful but I don’t think a minimal one is worth including.

I guess if you want this to be complex, you'd have to get the lexers to provide that info.
Not sure if that's feasible though.

However, just showing "[...]" on the folder header would be good enough for me: it shows that there's something hidden more clearly than just a line or even the block header. It would be a small but useful indicator that this is a fold.

Stefan

[Neil Hodgson]

Stefan Küng:

> Can you tell me how you did that?
> I can't see an SC_FOLDFLAG_ for this, or a margin style that could do that?

I pasted your code in and removed the SC_FOLDFLAG_ test. People generally can’t tell what the results of drawing code will look like so its a good idea to show an example or mock-up.

> I guess if you want this to be complex, you'd have to get the lexers to provide that info.
> Not sure if that's feasible though.

Fold visuals similar to Visual Studio or other applications is the most common request.

At least initially, it could be the application’s role to determine what the text is with a default of “…”.

Neil

[Kit Yam Tse]

However, just showing "[...]" on the folder header would be good enough for me: it shows that there's something hidden more clearly than just a line or even the block header. It would be a small but useful indicator that this is a fold.

Agree, having a smaller marker at the end of the fold starting line is useful, the marker don't have to support any fancy navigation or selection, only a marker here would be good enough to me.

Kit 

[Wonson]

Neil Hodgson於 2016年9月11日星期日 UTC+8下午6時23分03秒寫道：

If you want it flexible enough to judge the blob, i have some ideas. i think there are two ways to achieve it:
- one is to use lexer to determine the folding type, that require a coordination and rewrite of lexers
- one is to SCI_SETFOLDINGHANDLER with IFolderHandler to let outside handle it

class IFolderHandler {
char *returnMeABlob(char* firstLine, char *textToBeFolded);
}

[Wonson]

Wonson於 2016年9月15日星期四 UTC+8下午12時52分51秒寫道：

or use range, so that outside can decide use text to judge or use styleAt to do it

class IFolderHandler { 
    char *returnMeABlob(Sci_TextRange firstLine, Sci_TextRange textToBeFolded); 
}

[Neil Hodgson]

Wonson:

> - one is to SCI_SETFOLDINGHANDLER with IFolderHandler to let outside handle it

The standard old way of performing folding in Scintilla is for the application to handle the margin click by calling SCI_TOGGLEFOLD(line) or SCI_SETFOLDEXPANDED(line, 0). The application could provide a string at this point so SCI_TOGGLEFOLDSHOWTEXT(line, "<table>…</table>”).

Neil

[Wonson]

Sounds good. Will this be available in next release?

Neil Hodgson於 2016年9月16日星期五 UTC+8下午6時28分36秒寫道：

[Neil Hodgson]

Wonson:

> Sounds good. Will this be available in next release?

No one is working on this.

Neil

[Wonson]

I would like to contribute this part. I want to discuss something further.

where and how (which class/which format (WordList etc.) ) would you like to save this "BLOB" for a line?

Neil Hodgson於 2016年9月19日星期一 UTC+8下午12時16分05秒寫道：

[Neil Hodgson]

Wonson:

> I would like to contribute this part. I want to discuss something further.
>
> where and how (which class/which format (WordList etc.) ) would you like to save this "BLOB" for a line?

A similar data structure could be LineAnnotation in PerLine.h which stores a blob for each line. LineAnnotation has the disadvantage of allocating an element for every line.

Contraction tags are likely to be very sparse as they are only really needed for visible contractions and there will be many empty elements with LineAnnotation. Thus it would be a good candidate for something similar to RunStyles which behaves like run-length encoding but there is no example of RunStyles with pointers. Take a look at modifying RunStyles with SplitVector<char *> (or maybe SplitVector<std::string> if it works) in the place of SplitVector<int> styles.

I’ll accept either something that allocates an element per line like LineAnnotation or is sparser like RunStyles but would expect that it would eventually evolve towards RunStyles if contraction tags are widely or intensively used. Something like a std::map<line, string> may work OK for small files but its likely to cause performance problems with larger files since it has to renumber elements when lines added/removed and to copy elements when contracting/expanding folds.

PerLine is Document state and contraction tags are View (Editor) state and there is no close analogue on Editor. ContractionState is on view, is a bit similar, and is associated with folding so this may be a reasonable home for it.

There should be minimal storage and processing costs unless the application decides to use this feature.

Neil

[Wonson]

How about modify RunStyle to SparseVector<int> which able to accept other type of sparse content (in this case, char * or std::string)?

Neil Hodgson於 2016年9月27日星期二 UTC+8下午3時43分55秒寫道：

[Wonson]

Confirmed std::string is not compatible with SplitVector as a back store in SparseVector, so I use <char *> for now.

I noticed that SCI_MARGINSETTEXT and SCI_ANNOTATIONSETTEXT directly takes the char * from outside and makes no copy. Should I follow that practice?

So should we assume that char* passed in is allocated to the heap and would not be deallocated? 

Wonson於 2016年9月27日星期二 UTC+8下午5時17分12秒寫道：

[Neil Hodgson]

Wonson:

> I noticed that SCI_MARGINSETTEXT and SCI_ANNOTATIONSETTEXT directly takes the char * from outside and makes no copy. Should I follow that practice?

Copies are made in AllocateAnnotation.

> How about modify RunStyle to SparseVector<int> which able to accept other type of sparse content (in this case, char * or std::string)?

RunStyle would normally use 1 element for a present value (string here) and then (commonly) another for the NULLs up to the next present value. It may be possible to tweak slightly so that only present values are stored and NULLs assumed between, potentially halving the number of elements although not the text storage part.

Neil

[Neil Hodgson]

Wonson:

> Confirmed std::string is not compatible with SplitVector as a back store in SparseVector, so I use <char *> for now.

The problem with std::string in SplitVector appears to be the use of memcpy/memmove. Here is an updated version that uses std::copy which works OK in the unit tests and also when used in SciTE. Its also just as fast as memmove with both MSVC 2015 and MinGW 4.9.3.

While it would be more efficient if it used std::move instead of std::copy in some cases like GapTo, its difficult to determine if std::move is available.

Neil

[Wonson]

Nice. Split vector works well. But for my SparseVector<?> (RunStyles with template in fact), some non-exist value returns 0 / NULL at the moment, so I will stick with SparseVector<char *> for now to achieve the folding feature first.

Any "What you see is not the exact text" reference code? 

What's in my mind now is to make a 

DrawFoldDisplayText

 and then call it in

void EditView::DrawLine(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,

       int line, int lineVisible, int xStart, PRectangle rcLine, int subLine, DrawPhase phase)

?

and maybe make one more preset style type for it.

is that correct?

Neil Hodgson於 2016年9月29日星期四 UTC+8下午3時22分36秒寫道：

[Wonson]

One more thing. Should folding display text have higher priority then annotation? or draw them both?

Wonson於 2016年9月29日星期四 UTC+8下午6時19分42秒寫道：

[Neil Hodgson]

Wonson:

> What's in my mind now is to make a

> DrawFoldDisplayText
> and then call it in
> void EditView::DrawLine(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,
>
> int line, int lineVisible, int xStart, PRectangle rcLine, int subLine, DrawPhase phase)

It might be a separate phase if it could go over/under other elements although that will become clearer as its implemented.

> ?
> and maybe make one more preset style type for it.

Or use the style of the previous character or of the first character that is hidden by folding to match nicely.

> One more thing. Should folding display text have higher priority then annotation? or draw them both?

I’m not really seeing the conflict since annotations go on extra lines so both can be drawn. If the question is whether the annotation for a line being folded should be suppressed then I think there are cases where that isn’t wanted because the annotation is an explanation for the construct that is still useful when folded.

Neil

[Wonson]

Hi Neil,

I've finished basic part of "show fold display text" task. And add a bit modification to original folding, see if you ok with it.

Modification:

• Selection caret changed when folding
  

//Case 1:
//Before folding: (caret inside folding text / selection part of folding text)
....
{
     .... |
}

//After folding: (caret becomes in front of folding parent)
....
|{

//Case 2:
//Before folding: (selection includes the whole folding text)
....
{
     ....
}
...
 
//After folding: (selection still includes the whole folding part)
....
{
...

Other carets outside the folding text is unaffected. Enable this feature with fold flag SC_FOLDFLAG_RESETCARETTOFOLDSTART

• Expand folding part when modifying folded parent line (Not yet really implemented)

Added a function named

SelectionRange Editor::FoldExpandIfModified(SelectionRange range)

which may expand the folding part and returns a new insert position

But to achieve the expected behaviour i said, we need to call this function everywhere modifying the pdoc. This is the part I haven't done.

The reason I made the feature is because of 2 points:

1. I notice that modifying the fold parent line might make the block losing the folding state, e.g.

/** => /SomeTextHere**

Though you remove "SomeTextHere", folding state can be back, so I think the document still holds the folded text. But in UI, when it lost the folding state, the lines just like somehow disappeared, which is strange.

2. Also, when this TOGGLEFOLDTEXT feature applied. The text which occupied the whole line are supposed as one block and always occupying the fold parent line. 

case like

Some[<FoldBlob>...</FoldBlob>]Text // fold parent line

would be very weird.

So I think when it is reasonable to expand text when modifying the folding part.

Current Problems:

• Caret abnormal when TOGGLEFOLDTEXT applied
  

// Example
// Text Before Folding
/**

// Text After Folding
[Here is the BLOB...]

// Current caret problem: line max position
[He|re is the BLOB...] //Problem: This is the last caret position because the original first line string(/**) is 3 char only

// Expected caret behaviour: line max position
[Here is the BLOB...]|

// Most Expected caret behaviour:
//Before Typing
|[Here is the BLOB...]

//Now Type 1 right arrow...
[Here is the BLOB...]|

//Now Type 1 left arrow again...
|[Here is the BLOB...]

• Leak of folding text block when ContractedState deallocation 
• Since seems it cannot iterate though it to delete (?)

Stefan Küng於 2016年9月10日星期六 UTC+8上午5時18分52秒寫道：

[Wonson]

...

About the Current Problem part, I have no idea how to do it. See if you can give me some direction or directly apply a quick fix.

Regards,

Wonson

Wonson於 2016年10月4日星期二 UTC+8下午4時22分26秒寫道：

[Neil Hodgson]

Wonson:

> Modification:
>
> • Selection caret changed when folding
> //Case 1:
> //Before folding: (caret inside folding text / selection part of folding text)
> ....
> {
> .... |
> }

This is another new feature and tangling it into fold header blobs will be confusing.

> The reason I made the feature is because of 2 points:
> 1. I notice that modifying the fold parent line might make the block losing the folding state, e.g.
> /** => /SomeTextHere**
> Though you remove "SomeTextHere", folding state can be back, so I think the document still holds the folded text. But in UI, when it lost the folding state, the lines just like somehow disappeared, which is strange.

When the fold causing text is removed, it should automatically unfold.

> 2. Also, when this TOGGLEFOLDTEXT feature applied. The text which occupied the whole line are supposed as one block and always occupying the fold parent line.

Replacing the whole line is not only possible behaviour. its more common to leave the header line text but to append a header blob. Like this
http://www.scintilla.org/FoldHeader.png

> <patch.diff>

CopyString doesn’t allocate an extra byte for a NUL which leads to displaying memory junk. Add 1 to strlen.

SCI_TOGGLEFOLDTEXT should both set the text in cs and then call FoldLine like SCI_TOGGLEFOLD.

Neil

[Wonson]

Neil, 

I changed the folding style, see if it is ok.

https://www.sendspace.com/file/h7riax

The video is a bug demo that shows no auto unfold when it lost folding state.

Wonson

Stefan Küng於 2016年9月10日星期六 UTC+8上午5時18分52秒寫道：

[Neil Hodgson]

Wonson:

> https://www.sendspace.com/file/h7riax
> The video is a bug demo that shows no auto unfold when it lost folding state.

ScintillaTest is a shell to experiment with and doesn’t always have everything properly implemented. Can you reproduce the problem with SciTE?

Neil

[Neil Hodgson]

Wonson:

> • Leak of folding text block when ContractedState deallocation

> • Since seems it cannot iterate though it to delete (?)

This is made more difficult because the SparseVector doesn’t know whether its items are values, class allocations or array allocations. It may be possible to use the C++ template machinery to discover this and do the right thing.

For now though, its simpler to add some code to ContractionState::DeleteLine to free the allocation. Its only dealing with a single line. Each line with a contraction tag has a unique allocation that is not shared with other lines. Thus, ContractionState should retrieve the value for the line and delete[] that allocation before calling DeleteRange.

In ContractionState::Clear iterate over all lines (or be more clever with EndRun) and delete each non-NULL value before performing “delete foldDispayTexts".

Neil

[Wonson]

Neil,

I found a Windows machine to run SciTE. There is auto unfold. I didn't try mac version.

ScintillaTest is a shell to experiment with and doesn’t always have everything properly implemented.

Then do you mean this behaviour should be implemented in application code? 

As I think backend should properly handle this case, and if it does, I suppose for any UI containing scintilla should see this same result.

Wonson

Neil Hodgson於 2016年10月4日星期二 UTC+8下午9時11分52秒寫道：

[Kit Yam Tse]

FYI, this problem affects some downstream projects, such as notepad++; however, some other downstream projects, such as Geany, is not affected by this project.

Kit

see also: https://github.com/notepad-plus-plus/notepad-plus-plus/issues/2013

[Wonson]

Neil,

I split the "reset caret when fold" feature and "toggle line text" into 2 patch.

Basically it works for me now. See if you are satisfied with the result.

Wonson

Neil Hodgson於 2016年10月4日星期二 UTC+8下午5時43分40秒寫道：

[Neil Hodgson]

Wonson:

> I split the "reset caret when fold" feature and "toggle line text" into 2 patch.
>
> Basically it works for me now. See if you are satisfied with the result.

I’d like to see what others think about the feature particularly the appearance. Other editors have various appearances and I wouldn’t want to make it difficult to emulate them.

Scintilla.h is automatically generated from Scintilla.iface so API changes should go there first. Then run scintilla/scripts/HFacer.py. Allocating the next message number:

# Switch a header line between expanded and contracted and show some text.
fun void ToggleFoldShowText=2698(int line, string text)

Since lexer styles restart at 40, STYLE_LASTPREDEFINED can not be changed.

DrawFoldDisplayText has unused arguments, subLine and xStart.

The StyledText class is used to allow annotations and margins with multiple styles. Its not needed for drawing a string in a single style. The width tracking doesn’t belong here and is inaccurate since it doesn’t take into account the start position of the text.

Checking character by character in the whole hidden region, which may be thousands of lines long is inefficient. Its probably enough to just check if the line end of the fold point line is selected. If you really want to only show selection if the whole hidden area is selected then make a range from the start to end then check through the selections seeing if they cover the whole range.

Drawing is performance sensitive and it may take some time to discover all the ramifications of these changes.

The patch uses spaces for indentation instead of tabs which also means it contains extra unnecessary lines.

RunStyles/SparseVector is made much more complex by splitting and merging runs. A simpler class that just deals with a single element at each partition start should be able to handle the needs of contraction tags. Its also likely that it will be useful for other features such as annotations. Attached is an incomplete and buggy SparseText class which is currently about a quarter of the size and is unlikely to be more than half the size when finished.

Existing SC_FOLDFLAG* just change appearance. RESETCARETTOFOLDSTART changes behaviour. It also removes any other selections and the rectangular selection mode.

Neil

[Neil Hodgson]

The current implementation doesn’t look great when the text is wrapped or line ends are visible.

For wrapped text, the contraction tag is drawn on each subline.
http://www.scintilla.org/FoldTagWrap.png

For visible line ends, the tag is drawn over the line end. It’d probably be OK to just not draw tags when in visible line ends mode. Another possibility is to have an explicit flag to make tags visible that would then suppress line ends.
http://www.scintilla.org/FoldTagEOL.png

Neil

[Wonson]

Neil,

Since lexer styles restart at 40, STYLE_LASTPREDEFINED can not be changed. 

# Styles in range 32..38 are predefined for parts of the UI and are not used as normal styles.

# Style 39 is for future use.

So can I use 39 for it? Then we use up all the predefined styles?

Wonson

 

Neil Hodgson於 2016年10月6日星期四 UTC+8上午6時00分31秒寫道：

[Wonson]

About the auto unfold "bug", I found that using SCI_SETAUTOMATICFOLD(SC_AUTOMATICFOLD_CHANGE) can fix it.

Wonson於 2016年10月5日星期三 UTC+8上午10時54分42秒寫道：

[Wonson]

But I still think caret should be move to a reasonable visible position when fold, just like some IDE does. Maybe it can only works when sel.Count == 1 for now, because I have no idea how the behaviour should be when multiple caret enabled.

Wonson於 2016年10月6日星期四 UTC+8下午12時03分54秒寫道：

[Wonson]

Neil,

This patch should fixed the below:

 Scintilla.h is automatically generated from Scintilla.iface so API changes should go there first.

 DrawFoldDisplayText has unused arguments

 Checking character by character in the whole hidden region, which may be thousands of lines long is inefficient. 

 For wrapped text, the contraction tag is drawn on each subline. 

 For visible line ends, the tag is drawn over the line end.

Also, I make use of the SparseText you provided.

However, about the StyleText, I leave it there for now, because I am not really familiar with the drawing code yet.

Wonson 

Neil Hodgson於 2016年10月6日星期四 UTC+8上午6時00分31秒寫道：

Wonson:

[Wonson]

Wonson於 2016年10月6日星期四 UTC+8下午6時06分10秒寫道：

[Wonson]

Fix indentation. Thanks to stupid XCode.

Wonson於 2016年10月6日星期四 UTC+8下午6時10分09秒寫道：

[Neil Hodgson]

Wonson:

> Fix indentation. Thanks to stupid XCode.

…
> <patch-togglelinetext.diff>

That set of patches has become complex and does not apply cleanly to current repository or 3.6.7. There doesn’t appear to be a changeset ID from the repository in the patch that would allow Hg to synchronise. A patch against 3.6.7 would be easier to apply.

A set of changed files produced from earlier patches with my changes can be downloaded from www.scintilla.org/scan.zip
This contains a better tested SparseVector class with specializations for const char * so SparseVector handles allocation and deallocation instead of client code.

Neil

[Neil Hodgson]

Wonson:

> But I still think caret should be move to a reasonable visible position when fold, just like some IDE does. Maybe it can only works when sel.Count == 1 for now, because I have no idea how the behaviour should be when multiple caret enabled.

For each selection: move out of way of collapsing text. Remove duplicates.

Neil

[Neil Hodgson]

Wonson:

> So can I use 39 for it? Then we use up all the predefined styles?

I’m uncertain. It depends on whether this is only going to use a single style or if it may use multiple styles like margins and annotations. Creating a StyledText makes it look like it wants multiple styles since that is the purpose of StyledText.

Neil

[Neil Hodgson]

Wonson:

> Then do you mean this behaviour should be implemented in application code?

The folding feature is meant to be very customisable to allow a wide variety of folding implementations. For example, an application may augment the fold points found by Scintilla’s folders with other fold points detected in the text or manually set by the user.

When Scintilla detects that the fold structure has changed in a way that could require lines to be shown, it sends an SCN_NEEDSHOWN notification to the application which should, in most cases, show the line. ScintillaTest does not handle SCN_NEEDSHOWN.
http://www.scintilla.org/ScintillaDoc.html#SCN_NEEDSHOWN

There are also automatic folding modes that allow applications to use generic implementations of aspects of folding.
http://www.scintilla.org/ScintillaDoc.html#SCI_SETAUTOMATICFOLD

Neil

[Neil Hodgson]

Neil Hodgson:

> That set of patches has become complex and does not apply cleanly to current repository or 3.6.7.

It appears that context lines in the patch don’t always start with a space character and this is confusing application of the patch. Line 1374, for example doesn’t start with ‘+’, ‘-‘, or ‘ ‘ so is treated as a comment instead of context. Either a mailer is munging the text or the patch creator is formatting incorrectly.

Neil

[Wonson]

Neil,

I try to patch it again, see if this is okay. And this implementation are based on 3.6.7 from the beginning.

(Email moment ago is attached incorrectly)

Wonson

Neil Hodgson於 2016年10月10日星期一 UTC+8下午1時42分10秒寫道：

[Neil Hodgson]

Wonson:

> I try to patch it again, see if this is okay. And this implementation are based on 3.6.7 from the beginning.
> (Email moment ago is attached incorrectly)

That applied better. You should have your repository ignore *.pyc files which are compiled Python so shouldn’t go into source code control.

With this code on Win32, the contraction tags don’t take account of the margins or horizontal scrolling so overlap the text and don’t move with it. Adding xStart to character positions is the key. You are probably checking only on Cocoa which uses a separate view for the margins and handles scrolling differently. If you can’t check on Win32 or GTK+, Scintilla builds with Qt on OS X and the Haven test app can be found at
http://www.scintilla.org/Haven.zip

Neil

[Neil Hodgson]

Here is a version of DrawFoldDisplayText with an xStart avoiding the StyledText class and drawing in multiple phases. Its too simplified as it doesn’t handle changing background for selection but its closer to being reasonable. It has to be called twice to draw the background and the foreground.

Neil

[Kit Yam Tse]

Neil,

Wonson is my colleague and he's working on something else now. So I repatch his changes and test it on Qt and macOS for him. 

The diffs with prefix test_ is used to activate the folding text in Qt and Cocoa test app. The feature diff is based on the latest commit, it can be apply on the branch easily.

It works great on Cocoa, however, I found that the fold display text can be selected in Qt app , but not Cocoa app. Any idea?

Kit

[Kit Yam Tse]

Hi all,

I just found that SparseText.h is not included in previous feature patch.

Here is the new patch with the file. 

Sorry for the inconvenience. 

Kit

[Neil Hodgson]

Kit Yam Tse:

> It works great on Cocoa, however, I found that the fold display text can be selected in Qt app , but not Cocoa app. Any idea?

The DrawFoldDisplayText included in the patch does not take into account multi-phase drawing. Its only called for phasesText but tries to draw text, background, selection, and outline rectangle.

Scintilla has 3 strategies for layered drawing with the most recent phasesMultiple being the ‘best’ (but slightly more expensive) with phasesTwo the default and phasesOne kept for compatibility.

If the desired functionality is to use a colour for the background in the standard manner over the tag then the best example would be the DrawEOL code which performs similar selection drawing for the [LF] and [CR][LF] blobs. It needs to be called in each possibly appropriate phase (drawBack, drawText, and drawSelectionTranslucent) and to perform the drawing needed for the passed phase. The rectangle goes over the background so could be drawn in drawText or possibly in the drawIndicatorsFore phase.

StyledText knows something about phases but it is expecting other circumstances so stop using StyledText. I posted some drawing code in a previous message which is incomplete but it respects the phase. The only reason for using StyledText is if the text may have different styles for some characters otherwise it is overly complex.

AlphaRectangle is used to draw both the background and outline rectangle but its using the style background colour in ways that are different to other styles. Drawing the background at half strength appears wrong to me: if a STYLE_ element is to determine the style of tags then it should use the full strength of the background colour like every other style does.

There is no style attribute for outline rectangle so, if this is to be drawn, it should be separate parameter or be determined algorithmically. It could be the text colour, a neutral mid-grey, or white|black determined as the most visible by being furthest from the background of either STYLE_FOLDDISPLAYTEXT or STYLE_DEFAULT ((r+g+b) > 3*0x7f) ? black : white.

Be sure to test with all three SCI_SETPHASESDRAW values. It would be acceptable to only support drawing fold tags in SC_PHASES_MULTIPLE or in both SC_PHASES_MULTIPLE and SC_PHASES_TWO. If this is done then there should not be any visual effect in the unsupported modes even if tags are set. It should also be documented.

Neil

[Kit Yam Tse]

   There is no style attribute for outline rectangle so, if this is to be drawn, it should be separate parameter or be determined algorithmically. It could be the text colour, a neutral mid-grey, or white|black determined as the most visible by being furthest from the background of either STYLE_FOLDDISPLAYTEXT or STYLE_DEFAULT ((r+g+b) > 3*0x7f) ? black : white.

Neil,

I think we should allow user to set the border colour of the rectangle, because predefined colours might not looks good on all themes. I have two ideas for setting this attribute, and they are:

1. introducing a set of new apis similar to indicators. We offer a set of predefined fold display box style, such as `FOLD_BOX` and `FOLD_ROUNDBOX`, and allow user set the fore, alpha, outline alpha, etc of these style.

2. introduce `SCI_STYLESETOUTLINE`. Not only the fold box, and also other styles will be able to have borders.

Any comment?

Kit 

[Neil Hodgson]

Kit Yam Tse:

> I think we should allow user to set the border colour of the rectangle, because predefined colours might not looks good on all themes. I have two ideas for setting this attribute, and they are:
>
> 1. introducing a set of new apis similar to indicators. We offer a set of predefined fold display box style, such as `FOLD_BOX` and `FOLD_ROUNDBOX`, and allow user set the fore, alpha, outline alpha, etc of these style.

I think this would be reasonable. It would also allow the box to be turned off. New constants should start with “SC_” for namespacing.

> 2. introduce `SCI_STYLESETOUTLINE`. Not only the fold box, and also other styles will be able to have borders.

This would require changing more code with some potentially subtle issues. For example, due to some platform limitations, text is segmented to ensure not too much is drawn at once and you don’t want to see box verticals drawn at these points.

[from before]

> It works great on Cocoa, however, I found that the fold display text can be selected in Qt app , but not Cocoa app. Any idea?

Something that you should be aware of is that there are 2 ways of drawing selections in Scintilla: as a solid background colour (ViewState::selAlpha == SC_ALPHA_NOALPHA) or as a translucent colour drawn over the text (ViewState::selAlpha is the transparency).

The selection range testing currently implemented inside DrawFoldDisplayText should be moved to a method on the Selection class similar to CharacterInSelection (since the Selection class is the centre for selection functionality) as
int Selection::SegmentInSelection(SelectionSegment ss) const

SelectionSegment is used because SelectionRange is unordered so its simpler to pass something with a guaranteed order. Ideally, SegmentInSelection would differentiate between the primary (1) and secondary selections (2) but for now returning 1 for any match is fine.

Neil

[Neil Hodgson]

The SparseVector.h file has been committed along with tests since it may be used for other features.

In ContractionState, it should be declared as
SparseVector<const char *> *foldDisplayLines;
Since there are specializations to handle memory management for <const char *>, there is no need for StringCopy or explicit freeing in ContractionState.

https://sourceforge.net/p/scintilla/code/ci/50b73fcff8d8ea84f0516865855223079315b18c/

Neil

[Kit Yam Tse]

Neil,

In Cocoa, we need `#include <assert.h>`, otherwise Xcode gives `~/scintilla-code/src/SparseVector.h:31:3: Use of undeclared identifier 'assert'` error.

Kit

[Kit Yam Tse]

Neil,

This patch should solve problems mentioned here. Folding text border is removed, I am working on a set of indicators like interfaces, which allow application set the folding box style.

  

The patch also fixes `select on folding display text` problem. The cause of the problem is that virtual spaces are ignored during calculate fold display text position, so select on the virtual spaces under fold display text looks like the fold display text is selected.

Kit

[Kit Yam Tse]

Neil,

In SplitVector.h, we used `PLATFORM_ASSERT` but not `assert`. If we use `PLATFORM_ASSERT` instead of assert`` in SparseVector.h, then there will be no error in Cocoa.

I think using `PLATFORM_ASSERT` is better then include a header file in another header file.

Kit

[Neil Hodgson]

Kit Yam Tse:

> Folding text border is removed, I am working on a set of indicators like interfaces, which allow application set the folding box style.

OK, but don’t define too many styles as it is difficult to predict what will be wanted.

> In SplitVector.h, we used `PLATFORM_ASSERT` but not `assert`. If we use `PLATFORM_ASSERT` instead of assert`` in SparseVector.h, then there will be no error in Cocoa.
> I think using `PLATFORM_ASSERT` is better then include a header file in another header file.

The #include <assert.h> only has to go in ContractionState.cxx since that is the only file that includes SparseVector.h currently. PLATFORM_ASSERT ties the data structure code to Scintilla where the Platform.h header is available and the data structure code is more widely useful than that. I would like to remove more PLATFORM_ASSERT calls in the future to make this code more reusable.

There are still calls to new char [] (CopyString) and delete [] in ContractionState.cxx which should be removed. Its safer to have SparseVector responsible for the memory management of the strings since it can ensure the correct thing happens in all circumstances.

The selection drawing works well in EditView now in my limited testing.

The WidestLine code doesn’t work though since it is measuring the width of just the tag and the lineWidthMaxSeen refers to the whole line including the text before the tag. The effect is that the horizontal scroller doesn’t grow to allow for tags which doesn’t really worry me for now. The similar code for annotations is OK because annotations have their own lines.

Leave STYLE_LASTPREDEFINED in Scintilla.iface/.h as it is used by clients including SciTE.

Neil

[Kit Yam Tse]

Neil,

> Folding text border is removed, I am working on a set of indicators like interfaces, which allow application set the folding box style.

   OK, but don’t define too many styles as it is difficult to predict what will be wanted.

I will add a new api `SCI_FOLDDISPLAYTEXTSETSTYLE(int style)`. There will be three styles:

1. `SC_FOLDDISPLAYTEXT_HIDDEN`, fold display text are not display
   
2. `SC_FOLDDISPLAYTEXT_STANDARD`, fold display text will be drawn without border
   
3. `SC_FOLDDISPLAYTEXT_BOXED`, fold display text are surrounded by a box, and the color of the box will be `SCI_STYLEGETFORE(STYLE_FOLDDISPLAYTEXT)`

By default, `SC_FOLDDISPLAYTEXT_STANDARD` will be used.

Kit

[Neil Hodgson]

Kit Yam Tse:

> I will add a new api `SCI_FOLDDISPLAYTEXTSETSTYLE(int style)`. There will be three styles:
> • `SC_FOLDDISPLAYTEXT_HIDDEN`, fold display text are not display
> • `SC_FOLDDISPLAYTEXT_STANDARD`, fold display text will be drawn without border
> • `SC_FOLDDISPLAYTEXT_BOXED`, fold display text are surrounded by a box, and the color of the box will be `SCI_STYLEGETFORE(STYLE_FOLDDISPLAYTEXT)`
> By default, `SC_FOLDDISPLAYTEXT_STANDARD` will be used.

OK. When you mentioned "indicator like” I thought it might mean something like BORDER_DASHED, BORDER_DOTTED, BORDER_WIGGLY, … which would be excessive in my opinion.

Neil

[Kit Yam Tse]

Neil,

Here is the changes of the patch

1. remove new/delete char * of fold text in ContractionState.cxx
2. ensure line max seen not smaller then the right border of fold text
3. add STYLE_LASTPREDEFINED back to iface
4. include <assert.h> in ContractionState.cxx
5. implement fold text style

Kit

[Neil Hodgson]

Kit Yam Tse:

> • implement fold text style

Thats looking quite good.

One issue is that with virtual space, the fold text box and background gets wider like this:
http://www.scintilla.org/FoldTagVS.png

To work in SCI_SETPHASESDRAW(0), the box should be drawn in a later phase so its not drawn over by the background. Its most similar to a foreground indicator so maybe
if (phase & drawIndicatorsFore)

Unless there is an intention to use multiple styles within the text, there is no reason to use the StyledText class. The tests stFoldText.text && ValidStyledText(vsDraw, 0, stFoldText) are always true. There can’t be new lines in the text so stFoldText.LineLength() is always strlen(foldDisplayText) and WidestLineWidth is the surface->WidthText of the whole text. stFoldText.style is always STYLE_FOLDDISPLAYTEXT.

Neil

[Kit Yam Tse]

Neil,

changes:

* fix fold text box width with visual space

* draw the box in drawIndicatorsFore phase

* remove test `stFoldText.text && ValidStyledText()`

* use `strlen()` to calculate fold text length

* use `WidthText()` to calculate fold text width

Kit

[Neil Hodgson]

Kit Yam Tse:

* fix fold text box width with visual space
* draw the box in drawIndicatorsFore phase
* remove test `stFoldText.text && ValidStyledText()`
* use `strlen()` to calculate fold text length
* use `WidthText()` to calculate fold text width

   Good.

   Looking at what gets drawn for each line showed there were some elements being redrawn multiple times. The DrawEOL draws the (commonly white) background at the end of lines and this was overdrawing some of the work in DrawFoldDisplayText which was then drawn again.

   There is a fair bit going on in DrawFoldDisplayText so its worth making it efficient. In SC_FOLDDISPLAYTEXT_HIDDEN mode, its effectively turned off so there is minimal cost. This should be checked very early in DrawFoldDisplayText. I think its is also reasonable to make this the default mode so an application has to opt in to the feature. This test makes the subsequent tests against the remaining 2 states redundant so that test should be removed.

   The background was being drawn in the drawIndicatorsFore phase and the code was executed before the text phase so, when multiple phases were drawn in one call, the text was being overdrawn by the background. Background versus text is the most important pat of draw layering: the background should always be drawn before the text. Added a separate block for drawBack and moved it before the text drawing.

   The call to DrawFoldDisplayText has to go after DrawEOL so that it draws the fold text over the line end so the modified was moved. This still isn’t as efficient as possible (there should be a test that one of the phases drawn in DrawFoldDisplayText is active) but its better than before. Here is the changed code from EditView.cxx (also attached):

void EditView::DrawFoldDisplayText(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,

                                                         int line, int xStart, PRectangle rcLine, int subLine, XYACCUMULATOR subLineStart, DrawPhase phase) {

       const bool lastSubLine = subLine == (ll->lines - 1);

       if (!lastSubLine)

               return;

       if ((model.foldDisplayTextStyle == SC_FOLDDISPLAYTEXT_HIDDEN) || !model.cs.GetFoldDisplayTextShown(line))

               return;

       PRectangle rcSegment = rcLine;

       const char *foldDisplayText = model.cs.GetFoldDisplayText(line);

       const int lengthFoldDisplayText = static_cast<int>(strlen(foldDisplayText));

       FontAlias fontText = vsDraw.styles[STYLE_FOLDDISPLAYTEXT].font;

       const int widthFoldDisplayText = static_cast<int>(surface->WidthText(fontText, foldDisplayText, lengthFoldDisplayText));

       int eolInSelection = 0;

       int alpha = SC_ALPHA_NOALPHA;

       if (!hideSelection) {

               int posAfterLineEnd = model.pdoc->LineStart(line + 1);

               eolInSelection = (subLine == (ll->lines - 1)) ? model.sel.InSelectionForEOL(posAfterLineEnd) : 0;

               alpha = (eolInSelection == 1) ? vsDraw.selAlpha : vsDraw.selAdditionalAlpha;

       }

       const XYPOSITION spaceWidth = vsDraw.styles[ll->EndLineStyle()].spaceWidth;

       XYPOSITION virtualSpace = model.sel.VirtualSpaceFor(model.pdoc->LineEnd(line)) * spaceWidth;

       rcSegment.left = xStart + static_cast<XYPOSITION>(ll->positions[ll->numCharsInLine] - subLineStart) + spaceWidth + virtualSpace;

       rcSegment.right = rcSegment.left + static_cast<XYPOSITION>(widthFoldDisplayText);

       ColourOptional background = vsDraw.Background(model.pdoc->GetMark(line), model.caret.active, ll->containsCaret);

       FontAlias textFont = vsDraw.styles[STYLE_FOLDDISPLAYTEXT].font;

       ColourDesired textFore = vsDraw.styles[STYLE_FOLDDISPLAYTEXT].fore;

       if (eolInSelection && (vsDraw.selColours.fore.isSet)) {

               textFore = (eolInSelection == 1) ? vsDraw.selColours.fore : vsDraw.selAdditionalForeground;

       }

       ColourDesired textBack = TextBackground(model, vsDraw, ll, background, eolInSelection,

                                                                                       false, STYLE_FOLDDISPLAYTEXT, -1);

       if (model.trackLineWidth) {

               if (rcSegment.right + 1> lineWidthMaxSeen) {

                       // Fold display text border drawn on rcSegment.right with width 1 is the last visble object of the line

                       lineWidthMaxSeen = static_cast<int>(rcSegment.right + 1);

               }

       }

       if ((phasesDraw != phasesOne) && (phase & drawBack)) {

               surface->FillRectangle(rcSegment, textBack);

       }

       if (phase & drawText) {

               if (phasesDraw != phasesOne) {

                       surface->DrawTextTransparent(rcSegment, textFont,

                               rcSegment.top + vsDraw.maxAscent, foldDisplayText,

                               lengthFoldDisplayText, textFore);

               } else {

                       surface->DrawTextNoClip(rcSegment, textFont,

                               rcSegment.top + vsDraw.maxAscent, foldDisplayText,

                               lengthFoldDisplayText, textFore, textBack);

               }

       }

       if (phase & drawIndicatorsFore) {

               if (model.foldDisplayTextStyle == SC_FOLDDISPLAYTEXT_BOXED) {

                       surface->PenColour(textFore);

                       surface->MoveTo(static_cast<int>(rcSegment.left), static_cast<int>(rcSegment.top));

                       surface->LineTo(static_cast<int>(rcSegment.left), static_cast<int>(rcSegment.bottom));

                       surface->MoveTo(static_cast<int>(rcSegment.right), static_cast<int>(rcSegment.top));

                       surface->LineTo(static_cast<int>(rcSegment.right), static_cast<int>(rcSegment.bottom));

                       surface->MoveTo(static_cast<int>(rcSegment.left), static_cast<int>(rcSegment.top));

                       surface->LineTo(static_cast<int>(rcSegment.right), static_cast<int>(rcSegment.top));

                       surface->MoveTo(static_cast<int>(rcSegment.left), static_cast<int>(rcSegment.bottom - 1));

                       surface->LineTo(static_cast<int>(rcSegment.right), static_cast<int>(rcSegment.bottom - 1));

               }

       }

       if (phase & drawSelectionTranslucent) {

               if (eolInSelection && vsDraw.selColours.back.isSet && (line < model.pdoc->LinesTotal() - 1) && alpha != SC_ALPHA_NOALPHA) {

                       SimpleAlphaRectangle(surface, rcSegment, SelectionBackground(vsDraw, eolInSelection == 1, model.primarySelection), alpha);

               }

       }

}

...

void EditView::DrawLine(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,

       int line, int lineVisible, int xStart, PRectangle rcLine, int subLine, DrawPhase phase) {

       if (subLine >= ll->lines) {

               DrawAnnotation(surface, model, vsDraw, ll, line, xStart, rcLine, subLine, phase);

               return; // No further drawing

       }

       // See if something overrides the line background color.

       const ColourOptional background = vsDraw.Background(model.pdoc->GetMark(line), model.caret.active, ll->containsCaret);

       const int posLineStart = model.pdoc->LineStart(line);

       const Range lineRange = ll->SubLineRange(subLine);

       const XYACCUMULATOR subLineStart = ll->positions[lineRange.start];

       if ((ll->wrapIndent != 0) && (subLine > 0)) {

               if (phase & drawBack) {

                       DrawWrapIndentAndMarker(surface, vsDraw, ll, xStart, rcLine, background, customDrawWrapMarker);

               }

               xStart += static_cast<int>(ll->wrapIndent);

       }

       if ((phasesDraw != phasesOne) && (phase & drawBack)) {

               DrawBackground(surface, model, vsDraw, ll, rcLine, lineRange, posLineStart, xStart,

                       subLine, background);

               DrawEOL(surface, model, vsDraw, ll, rcLine, line, lineRange.end,

                       xStart, subLine, subLineStart, background);

       }

       if (phase & drawIndicatorsBack) {

               DrawIndicators(surface, model, vsDraw, ll, line, xStart, rcLine, subLine, lineRange.end, true, model.hoverIndicatorPos);

               DrawEdgeLine(surface, vsDraw, ll, rcLine, lineRange, xStart);

               DrawMarkUnderline(surface, model, vsDraw, line, rcLine);

       }

       if (phase & drawText) {

               DrawForeground(surface, model, vsDraw, ll, lineVisible, rcLine, lineRange, posLineStart, xStart,

                       subLine, background);

       }

       if (phase & drawIndentationGuides) {

               DrawIndentGuidesOverEmpty(surface, model, vsDraw, ll, line, lineVisible, rcLine, xStart, subLine);

       }

       if (phase & drawIndicatorsFore) {

               DrawIndicators(surface, model, vsDraw, ll, line, xStart, rcLine, subLine, lineRange.end, false, model.hoverIndicatorPos);

       }

       // End of the drawing of the current line

       if (phasesDraw == phasesOne) {

               DrawEOL(surface, model, vsDraw, ll, rcLine, line, lineRange.end,

                       xStart, subLine, subLineStart, background);

       }

       DrawFoldDisplayText(surface, model, vsDraw, ll, line, xStart, rcLine, subLine, subLineStart, phase);

       if (!hideSelection && (phase & drawSelectionTranslucent)) {

               DrawTranslucentSelection(surface, model, vsDraw, ll, line, rcLine, subLine, lineRange, xStart);

       }

       if (phase & drawLineTranslucent) {

               DrawTranslucentLineState(surface, model, vsDraw, ll, line, rcLine);

       }

}

[Kit Yam Tse]

Neil,

changes:

1. Rebase the patch to commit 2a226d95a66d88d6b83815c6e97b5912b8023d8b

2. Replace `DrawFoldDisplayText()` with the one found in attachment of previous message

3. Replace `DrawLine()` with the one found in attachment of previous message

4. Add a new function `fillLineRemainder()`, which is used to fill the background from the end of last element of the line to the end of the line.

    The function will be called once in each line drawing once and only once by either `DrawFoldDisplayText()` or `DrawEOL()`. i.e. no overdrawing

5. Set `SC_FOLDDISPLAYTEXT_HIDDEN` as default fold display text style

Kit

[Neil Hodgson]

Kit Yam Tse:

Thanks. I am willing to commit that code as is. There are a few minor quibbles though that you can ignore if you want.

> 4. Add a new function `fillLineRemainder()`, which is used to fill the background from the end of last element of the line to the end of the line.
> The function will be called once in each line drawing once and only once by either `DrawFoldDisplayText()` or `DrawEOL()`. i.e. no overdrawing

1. There is a +1 in the “Fill remainder” section of DrawFoldDislayText that appears to be to avoid overdrawing the box’s right line. I think this is better done by moving the code section to just after the other ‘drawBack’ section (and copying rcSegment into a temporary instead of modifying it) so that the box lines overdraw this line remainder background.

2. The ‘ColourOptional background’ is now recalculated in 3 locations making it more difficult to change. I think it would be better to pass the value calculated in DrawLine through DrawFoldDislayText to fillLineRemainder.

3. The box is drawn with a sequence of 8 MoveTo or LineTo calls which can be simplified. Since LineTo moves the current position, its possible to use 1 MoveTo and 4 LineTo calls. Its likely the current code is because it was copied from ANNOTATION_BOXED which draws a connected box over multiple lines which isn’t the case here.

4. Methods get initial capitals in Scintilla so fillLineRemainder -> FillLineRemainder.

Neil

[Neil Hodgson]

The folding code has been committed as
Main:
https://sourceforge.net/p/scintilla/code/ci/be11b0c900e967e3171220a2113faac6ccdb04c2/
Fix for API:
https://sourceforge.net/p/scintilla/code/ci/7431301fedd036f190e9ce66375d23a83a66c243/

> 1. There is a +1 in the “Fill remainder” section of DrawFoldDislayText that appears to be to avoid overdrawing the box’s right line. I think this is better done by moving the code section to just after the other ‘drawBack’ section (and copying rcSegment into a temporary instead of modifying it) so that the box lines overdraw this line remainder background.

Made half of this change: the code has been moved and a temporary used but the +1 hasn’t been fixed.

> 4. Methods get initial capitals in Scintilla so fillLineRemainder -> FillLineRemainder.

This was fixed as well.

The box drawing and code duplication issues mentioned in the last post were not fixed.

Neil