Bug 333370

Comment 1 Felipe Heidrich 2011-01-04 11:13:34 EST

Note that all classes org.eclipse.swt.graphics are final.
I can't change that.

What are doing ? What do you mean by 'refactoring of StyledText'
?

Comment 2 Aaron Digulla 2011-01-04 14:04:00 EST

Well, I forked SWT and StyledText and have started with a some refactoring to make the code more reusable/manageable/easier to test and to compile.

When I say "test", I mean this: http://epen.hg.sourceforge.net/hgweb/epen/StyledText/file/5a5ee083a71c/src/test/java/de/pdark/styledtext/StyledTextTest.java

This code runs all the existing Snippets and verifies the result without opening a shell.

See http://epen.hg.sourceforge.net/hgweb/epen/ for the projects. SWT-Fork is the ground work, StyledText is just most editor stuff from org.eclipse.swt.custom so I can experiment more easily.

My ultimate goal is to turn StyledText into an editor framework that you can use to build powerful editors which includes layered, hierarchical styles (so you can have styles that inherit from each other or a spelling error layer that is easy to create, and update) or which offers enough hooks to allow to synchronize the editor model with a second model or replace the underlying editor model with, say, a CharBuffer-based implementation so search'n'replace on a large file doesn't take that long, etc. Or fix bugs like Bug 35779 "Text Viewer and Editor needs to support word wrap"

The project allows to experiment with the code without breaking the Eclipse release cycle. If something turns out to be good and stable, it can be back ported.

> I can't change that.

If you can't decide this, who can?

Comment 3 Felipe Heidrich 2011-01-04 15:48:31 EST

- Interesting work, wouldn't it be easier to start from the scratch (instead of changing StyledText).

- Just to make sure, do you want us to change the public API in SWT in order for you to run tests ?
-- it is still not clear for me why you need to subclass TextLayout
-- if you already have a fork of SWT, why don't you change TextLayout in your fork ?

Comment 4 Aaron Digulla 2011-01-04 15:58:36 EST

(In reply to comment #3)
> - Interesting work, wouldn't it be easier to start from the scratch (instead of
> changing StyledText).

I was wondering the same thing. But as it happens, it's easier than it looks to refactor the beast. For example, I could move all code to manage styles in 2-3 hours into a StyleRangeModel.

A next step could be to create an interface for it so we can have custom style models that extend the existing one.

> - Just to make sure, do you want us to change the public API in SWT in order
> for you to run tests ?

I would like to know if it is possible to merge my changes back into the official SWT. So whenever I find something that meets some criteria (small change, small impact, no big performance loss, etc), I'd report that back to avoid drifting too far away from the official code.

> -- it is still not clear for me why you need to subclass TextLayout

So I can get rid of disposeTextLayout() in StyledTextRenderer. With the change, I can add this class:

public class CachedTextLayout extends TextLayout {
	
	public CachedTextLayout(Device device) {
		super(device);
	}

	@Override
	public void dispose() {
		// Do nothing
	}
	
	public void cachedDispose() {
		super.dispose();
	}
}

and return that from getTextLayout(). Call sites then can always invoke dispose() on the layout and I don't need to search the cache every time.

> -- if you already have a fork of SWT, why don't you change TextLayout in your
> fork ?

See above.

Comment 5 Felipe Heidrich 2011-01-04 16:39:55 EST

I see what you mean. But I prefer to defer any changes that involves changing
the API.


Keep in mind that StyledText stability and performance are vital to Eclipse, for that reason we are not very keen about big changes to it as it always requires extensive testing.

Comment 6 Lars Vogel 2019-09-24 13:50:13 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

Comment 7 Alexander Fedorov 2019-09-27 03:59:18 EDT

Removing "final" modifier will break "A good object comes from either a final or abstract class" rule [1]

[1] https://www.yegor256.com/2014/11/20/seven-virtues-of-good-object.html

I suggest to close this one

Comment 8 Niraj Modi 2019-09-27 04:25:07 EDT

(In reply to Alexander Fedorov from comment #7)
> Removing "final" modifier will break "A good object comes from either a
> final or abstract class" rule [1]
> 
> [1] https://www.yegor256.com/2014/11/20/seven-virtues-of-good-object.html
> 
> I suggest to close this one

Closing, as per above reasons cited in comment 5 and comment 7