Discussion:
elastic tabstops for Scintilla
Nick Gravgaard
2013-02-18 21:29:51 UTC
Permalink
Hi everyone.

I posted to this mailing list some years ago when I was thinking about
how I might implement elastic tabstops
(http://nickgravgaard.com/elastictabstops/) for Scintilla. I never got
around to doing that, but some time ago David Kinder approached me to
tell me he'd ported my old Gedit (GTK) code to Scintilla as part of his
work on the Windows version of Inform 7 (a system for writing text
adventure games). With David's blessings I've put the code up on GitHub
at https://github.com/nickgravgaard/ElasticTabstopsForScintilla

I hope some of you will take this code and use it in your own projects
(a web search shows there seems to be some demand for this feature). If
you need to figure out how to convert text formatted using elastic
tabstops to use spaces instead (and vice versa), there's some Python
code I wrote which does that at
http://pypi.python.org/pypi/ElasticTabstops/ - hopefully it should be
straightforward to port to other languages.

What do you think? - if I can help in any way just shout,
Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
Philippe Lhoste
2013-02-20 16:35:04 UTC
Permalink
On 18/02/2013 22:29, Nick Gravgaard wrote:
> I posted to this mailing list some years ago when I was thinking about
> how I might implement elastic tabstops
> (http://nickgravgaard.com/elastictabstops/) for Scintilla. I never got
> around to doing that, but some time ago David Kinder approached me to
> tell me he'd ported my old Gedit (GTK) code to Scintilla as part of his
> work on the Windows version of Inform 7 (a system for writing text
> adventure games). With David's blessings I've put the code up on GitHub
> at https://github.com/nickgravgaard/ElasticTabstopsForScintilla
>
> I hope some of you will take this code and use it in your own projects
> (a web search shows there seems to be some demand for this feature). If
> you need to figure out how to convert text formatted using elastic
> tabstops to use spaces instead (and vice versa), there's some Python
> code I wrote which does that at
> http://pypi.python.org/pypi/ElasticTabstops/ - hopefully it should be
> straightforward to port to other languages.
>
> What do you think? - if I can help in any way just shout,

I don't think I would use it in code, as I don't align code or comments in the way you
show in your page.
But sometime I open some TSV (tab-separated values) file, and I often have to set tab
stops at a high value (eg. 50) to be sure everything is aligned, with some columns looking
empty. So, yes, I would use the feature in such use case.
Anyway, the idea is nice and useful.

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

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
Mike Lischke
2013-02-20 17:05:45 UTC
Permalink
>> What do you think? - if I can help in any way just shout,
>
> I don't think I would use it in code, as I don't align code or comments in the way you show in your page.
> But sometime I open some TSV (tab-separated values) file, and I often have to set tab stops at a high value (eg. 50) to be sure everything is aligned, with some columns looking empty. So, yes, I would use the feature in such use case.
> Anyway, the idea is nice and useful.


There's certainly use for this in normal text documents and such, but IMO not in source code. Actually, I'm fighting for years in all the teams I have worked in plus all my projects over internet *not* to use any tab in source code files. The reason is simple: the layout is messed up as soon as the code is displayed in anything else but the writer's editor. Tab settings are different, so it often looks terrible. I'm sure Nick knows that, but he has other priorities than I have (and so many others saying that tabs in code are bad).

No intention to start a tab vs. space war here!

Mike
--
www.soft-gems.net

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
Neil Hodgson
2013-02-21 01:15:17 UTC
Permalink
Nick Gravgaard:

> With David's blessings I've put the code up on GitHub
> at https://github.com/nickgravgaard/ElasticTabstopsForScintilla

There may be uses for explicit tab stops in other cases than elastic tabstops.

There are a couple of problems with the changes to Scintilla. The main one is that the tab stop positions are held by the document instead of the view, Editor. When there are multiple views on a document, each view may have different visual choices - fonts, sizes and magnification may differ between views so the tab positions will be at different pixel positions. Tab stops should be stored by Editor but it does not have the 'per-line' infrastructure implemented in Document so Editor would need an additional data structure to hold each line's tab stops and code to maintain that when lines are added and deleted.

Scintilla's external interface is described in include/Scintilla.iface which only allows a few argument types which do not include int*. A pair of APIs could be used: TabStopsClear(line) and TabStopAdd(line, xStop).

The elastic tabs implementation calls SCI_TEXTWIDTH(0, ...) which returns the width measured in style 0. Multiple styles may be used in Scintilla with different widths: for example a monospaced font for identifiers, and a proportional italic font for comments. Measuring only in style 0 may lead to the columns being the wrong width and text appearing in a different column than desired.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
Nick Gravgaard
2014-07-08 22:10:31 UTC
Permalink
On Thu, 21 Feb 2013, at 02:15, Neil Hodgson wrote:
>
> Nick Gravgaard:
>
> > With David's blessings I've put the code up on GitHub
> > at https://github.com/nickgravgaard/ElasticTabstopsForScintilla
>
> There may be uses for explicit tab stops in other cases than elastic
> tabstops.
>
> There are a couple of problems with the changes to Scintilla. The main
> one is that the tab stop positions are held by the document instead of
> the view, Editor. When there are multiple views on a document, each
> view may have different visual choices - fonts, sizes and
> magnification may differ between views so the tab positions will be at
> different pixel positions. Tab stops should be stored by Editor but it
> does not have the 'per-line' infrastructure implemented in Document so
> Editor would need an additional data structure to hold each line's tab
> stops and code to maintain that when lines are added and deleted.
>
> Scintilla's external interface is described in include/Scintilla.iface
> which only allows a few argument types which do not include int*. A
> pair of APIs could be used: TabStopsClear(line) and TabStopAdd(line,
> xStop).
>
> The elastic tabs implementation calls SCI_TEXTWIDTH(0, ...) which
> returns the width measured in style 0. Multiple styles may be used in
> Scintilla with different widths: for example a monospaced font for
> identifiers, and a proportional italic font for comments. Measuring
> only in style 0 may lead to the columns being the wrong width and text
> appearing in a different column than desired.

I finally got around to doing something about this.

Neil, can you have a look at the new code at
https://github.com/nickgravgaard/ElasticTabstopsForScintilla and see
what you think?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-09 00:09:51 UTC
Permalink
Nick Gravgaard:

> I finally got around to doing something about this.
>
> Neil, can you have a look at the new code at
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla and see
> what you think?

LineTabStops is a PerLine but its not connected up to anything that provides the InsertLine and RemoveLine calls responsible for maintaining the list's correspondence to the document. It probably doesn't matter as inserting or deleting a line probably requires recalculating the tabstops for all the following lines.

Since changing tabstops from an Editor instance should only affect that instance, it should not be necessary to go through Document::NotifyModified - instead call Editor::NotifyModified directly. It may only need to call Redraw().

SplitVector is split so that large blocks of data work: there will be few tabstops in a line so its better to use std::vector for TabstopList. It is likely worthwhile retaining SplitVector for the list of lines, however, although its benefits would mostly be if InsertLine/RemoveLine were hooked up.

Scintilla's API convention doesn't include integer arrays as they are more difficult to create in scripting languages. The conventional way of exposing this functionality would be with a SCI_CLEARTABSTOPS(line) and SCI_ADDTABSTOP(line, x). Maybe add a SCI_GETNEXTTABSTOP(line, x) to allow the application code to retrieve the tab stops and also allow it to avoid extra updates and painting when the stops don't change.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-22 02:01:14 UTC
Permalink
On Wed, 9 Jul 2014, at 01:09, Neil Hodgson wrote:

Nick Gravgaard:


I finally got around to doing something about this.



Neil, can you have a look at the new code at

[1]https://github.com/nickgravgaard/ElasticTabstopsForScintilla

and see what you think?


LineTabStops is a PerLine but its not connected up to
anything that provides the InsertLine and RemoveLine calls
responsible for maintaining the list’s correspondence to the
document. It probably doesn’t matter as inserting or deleting a
line probably requires recalculating the tabstops for all the
following lines.



Since changing tabstops from an Editor instance should only
affect that instance, it should not be necessary to go through
Document::NotifyModified - instead call Editor::NotifyModified
directly. It may only need to call Redraw().



SplitVector is split so that large blocks of data work:
there will be few tabstops in a line so its better to use
std::vector for TabstopList. It is likely worthwhile retaining
SplitVector for the list of lines, however, although its
benefits would mostly be if InsertLine/RemoveLine were hooked
up.



Scintilla’s API convention doesn’t include integer arrays as
they are more difficult to create in scripting languages. The
conventional way of exposing this functionality would be with a
SCI_CLEARTABSTOPS(line) and SCI_ADDTABSTOP(line, x). Maybe add
a SCI_GETNEXTTABSTOP(line, x) to allow the application code to
retrieve the tab stops and also allow it to avoid extra updates
and painting when the stops don’t change.



Okay, I've just pushed some new code to GitHub. Neil, can you
have another look?



The changes to the API make parts of the code much simpler,
which is usually a good sign :)



Nick

References

1. https://github.com/nickgravgaard/ElasticTabstopsForScintilla

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-22 09:53:17 UTC
Permalink
Nick Gravgaard:

> Okay, I've just pushed some new code to GitHub. Neil, can you have another look?
>
> The changes to the API make parts of the code much simpler, which is usually a good sign :)

It can probably be made a bit simpler still. I can't see why NextTabsstopPos needs a Document argument. The Document class can leave the NotifyModified method private as the new code is using Editor::NotifyModified. This means that Document.h is now unmodified.

SC_MOD_CHANGELINESTATE isn't a generic notification for changing a line - it is used by lexers to mark state changes that are not encoded into the style, mostly due to too few bits in a style byte. Things like current scripting language or comment nesting level. A new notification type should be defined, called something like SC_MOD_CHANGETABSTOPS. The application may want to see current SC_MOD_CHANGELINESTATE but not be interested in observing tab stop changes since it is generating the tab stop changes.

The solution won't build for me as it is missing an afxwin.h file, so I haven't seen it work.

Documentation for the three new calls would help.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-22 16:36:29 UTC
Permalink
On Tue, 22 Jul 2014, at 10:53, Neil Hodgson wrote:
> Nick Gravgaard:
>
> > Okay, I've just pushed some new code to GitHub. Neil, can you have another look?
> >
> > The changes to the API make parts of the code much simpler, which is usually a good sign :)
>
> It can probably be made a bit simpler still. I can't see why
> NextTabsstopPos needs a Document argument. The Document class can
> leave the NotifyModified method private as the new code is using
> Editor::NotifyModified. This means that Document.h is now unmodified.

To make this a bit easier I'm replying inline with GitHub commit links.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/259d73b75d5de53615c63d7de72b564ba0685528

> SC_MOD_CHANGELINESTATE isn't a generic notification for changing a
> line - it is used by lexers to mark state changes that are not encoded
> into the style, mostly due to too few bits in a style byte. Things
> like current scripting language or comment nesting level. A new
> notification type should be defined, called something like
> SC_MOD_CHANGETABSTOPS. The application may want to see current
> SC_MOD_CHANGELINESTATE but not be interested in observing tab stop
> changes since it is generating the tab stop changes.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/ac369574fa0b6906a2d0aa02b1f8159791bda42f

> The solution won't build for me as it is missing an afxwin.h file, so
> I haven't seen it work.

Hmm, it works for me on VS 2013 but I don't seem to have one in my
solution - the only thing I can find is a reference to
c:\Program Files (x86)\Microsoft Visual Studio
12.0\VC\atlmfc\include\afxwin.h
I haven't done any MFC stuff for ages. What can I do on my end to make
this solution buildable for you?

> Documentation for the three new calls would help.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/9c51039d09c6f38d92ff415c6e8604f3192c3d4d

-----

I've just noticed a problem where LineTabstops::InsertLine isn't being
called when I insert a line in the buffer. I remembered you wrote this
back in an old email:

> Tab stops should be stored by Editor but it does not have
> the 'per-line' infrastructure implemented in Document so Editor would need an additional data
> structure to hold each line's tab stops and code to maintain that when lines are added and deleted.

In my code, Editor has an instance of LineTabstops, but I need to write
the "code to maintain that when lines are added and deleted". How would
you like me to do this?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-22 20:00:59 UTC
Permalink
On 14-07-22 09:36 AM, Nick Gravgaard wrote:
> On Tue, 22 Jul 2014, at 10:53, Neil Hodgson wrote:
>> [snip]
>> The solution won't build for me as it is missing an afxwin.h file, so
>> I haven't seen it work.
>
> Hmm, it works for me on VS 2013 but I don't seem to have one in my
> solution - the only thing I can find is a reference to
> c:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\atlmfc\include\afxwin.h
> I haven't done any MFC stuff for ages. What can I do on my end to make
> this solution buildable for you?
>

You probably need the full version of Visual Studio (as opposed to
"express" editions) to get those MFC headers or at least installing one
of those Windows SDKs that provide them.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-23 00:34:06 UTC
Permalink
Nick Gravgaard:

> To make this a bit easier I'm replying inline with GitHub commit links.
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/259d73b75d5de53615c63d7de72b564ba0685528

Yes, that is easy to understand.

>> SC_MOD_CHANGETABSTOPS. The application may want to see current
>> SC_MOD_CHANGELINESTATE but not be interested in observing tab stop
>> changes since it is generating the tab stop changes.
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/ac369574fa0b6906a2d0aa02b1f8159791bda42f

If you want to experiment with only redrawing the line, the API is InvalidateRange as used about 20 lines after your change in NotifyModified. I suspect that changing tab positions will often happen over multiple lines so there will not be a large performance difference either way.

>> The solution won't build for me as it is missing an afxwin.h file, so
>> I haven't seen it work.
>
> Hmm, it works for me on VS 2013 but I don't seem to have one in my
> solution - the only thing I can find is a reference to
> c:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\atlmfc\include\afxwin.h
> I haven't done any MFC stuff for ages. What can I do on my end to make
> this solution buildable for you?

OK, I see now. I'll download a full version of Visual C++.

>> Documentation for the three new calls would help.
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/9c51039d09c6f38d92ff415c6e8604f3192c3d4d

Thanks.

> In my code, Editor has an instance of LineTabstops, but I need to write
> the "code to maintain that when lines are added and deleted". How would
> you like me to do this?

Look at the code in Editor::NotifyModified that maintains the ContractionState which has a similar need to update for line insertion and deletion. Put your change next to the calls to cs.InsertLines / cs.DeleteLines.

There are a bunch of minor issues that I can fix when this is ready. Scintilla.h doesn't get directly modified since it is generated from Scintilla.iface. Some of the formatting such as brace positions is different to the rest of Scintilla. The Editor class has been refactored since the 3.4.4 release with some functionality moved into EditModel and EditView so the location of ldTabStops and the methods will have to be decided. Since the tab stops are used when laying out they probably belong in EditView which now owns LayoutLine.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-23 09:52:17 UTC
Permalink
On Wed, 23 Jul 2014, at 01:34, Neil Hodgson wrote:
> Nick Gravgaard:
>
> >> SC_MOD_CHANGETABSTOPS. The application may want to see current
> >> SC_MOD_CHANGELINESTATE but not be interested in observing tab stop
> >> changes since it is generating the tab stop changes.
> >
> > https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/ac369574fa0b6906a2d0aa02b1f8159791bda42f
>
> If you want to experiment with only redrawing the line, the API is
> InvalidateRange as used about 20 lines after your change in
> NotifyModified. I suspect that changing tab positions will often
> happen over multiple lines so there will not be a large performance
> difference either way.

I experimented quickly but couldn't get InvalidateRange to work, so I
left it as it was.

> > In my code, Editor has an instance of LineTabstops, but I need to write
> > the "code to maintain that when lines are added and deleted". How would
> > you like me to do this?
>
> Look at the code in Editor::NotifyModified that maintains the
> ContractionState which has a similar need to update for line insertion
> and deletion. Put your change next to the calls to cs.InsertLines /
> cs.DeleteLines.

Brilliant, that did the trick! :)

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-23 03:10:57 UTC
Permalink
Downloaded VC++ 2014 and its running. There are a couple of approximately 1K leaks seen at exit.

More of the feature would be checked if there was some styling. SCI_SETLEXER(SCLEX_CPP) then make the comments big and green SCI_STYLESETFORE(SCE_C_COMMENT, 0x008000) SCI_STYLESETSIZE(SCE_C_COMMENT, 14).

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-23 11:27:23 UTC
Permalink
On Wed, 23 Jul 2014, at 04:10, Neil Hodgson wrote:
> Downloaded VC++ 2014 and its running. There are a couple of
> approximately 1K leaks seen at exit.

Fun - I'll look into it.

> More of the feature would be checked if there was some styling.
> SCI_SETLEXER(SCLEX_CPP) then make the comments big and green
> SCI_STYLESETFORE(SCE_C_COMMENT, 0x008000)
> SCI_STYLESETSIZE(SCE_C_COMMENT, 14).

I tried doing this here:
https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/d9b6303fc23719cc866a88871b27a74fc53c50ed

Comments are now green, but the size remains the same as the rest of the
text. I tried making them italic but that didn't work either. Any ideas?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-23 22:20:19 UTC
Permalink
Nick Gravgaard:

> I tried doing this here:
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/d9b6303fc23719cc866a88871b27a74fc53c50ed
>
> Comments are now green, but the size remains the same as the rest of the
> text. I tried making them italic but that didn't work either. Any ideas?

There should be a SCI_STYLECLEARALL before setting individual style elements. The normal sequence is to set the base font and size in STYLE_DEFAULT then call SCI_STYLECLEARALL to make all styles use that font/size.

> I fixed one memory leak here:
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/ea55b01c1a08cf641712a167e8a5f1b3af3fc847
>
> Visual Leak Detector says there is still one memory leak. but I haven't
> been able to fix it yet. I think it may be due to the use of the global
> grid_buffer pointer which points to frequently realloc'd memory, but I'm
> not sure.

Leaks in the demo app aren't important but fixing all the demo app leaks makes it easier to see if there are leaks in the new Scintilla code such as could occur, for example, when a line is deleted.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-27 14:02:55 UTC
Permalink
On Wed, 23 Jul 2014, at 23:20, Neil Hodgson wrote:
> > Comments are now green, but the size remains the same as the rest of the
> > text. I tried making them italic but that didn't work either. Any ideas?
>
> There should be a SCI_STYLECLEARALL before setting individual style
> elements. The normal sequence is to set the base font and size in
> STYLE_DEFAULT then call SCI_STYLECLEARALL to make all styles use that
> font/size.

Brilliant, that did it. In doing so I found a bug in my elastic tabstops
code which I've also fixed.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/1ea2b3482451c126b6dd43774a16f967e1e52570

> Leaks in the demo app aren't important but fixing all the demo app
> leaks makes it easier to see if there are leaks in the new Scintilla
> code such as could occur, for example, when a line is deleted.

Visual Leak Detector now says no memory leaks were detected.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/c2b685fe5b89facf80643fb7e9dad671bb7dc2fa

Do you think you will be able to merge the Scintilla changes soon? If
not, what else needs to be done?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-27 23:42:28 UTC
Permalink
Nick Gravgaard:

> Do you think you will be able to merge the Scintilla changes soon? If
> not, what else needs to be done?

The tab positions vector is fully allocated for each line even if the application never sets a tab stop. Each line has at least an empty std::vector so that is probably 3 pointer/ints.

Applications that do not use tab stops should not have to pay this cost, probably by deferring allocation of ldTabStops to when SCI_ADDTABSTOP is initially called. The other code then needs to be made safe for the not-yet-allocated state.

Another area to look at is document switching (SetDocPointer) and deletion (ClearAll). When the document is switched, the tab stops are no longer valid and should be deleted and the application may then recreate tab stops for the new document. The ContractionState variable cs has similar life time issues and can be a guide.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-28 21:27:11 UTC
Permalink
On Mon, 28 Jul 2014, at 00:42, Neil Hodgson wrote:
> Nick Gravgaard:
>
> > Do you think you will be able to merge the Scintilla changes soon? If
> > not, what else needs to be done?
>
> The tab positions vector is fully allocated for each line even if the
> application never sets a tab stop. Each line has at least an empty
> std::vector so that is probably 3 pointer/ints.
>
> Applications that do not use tab stops should not have to pay this
> cost, probably by deferring allocation of ldTabStops to when
> SCI_ADDTABSTOP is initially called. The other code then needs to be
> made safe for the not-yet-allocated state.

Sounds sensible.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/6bbd39573b7cb04a752d80ea1c28495df605b4eb

> Another area to look at is document switching (SetDocPointer) and
> deletion (ClearAll). When the document is switched, the tab stops are
> no longer valid and should be deleted and the application may then
> recreate tab stops for the new document. The ContractionState variable
> cs has similar life time issues and can be a guide.

How does this look?

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/cc5df447c0df880ef0e098404e57a4186675c62f

Anything else?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-29 02:51:22 UTC
Permalink
Nick Gravgaard:

> How does this look?
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/cc5df447c0df880ef0e098404e57a4186675c62f

From a quick examination it looks good but I'll want to think a bit more about it.

Is performance adequate on large functions like Editor::WndProc?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-29 21:30:23 UTC
Permalink
On Tue, 29 Jul 2014, at 03:51, Neil Hodgson wrote:
> Nick Gravgaard:
>
> > How does this look?
> >
> > https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/cc5df447c0df880ef0e098404e57a4186675c62f
>
> From a quick examination it looks good but I'll want to think a bit
> more about it.

Okay - please tell me if you need anything from me. I'd love to see
Scintilla become one of the text widgets that support explicit tabstops,
and as you said before, there are other uses for them besides elastic
tabstops (which is why Visual Studio, GtkSourceView and Java Swing's
text widget support them).

My goal with this work has always been to get explicit tabstops into
Scintilla so that all the editors that use it can easily implement
elastic tabstops. If you search the internet you'll find there are lots
of people who really want this functionality.

> Is performance adequate on large functions like Editor::WndProc?

Performance seems okay to me. In my demo app I copied and pasted the
table at the bottom of the example text so I had a table over 100 lines
long and 4 columns wide and it still seemed reasonable. I'm pretty sure
any slow down would be due to the non-optimal elastic tabstops
implementation in my demo app rather than the explicit tabstop code in
my branch of Scintilla.

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-30 07:07:16 UTC
Permalink
Nick Gravgaard:

> Okay - please tell me if you need anything from me.

If you are interested you could update the copy of Scintilla you are modifying to the current state in Hg, since Editor has been broken up. That way, we'll both be checking the final form instead of me trying to apply your changes to the new structure. ldTabStops and related methods should probably move to EditView since they affect layout while the external API stays on Editor.

When handling line deletion inside Editor::NotifyModified, it looks like the lines being deleted are not contiguous since the loop index is incrementing. Say its asked to delete 3 lines starting at line 0. It will delete lines 0, 1, and 2. However, after line 0 has been deleted, line 1 moves to the 0 position and line 2 is in the 1 position. Thus original line numbers 0, 2, and 4 are deleted.

Neil


--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-05 01:08:41 UTC
Permalink
Me:

> If you are interested you could update the copy of Scintilla you are modifying to the current state in Hg, since Editor has been broken up.

This isn't important.

> When handling line deletion inside Editor::NotifyModified, it looks like the lines being deleted are not contiguous since the loop index is incrementing. Say its asked to delete 3 lines starting at line 0. It will delete lines 0, 1, and 2. However, after line 0 has been deleted, line 1 moves to the 0 position and line 2 is in the 1 position. Thus original line numbers 0, 2, and 4 are deleted.

This is. Configurable tabstops should not be included until I hear my analysis is wrong or the code is fixed.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-05 08:51:05 UTC
Permalink
On Tue, 5 Aug 2014, at 02:08, Neil Hodgson wrote:
> Me:
>
> > If you are interested you could update the copy of Scintilla you are modifying to the current state in Hg, since Editor has been broken up.
>
> This isn't important.
>
> > When handling line deletion inside Editor::NotifyModified, it looks like the lines being deleted are not contiguous since the loop index is incrementing. Say its asked to delete 3 lines starting at line 0. It will delete lines 0, 1, and 2. However, after line 0 has been deleted, line 1 moves to the 0 position and line 2 is in the 1 position. Thus original line numbers 0, 2, and 4 are deleted.
>
> This is. Configurable tabstops should not be included until I hear my
> analysis is wrong or the code is fixed.

Okay, I'll try and have a look at this tonight.

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-05 22:42:59 UTC
Permalink
On Tue, 5 Aug 2014, at 09:51, Nick Gravgaard wrote:
> On Tue, 5 Aug 2014, at 02:08, Neil Hodgson wrote:
> > Me:
> >
> > > When handling line deletion inside Editor::NotifyModified, it looks like the lines being deleted are not contiguous since the loop index is incrementing. Say its asked to delete 3 lines starting at line 0. It will delete lines 0, 1, and 2. However, after line 0 has been deleted, line 1 moves to the 0 position and line 2 is in the 1 position. Thus original line numbers 0, 2, and 4 are deleted.
> >
> > This is. Configurable tabstops should not be included until I hear my
> > analysis is wrong or the code is fixed.
>
> Okay, I'll try and have a look at this tonight.

Is this okay?

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/447b60ee0f72b000ca6fb94bde057b0626bcf83f

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-06 06:17:39 UTC
Permalink
Nick Gravgaard:

> Is this okay?
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/447b60ee0f72b000ca6fb94bde057b0626bcf83f

Isn't that off-by-one? Asked to delete lines 1-3 it will delete lines 2-4.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-06 22:36:07 UTC
Permalink
On Wed, 6 Aug 2014, at 07:17, Neil Hodgson wrote:
> Nick Gravgaard:
>
> > Is this okay?
> >
> > https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/447b60ee0f72b000ca6fb94bde057b0626bcf83f
>
> Isn't that off-by-one? Asked to delete lines 1-3 it will delete lines
> 2-4.

Sorry, my mistake :(

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/31ddf452f922cf1c314d5af96d4a4cda97019c3c

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-08 00:09:21 UTC
Permalink
Committed as
https://sourceforge.net/p/scintilla/code/ci/3f3ae214c626d7276aee35a14d1ffa1ad848c9cd/

Much of the code was moved into the EditView class.

Some basic tests added to test/simpleTests.py although more tests are always good as they can prevent regressions.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-08 08:44:59 UTC
Permalink
On Fri, 8 Aug 2014, at 01:09, Neil Hodgson wrote:
> Committed as
> https://sourceforge.net/p/scintilla/code/ci/3f3ae214c626d7276aee35a14d1ffa1ad848c9cd/

Brilliant :)

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-08 14:39:39 UTC
Permalink
On 14-08-08 01:44 AM, Nick Gravgaard wrote:
> On Fri, 8 Aug 2014, at 01:09, Neil Hodgson wrote:
>> Committed as
>> https://sourceforge.net/p/scintilla/code/ci/3f3ae214c626d7276aee35a14d1ffa1ad848c9cd/
>
> Brilliant :)
>

Any chance of some details/docs or samples on how apps can implement
this with Scintilla? It looks like part of the implementation will be
left up to each application (why?), so it'd be cool to have a little
information on what they need to do. I would like to add support in
Geany if it's not too difficult or too much code.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-08 14:55:53 UTC
Permalink
On Fri, 8 Aug 2014, at 15:39, Matthew Brush wrote:
> On 14-08-08 01:44 AM, Nick Gravgaard wrote:
> > On Fri, 8 Aug 2014, at 01:09, Neil Hodgson wrote:
> >> Committed as
> >> https://sourceforge.net/p/scintilla/code/ci/3f3ae214c626d7276aee35a14d1ffa1ad848c9cd/
> >
> > Brilliant :)
> >
>
> Any chance of some details/docs or samples on how apps can implement
> this with Scintilla? It looks like part of the implementation will be
> left up to each application (why?), so it'd be cool to have a little
> information on what they need to do. I would like to add support in
> Geany if it's not too difficult or too much code.

Sure. I have a working implementation of elastic tabstops for Scintilla
here:

https://github.com/nickgravgaard/ElasticTabstopsForScintilla

Feel free to reuse the code, but I'd love to know if you make any
improvements (pull requests are especially welcome).

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-09 12:54:55 UTC
Permalink
On 14-08-08 07:55 AM, Nick Gravgaard wrote:
> On Fri, 8 Aug 2014, at 15:39, Matthew Brush wrote:
>> On 14-08-08 01:44 AM, Nick Gravgaard wrote:
>>> On Fri, 8 Aug 2014, at 01:09, Neil Hodgson wrote:
>>>> Committed as
>>>> https://sourceforge.net/p/scintilla/code/ci/3f3ae214c626d7276aee35a14d1ffa1ad848c9cd/
>>>
>>> Brilliant :)
>>>
>>
>> Any chance of some details/docs or samples on how apps can implement
>> this with Scintilla? It looks like part of the implementation will be
>> left up to each application (why?), so it'd be cool to have a little
>> information on what they need to do. I would like to add support in
>> Geany if it's not too difficult or too much code.
>
> Sure. I have a working implementation of elastic tabstops for Scintilla
> here:
>
> https://github.com/nickgravgaard/ElasticTabstopsForScintilla
>
> Feel free to reuse the code, but I'd love to know if you make any
> improvements (pull requests are especially welcome).
>

Thanks, I'll have a try at porting it to C/GTK+ for Geany.

Would it be possible to put the implementation logic into Scintilla so
it "just works" out of the box? Maybe modifying `SCI_SETUSETABS` or
adding new message like `SCI_SETINDENTMODE` to take enum parameter
similar to:

SC_SPACES =0,
SC_TABS =1,
SC_ELASTICTABSTOPS =2

I have no idea if this is feasible, but it seems like if the same
implementation needs to be written for each application, putting it into
Scintilla would make using elastic tabstops more simple and consistent
across applications.

Thanks,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-08-10 22:01:26 UTC
Permalink
On Sat, 9 Aug 2014, at 13:54, Matthew Brush wrote:
> Would it be possible to put the implementation logic into Scintilla so
> it "just works" out of the box? Maybe modifying `SCI_SETUSETABS` or
> adding new message like `SCI_SETINDENTMODE` to take enum parameter
> similar to:
>
> SC_SPACES =0,
> SC_TABS =1,
> SC_ELASTICTABSTOPS =2
>
> I have no idea if this is feasible, but it seems like if the same
> implementation needs to be written for each application, putting it into
> Scintilla would make using elastic tabstops more simple and consistent
> across applications.

I'd like this too, and moving the implementation of elastic tabstops
inside the text widget would mean that a significant performance
improvement could be made.

It's not my decision to make though, and I have an idea about how to
extend Scintilla in a generic way that would give me the performance
benefits I want. I'll describe that in another email.

Neil, what do you think about me moving the implementation of elastic
tabstops inside Scintilla?

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-10 23:17:07 UTC
Permalink
Matthew Brush:

> Thanks, I'll have a try at porting it to C/GTK+ for Geany.
>
> Would it be possible to put the implementation logic into Scintilla so it "just works" out of the box?

Elastic tabstops is a feature that, to me, doesn't really work well. There is an implementation available for gedit, which I have tried in the past but wasn't able to install just now in Mint 17 with real gedit (as well as Pluma) installed.

Sufficient demand and promises of maintenance would get it into Scintilla. I think it would be better for an application using Scintilla to refine an implementation and demonstrate that it was actually wanted before including within Scintilla.

Elastic tabstops will decrease performance. Its likely that this will be reasonable on normal functions but it will cause problems on some larger files. My understanding of the code is that it, upon every modification, finds the line preceding and succeeding the change that have text in column 0 (often the scope of a function). Then it finds the positions of columns and sets them for all lines in the range. To find the accurate column positions, all lines in the range must be lexed and layed out which takes some time so may make the UI unresponsive or be implemented as a background operation which adds complexity (such as ensuring that the elastic background operations precedes any wrapping background operations).

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Nick Gravgaard
2014-07-23 14:06:36 UTC
Permalink
On Wed, 23 Jul 2014, at 04:10, Neil Hodgson wrote:
> Downloaded VC++ 2014 and its running. There are a couple of
> approximately 1K leaks seen at exit.

I fixed one memory leak here:
https://github.com/nickgravgaard/ElasticTabstopsForScintilla/commit/ea55b01c1a08cf641712a167e8a5f1b3af3fc847

Visual Leak Detector says there is still one memory leak. but I haven't
been able to fix it yet. I think it may be due to the use of the global
grid_buffer pointer which points to frequently realloc'd memory, but I'm
not sure.

https://github.com/nickgravgaard/ElasticTabstopsForScintilla/blob/ea55b01c1a08cf641712a167e8a5f1b3af3fc847/ElasticTabstops.cpp#L190

Nick

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Loading...