Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

Hi Scott,

The CSS reference is here:

modules/javafx.graphics/src/main/docs/javafx/scene/doc-files/cssref.html

As for your question about indentation, reformatting entire files or 
large blocks of code you aren't touching is discouraged. In your 
specific case, reformatting the methods in the StyleableProperties 
nested class that are adjacent to code you add or modify seems fine, as 
long as you only change the indentation and not the line wrapping. This 
will allow reviewers to turn on "hide whitespace changes" (which is an 
option of the GitHub diffs view, and is default for the webrev).


-- Kevin


On 11/5/2019 6:03 PM, Scott Palmer wrote:

I finally had a moment to get back at this.

I've changed the name of the property from tabWidth to tabSize and 
implemented the CSS property '-fx-tab-size'. The CSS documentation 
will need to be updated. I didn't see where the CSS document is 
located in the source tree.


While adding CSS support I noticed that in both Text.java and 
TextFlow.java the indentation is wrong for the StyleableProperties 
nested class (has one extra space).  I wasn't sure how much I should 
change in the scope of this feature.  If I don't fix some indenting my 
changes will look funny beside the lines of existing code.  If I fix 
all of the broken indenting I'll change a lot of lines that have 
nothing to do with this feature. If I just properly indent the lines I 
change or add, the indenting will be more chaotic than it is currently.


I have noticed a rendering anomaly with a tab size of zero (the value 
I was initially clamping to) and rather than dealing with that I took 
the easy way out and changed it to agree with the minimum of 1 that 
Kevin indicated for the range.


I have not limited the maximum tab size.  I know Phil has expressed 
that MAXINT is not his preference, but I can't think of a good 
criteria for what the maximum should be.  I can imagine someone using 
a tab separator to align columns of text that have much more than 16 
characters for example.  I did a quick test with the tab size set to 
Integer.MAX_VALUE-1 and nothing blew up.  Of course the text after the 
tab was nowhere to be seen.



Regards,

Scott


On Thu, Oct 17, 2019 at 3:42 PM Phil Race > wrote:




On 10/17/19 12:32 PM, Kevin Rushforth wrote:
> It seems that you have a path forward then. So, there are a couple
> questions to answer:
>
> 1. Should the type of the property be int or double? If it
really is
> only ever going to be a number of spaces, then int seems fine.
That,
> along with the name of the property, would underscore the fact that
> no, this isn't a size where units make sense; it's a number of
spaces.

A discrete number of spaces, so int.

>
> 2. Should the range be [1,MAXINT] or [1,16] or [1,something-else]?
> Once that is decided, I think clamp on use would be the most
> consistent with other similar properties, but that's worth checking.

If we go with "MAXINT" (which is not my preference), I think there'd
need to be some testing of
the various code paths and pipelines to be sure that various
values from
large, through extreme,
all do something sensible, and of course, no exceptions or crashes.

Various code paths means Text, TextFlow,  linewrapping or not ...
complex text and simple text too.
Perhaps complex text should be tested anyway, although I've been
assuming that we are handling
tab spacing outside of that but didn't verify it.

-phil.

>
> -- Kevin
>
>
> On 10/17/2019 12:11 PM, Scott Palmer wrote:
>> You’re right.  Something I was reading indicated that while
there was
>> no default unit, ‘px’ was implicitly appended.  I can’t find that
>> now. Go figure.
>>
>> Everything I find now about CSS tab-size is in agreement with what
>> David wrote.  It’s a number of spaces.  With units it would be a
>> ‘length’ but nothing supports that.
>>
>> So I think it makes sense to add -fx-tab-size as a CSS
property, only
>> supporting a number with no units.
>>
>> Scott
>>
>>
>>> On Oct 17, 2019, at 2:32 PM, Phil Race mailto:philip.r...@oracle.com>> wrote:
>>>
>>> Really ? It defaults to pixels ? Is that just inherited as
being the
>>> default CSS unit ?
>>> Is that FX's implementation
>>>
>>> Hmm. A bit of reading about web CSS says that strictly anything
>>> without an explicit unit should be ignored.
>>> The only exception is zero, where it doesn't matter.
>>>
>>> Eg see :
>>>
https://stackoverflow.com/questions/7907749/default-unit-for-font-size
>>>
>>> Although I am sure there are more authoratative sources than that.
>>>
>>> -phil.
>>>
>>> On 10/17/19 11:22 AM, Scott Palmer wrote:
 So do we go ahead and implement tabSize without the CSS support?
 I’m not sure 

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

I finally had a moment to get back at this.

I've changed the name of the property from tabWidth to tabSize and
implemented the CSS property '-fx-tab-size'.  The CSS documentation will
need to be updated. I didn't see where the CSS document is located in the
source tree.

While adding CSS support I noticed that in both Text.java and TextFlow.java
the indentation is wrong for the StyleableProperties nested class (has one
extra space).  I wasn't sure how much I should change in the scope of this
feature.  If I don't fix some indenting my changes will look funny beside
the lines of existing code.  If I fix all of the broken indenting I'll
change a lot of lines that have nothing to do with this feature.  If I just
properly indent the lines I change or add, the indenting will be more
chaotic than it is currently.

I have noticed a rendering anomaly with a tab size of zero (the value I was
initially clamping to) and rather than dealing with that I took the easy
way out and changed it to agree with the minimum of 1 that Kevin indicated
for the range.

I have not limited the maximum tab size.  I know Phil has expressed that
MAXINT is not his preference, but I can't think of a good criteria for what
the maximum should be.  I can imagine someone using a tab separator to
align columns of text that have much more than 16 characters for example.
I did a quick test with the tab size set to Integer.MAX_VALUE-1 and nothing
blew up.  Of course the text after the tab was nowhere to be seen.


Regards,

Scott


On Thu, Oct 17, 2019 at 3:42 PM Phil Race  wrote:

>
>
> On 10/17/19 12:32 PM, Kevin Rushforth wrote:
> > It seems that you have a path forward then. So, there are a couple
> > questions to answer:
> >
> > 1. Should the type of the property be int or double? If it really is
> > only ever going to be a number of spaces, then int seems fine. That,
> > along with the name of the property, would underscore the fact that
> > no, this isn't a size where units make sense; it's a number of spaces.
>
> A discrete number of spaces, so int.
>
> >
> > 2. Should the range be [1,MAXINT] or [1,16] or [1,something-else]?
> > Once that is decided, I think clamp on use would be the most
> > consistent with other similar properties, but that's worth checking.
>
> If we go with "MAXINT" (which is not my preference), I think there'd
> need to be some testing of
> the various code paths and pipelines to be sure that various values from
> large, through extreme,
> all do something sensible, and of course, no exceptions or crashes.
>
> Various code paths means Text, TextFlow,  linewrapping or not ...
> complex text and simple text too.
> Perhaps complex text should be tested anyway, although I've been
> assuming that we are handling
> tab spacing outside of that but didn't verify it.
>
> -phil.
>
> >
> > -- Kevin
> >
> >
> > On 10/17/2019 12:11 PM, Scott Palmer wrote:
> >> You’re right.  Something I was reading indicated that while there was
> >> no default unit, ‘px’ was implicitly appended.  I can’t find that
> >> now. Go figure.
> >>
> >> Everything I find now about CSS tab-size is in agreement with what
> >> David wrote.  It’s a number of spaces.  With units it would be a
> >> ‘length’ but nothing supports that.
> >>
> >> So I think it makes sense to add -fx-tab-size as a CSS property, only
> >> supporting a number with no units.
> >>
> >> Scott
> >>
> >>
> >>> On Oct 17, 2019, at 2:32 PM, Phil Race  wrote:
> >>>
> >>> Really ? It defaults to pixels ? Is that just inherited as being the
> >>> default CSS unit ?
> >>> Is that FX's implementation
> >>>
> >>> Hmm. A bit of reading about web CSS says that strictly anything
> >>> without an explicit unit should be ignored.
> >>> The only exception is zero, where it doesn't matter.
> >>>
> >>> Eg see :
> >>> https://stackoverflow.com/questions/7907749/default-unit-for-font-size
> >>>
> >>> Although I am sure there are more authoratative  sources than that.
> >>>
> >>> -phil.
> >>>
> >>> On 10/17/19 11:22 AM, Scott Palmer wrote:
>  So do we go ahead and implement tabSize without the CSS support?
>  I’m not sure the CSS property should be added before unit issues
>  are worked out, as I think the units would be expected in the CSS
>  context. E.g. CSS implicitly adds ‘px’ if no unit is specified.  If
>  our tabSize units are spaces, that breaks.
> 
>  Scott
> 
> > On Oct 17, 2019, at 2:16 PM, Kevin Rushforth
> >  wrote:
> >
> > It might make sense to just add the tabSize property now, and
> > later consider adding a tabUnits property in the future if needed.
> > By default, having the units be "number of spaces in the current
> > font" is what makes the most sense, so before we could add
> > tabUnits we would need to extend it as you suggest. I'm not sure
> > it's needed, though, so that would be another reason not to do it
> > now.
> >
> > It's probably best to have the type of tabSize be double rather
> > than 

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

I think what we’ve settled on fits CSS for the web. I agree we shouldn’t 
deviate from that if possible. 

Scott

> On Oct 17, 2019, at 5:30 PM, Pedro Duque Vieira  
> wrote:
> 
> Hi,
> 
> Kevin asked for developers opinions on this issue, just thought I'd leave
> my own.
> 
> Didn't have time to read the thread thoroughly enough, don't have much time
> at the moment to do it. Apologies for that.
> 
> Just wanted to make a comment about the CSS support for this feature. I
> think if you add it, it would be great if it follows the CSS web spec or at
> least it doesn't go against it.
> I think there's a lot of value if we start more and more following the web
> css spec, it would mean a web designer could eventually transfer his skills
> directly to javafx or that we could use tools used in the web or that we
> could simply copy paste a CSS web example into JavaFX and it would just
> work (though we're still a bit far from that but could be a possibility in
> the future).
> 
> There are a good deal of big companies already involved in developing web
> css, I'm sure they've put a lot of thought into it (not speaking of the
> early days of CSS which was very bad).
> 
> Thanks,
> 
> -- 
> Pedro Duque Vieira - https://www.pixelduke.com

Re: JDK-8130738 TextFlow's tab width is static

[Pedro Duque Vieira]

Hi,

Kevin asked for developers opinions on this issue, just thought I'd leave
my own.

Didn't have time to read the thread thoroughly enough, don't have much time
at the moment to do it. Apologies for that.

Just wanted to make a comment about the CSS support for this feature. I
think if you add it, it would be great if it follows the CSS web spec or at
least it doesn't go against it.
I think there's a lot of value if we start more and more following the web
css spec, it would mean a web designer could eventually transfer his skills
directly to javafx or that we could use tools used in the web or that we
could simply copy paste a CSS web example into JavaFX and it would just
work (though we're still a bit far from that but could be a possibility in
the future).

There are a good deal of big companies already involved in developing web
css, I'm sure they've put a lot of thought into it (not speaking of the
early days of CSS which was very bad).

Thanks,

-- 
Pedro Duque Vieira - https://www.pixelduke.com

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

On 10/17/19 12:32 PM, Kevin Rushforth wrote:
It seems that you have a path forward then. So, there are a couple 
questions to answer:


1. Should the type of the property be int or double? If it really is 
only ever going to be a number of spaces, then int seems fine. That, 
along with the name of the property, would underscore the fact that 
no, this isn't a size where units make sense; it's a number of spaces.


A discrete number of spaces, so int.



2. Should the range be [1,MAXINT] or [1,16] or [1,something-else]? 
Once that is decided, I think clamp on use would be the most 
consistent with other similar properties, but that's worth checking.


If we go with "MAXINT" (which is not my preference), I think there'd 
need to be some testing of
the various code paths and pipelines to be sure that various values from 
large, through extreme,

all do something sensible, and of course, no exceptions or crashes.

Various code paths means Text, TextFlow,  linewrapping or not ... 
complex text and simple text too.
Perhaps complex text should be tested anyway, although I've been 
assuming that we are handling

tab spacing outside of that but didn't verify it.

-phil.



-- Kevin


On 10/17/2019 12:11 PM, Scott Palmer wrote:
You’re right.  Something I was reading indicated that while there was 
no default unit, ‘px’ was implicitly appended.  I can’t find that 
now. Go figure.


Everything I find now about CSS tab-size is in agreement with what 
David wrote.  It’s a number of spaces.  With units it would be a 
‘length’ but nothing supports that.


So I think it makes sense to add -fx-tab-size as a CSS property, only 
supporting a number with no units.


Scott



On Oct 17, 2019, at 2:32 PM, Phil Race  wrote:

Really ? It defaults to pixels ? Is that just inherited as being the 
default CSS unit ?

Is that FX's implementation

Hmm. A bit of reading about web CSS says that strictly anything 
without an explicit unit should be ignored.

The only exception is zero, where it doesn't matter.

Eg see : 
https://stackoverflow.com/questions/7907749/default-unit-for-font-size


Although I am sure there are more authoratative  sources than that.

-phil.

On 10/17/19 11:22 AM, Scott Palmer wrote:
So do we go ahead and implement tabSize without the CSS support?   
I’m not sure the CSS property should be added before unit issues 
are worked out, as I think the units would be expected in the CSS 
context. E.g. CSS implicitly adds ‘px’ if no unit is specified.  If 
our tabSize units are spaces, that breaks.


Scott

On Oct 17, 2019, at 2:16 PM, Kevin Rushforth 
 wrote:


It might make sense to just add the tabSize property now, and 
later consider adding a tabUnits property in the future if needed. 
By default, having the units be "number of spaces in the current 
font" is what makes the most sense, so before we could add 
tabUnits we would need to extend it as you suggest. I'm not sure 
it's needed, though, so that would be another reason not to do it 
now.


It's probably best to have the type of tabSize be double rather 
than int. We do this for most attributes to leave the door open 
for fractional values. I don't know why anyone would want a tab 
that was 5.7 spaces, but if we ever were to add a tabUnits 
property, I could easily see wanting fractional values for some 
units.


-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon 
to attempt adding the CSS property as you mention in your 
previous email.  I considered tabSize as a name as well.  I don’t 
have a preference if we leave things as representing tabs as a 
number of spaces.  But it wouldn’t be too difficult to support 
units, making it mostly compatible with CSS rules.  The way I 
envision that is having two properties, tabSize and tabUnit.


David mentioned javafx.css.SizeUnits… I looked quickly at the 
java docs for it, and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no 
CSS unit like ‘ems’ but meaning the width of a space in the 
current font.  The problem with that is it would leave out the 
possibility to set the tab width to anything relative to the 
current implementation of 8 spaces. (In hindsight it should have 
been implemented based on ‘ems’, which for a fixed width font as 
typically used in a code editor would be the same as 8 spaces 
anyway)
Do we add something to SizeUnits so we can work around this?  
e.g. ‘fxsp’  (FX-space - fx prefix to avoid a potential collision 
with any future official CSS units)


// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width 
of space in current font


If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile 
benefit to enforcing a maximum.  If we want to stor

Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

It seems that you have a path forward then. So, there are a couple 
questions to answer:


1. Should the type of the property be int or double? If it really is 
only ever going to be a number of spaces, then int seems fine. That, 
along with the name of the property, would underscore the fact that no, 
this isn't a size where units make sense; it's a number of spaces.


2. Should the range be [1,MAXINT] or [1,16] or [1,something-else]? Once 
that is decided, I think clamp on use would be the most consistent with 
other similar properties, but that's worth checking.


-- Kevin


On 10/17/2019 12:11 PM, Scott Palmer wrote:

You’re right.  Something I was reading indicated that while there was no 
default unit, ‘px’ was implicitly appended.  I can’t find that now. Go figure.

Everything I find now about CSS tab-size is in agreement with what David wrote. 
 It’s a number of spaces.  With units it would be a ‘length’ but nothing 
supports that.

So I think it makes sense to add -fx-tab-size as a CSS property, only 
supporting a number with no units.

Scott



On Oct 17, 2019, at 2:32 PM, Phil Race  wrote:

Really ? It defaults to pixels ? Is that just inherited as being the default 
CSS unit ?
Is that FX's implementation

Hmm. A bit of reading about web CSS says that strictly anything without an 
explicit unit should be ignored.
The only exception is zero, where it doesn't matter.

Eg see : https://stackoverflow.com/questions/7907749/default-unit-for-font-size

Although I am sure there are more authoratative  sources than that.

-phil.

On 10/17/19 11:22 AM, Scott Palmer wrote:

So do we go ahead and implement tabSize without the CSS support?   I’m not sure 
the CSS property should be added before unit issues are worked out, as I think 
the units would be expected in the CSS context.  E.g. CSS implicitly adds ‘px’ 
if no unit is specified.  If our tabSize units are spaces, that breaks.

Scott


On Oct 17, 2019, at 2:16 PM, Kevin Rushforth  wrote:

It might make sense to just add the tabSize property now, and later consider adding a 
tabUnits property in the future if needed. By default, having the units be "number 
of spaces in the current font" is what makes the most sense, so before we could add 
tabUnits we would need to extend it as you suggest. I'm not sure it's needed, though, so 
that would be another reason not to do it now.

It's probably best to have the type of tabSize be double rather than int. We do 
this for most attributes to leave the door open for fractional values. I don't 
know why anyone would want a tab that was 5.7 spaces, but if we ever were to 
add a tabUnits property, I could easily see wanting fractional values for some 
units.

-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent w

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

You’re right.  Something I was reading indicated that while there was no 
default unit, ‘px’ was implicitly appended.  I can’t find that now. Go figure.

Everything I find now about CSS tab-size is in agreement with what David wrote. 
 It’s a number of spaces.  With units it would be a ‘length’ but nothing 
supports that.

So I think it makes sense to add -fx-tab-size as a CSS property, only 
supporting a number with no units.

Scott


> On Oct 17, 2019, at 2:32 PM, Phil Race  wrote:
> 
> Really ? It defaults to pixels ? Is that just inherited as being the default 
> CSS unit ?
> Is that FX's implementation
> 
> Hmm. A bit of reading about web CSS says that strictly anything without an 
> explicit unit should be ignored.
> The only exception is zero, where it doesn't matter.
> 
> Eg see : 
> https://stackoverflow.com/questions/7907749/default-unit-for-font-size
> 
> Although I am sure there are more authoratative  sources than that.
> 
> -phil.
> 
> On 10/17/19 11:22 AM, Scott Palmer wrote:
>> So do we go ahead and implement tabSize without the CSS support?   I’m not 
>> sure the CSS property should be added before unit issues are worked out, as 
>> I think the units would be expected in the CSS context.  E.g. CSS implicitly 
>> adds ‘px’ if no unit is specified.  If our tabSize units are spaces, that 
>> breaks.
>> 
>> Scott
>> 
>>> On Oct 17, 2019, at 2:16 PM, Kevin Rushforth  
>>> wrote:
>>> 
>>> It might make sense to just add the tabSize property now, and later 
>>> consider adding a tabUnits property in the future if needed. By default, 
>>> having the units be "number of spaces in the current font" is what makes 
>>> the most sense, so before we could add tabUnits we would need to extend it 
>>> as you suggest. I'm not sure it's needed, though, so that would be another 
>>> reason not to do it now.
>>> 
>>> It's probably best to have the type of tabSize be double rather than int. 
>>> We do this for most attributes to leave the door open for fractional 
>>> values. I don't know why anyone would want a tab that was 5.7 spaces, but 
>>> if we ever were to add a tabUnits property, I could easily see wanting 
>>> fractional values for some units.
>>> 
>>> -- Kevin
>>> 
>>> On 10/17/2019 10:40 AM, Scott Palmer wrote:
 Hi Phil,
 
 Thanks for taking a look.  I was going to get back to this soon to attempt 
 adding the CSS property as you mention in your previous email.  I 
 considered tabSize as a name as well.  I don’t have a preference if we 
 leave things as representing tabs as a number of spaces.  But it wouldn’t 
 be too difficult to support units, making it mostly compatible with CSS 
 rules.  The way I envision that is having two properties, tabSize and 
 tabUnit.
 
 David mentioned javafx.css.SizeUnits… I looked quickly at the java docs 
 for it, and it’s basically undocumented .  So I went to 
 https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit 
 like ‘ems’ but meaning the width of a space in the current font.  The 
 problem with that is it would leave out the possibility to set the tab 
 width to anything relative to the current implementation of 8 spaces. (In 
 hindsight it should have been implemented based on ‘ems’, which for a 
 fixed width font as typically used in a code editor would be the same as 8 
 spaces anyway)
 Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
 (FX-space - fx prefix to avoid a potential collision with any future 
 official CSS units)
 
 // Two tab-related properties
 public void setTabSize(int size); // defaults to 8
 public void setTabUnits(SizeUnits units); // ?? no unit for width of space 
 in current font
 
 If we did the above, I would consider adding this for convenience:
 public void setTabWidth(int size, TabUnits units);
 
 As far as setting a range, I’m not sure there is any worthwhile benefit to 
 enforcing a maximum.  If we want to store the value in a short instead of 
 an int to potentially save a couple bytes, sure.  Otherwise, if someone 
 wants to set tabs to be 20 spaces or 100, why should we prevent it?  If 
 there isn't a performance or memory impact, I wouldn’t enforce a maximum.
 
 Ignoring any out of range values rather than clamping seems fine to me as 
 well.  I was thinking of what happens if the value was bound to another 
 value that strayed out of range. Clamping would get you as close as 
 possible to the bound value, rather than stuck at the previously observed 
 value.  I just guessed that would be preferred, but if ignoring is more 
 consistent with other values, then I agree it makes sense.  As long as the 
 behaviour is documented, I can’t think of any significant downside either 
 way.
 
 
 Regards,
 
 Scott
 
 
> On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:
> 
> 

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

Hi,

Thanks for that. It also neatly solves the units problem.
I think it means we can just support the same and not go out of our way 
to make provision for pixels.


-phil.

On 10/17/19 11:46 AM, David Grieve wrote:

FWIW, w3c spec for tab-size is 
https://drafts.csswg.org/css-text-3/#propdef-tab-size
In this spec, the value of 'tab-size' is a  (or length -which no browsers 
support). For all intents and purposes,  indicates a number of spaces.


-Original Message-
From: openjfx-dev  On Behalf Of
Phil Race
Sent: Thursday, October 17, 2019 2:25 PM
To: Scott Palmer 
Cc: openjfx-dev@openjdk.java.net Mailing 
Subject: Re: JDK-8130738 TextFlow's tab width is static

Hi,

As I wrote, adding units complicates things.
We would have two linked properties in what you have below and one
modifies the interpretation of the other.

If we expose the number of spaces integer property today, I think we'd have
to add the units as a separate property, but it might be that this is what you'd
want anyway unless right now, today you add some new TabSize class which
includes both and perhaps today would support only "spaces" or "ems" or
something as the only value of the enum of sizes.
FWIW I assumed that CSS is using "ems" as an explicit name for the default if
you don't specify units.

With TabSize there you could later add "pixels".
But I am not sure it is really worth supporting pixels but I raised it because
we should have the discussion up front to know if we have a way to add it
later if it is needed.

If we aren't limiting the value, I see no reason to use short, since any value >
100 or thereabouts pretty much means " line break", or "clip" if there's no
line breaking.

However I still prefer a sensible maximum.

Kevin noted off-line that clamping isn't completely off-base for ranges.

-phil.

On 10/17/19 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt

adding the CSS property as you mention in your previous email.  I considered
tabSize as a name as well.  I don’t have a preference if we leave things as
representing tabs as a number of spaces.  But it wouldn’t be too difficult to
support units, making it mostly compatible with CSS rules.  The way I envision
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java
docs for it, and it’s basically undocumented .  So I went to


https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
.

w3.org%2FTR%2Fcss-values-

3%2F%23lengths&data=02%7C01%7CDavid.Griev
e%40microsoft.com%7Cec03031ecb6c484da76008d7533002ce%7C72f988bf8
6f141a
f91ab2d7cd011db47%7C1%7C0%7C637069337978947508&sdata=li0kW
L1C1hNRA

In0C5ehMIHz6WD%2B%2F3F1xgRH%2Fl9Piv4%3D&reserved=0 and I

see there

is no CSS unit like ‘ems’ but meaning the width of a space in the
current font.  The problem with that is it would leave out the
possibility to set the tab width to anything relative to the current
implementation of 8 spaces. (In hindsight it should have been
implemented based on ‘ems’, which for a fixed width font as typically
used in a code editor would be the same as 8 spaces anyway) Do we add
something to SizeUnits so we can work around this?  e.g. ‘fxsp’
(FX-space - fx prefix to avoid a potential collision with any future
official CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8 public void
setTabUnits(SizeUnits units); // ?? no unit for width of space in
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to

enforcing a maximum.  If we want to store the value in a short instead of an
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as

well.  I was thinking of what happens if the value was bound to another value
that strayed out of range. Clamping would get you as close as possible to the
bound value, rather than stuck at the previously observed value.  I just
guessed that would be preferred, but if ignoring is more consistent with
other values, then I agree it makes sense.  As long as the behaviour is
documented, I can’t think of any significant downside either way.


Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did not

pen ..

First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space chara

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

Really ? It defaults to pixels ? Is that just inherited as being the 
default CSS unit ?

Is that FX's implementation

Hmm. A bit of reading about web CSS says that strictly anything without 
an explicit unit should be ignored.

The only exception is zero, where it doesn't matter.

Eg see : 
https://stackoverflow.com/questions/7907749/default-unit-for-font-size


Although I am sure there are more authoratative  sources than that.

-phil.

On 10/17/19 11:22 AM, Scott Palmer wrote:

So do we go ahead and implement tabSize without the CSS support?   I’m not sure 
the CSS property should be added before unit issues are worked out, as I think 
the units would be expected in the CSS context.  E.g. CSS implicitly adds ‘px’ 
if no unit is specified.  If our tabSize units are spaces, that breaks.

Scott


On Oct 17, 2019, at 2:16 PM, Kevin Rushforth  wrote:

It might make sense to just add the tabSize property now, and later consider adding a 
tabUnits property in the future if needed. By default, having the units be "number 
of spaces in the current font" is what makes the most sense, so before we could add 
tabUnits we would need to extend it as you suggest. I'm not sure it's needed, though, so 
that would be another reason not to do it now.

It's probably best to have the type of tabSize be double rather than int. We do 
this for most attributes to leave the door open for fractional values. I don't 
know why anyone would want a tab that was 5.7 spaces, but if we ever were to 
add a tabUnits property, I could easily see wanting fractional values for some 
units.

-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent with other values, then 
I agree it makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.


Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did not pen ..

First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
 if (spaces < 0)
 spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range value.
This is practiced elsewhere in FX where illegal values are ignored rather than
throwing an exception, although it might be that clamping is quite common
in range cases.

The follow on to that is that what is a sensible maxi

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

On 10/17/19 11:16 AM, Kevin Rushforth wrote:
It might make sense to just add the tabSize property now, and later 
consider adding a tabUnits property in the future if needed. By 
default, having the units be "number of spaces in the current font" is 
what makes the most sense, so before we could add tabUnits we would 
need to extend it as you suggest. I'm not sure it's needed, though, so 
that would be another reason not to do it now.


It's probably best to have the type of tabSize be double rather than 
int. We do this for most attributes to leave the door open for 
fractional values. I don't know why anyone would want a tab that was 
5.7 spaces, but if we ever were to add a tabUnits property, I could 
easily see wanting fractional values for some units.


In a fixed width font that would seem bizarre.
And I think fixed width fonts are the main case when you *want* tabs.

This is another reason why I don't think we should do pixels ...

-phil.




-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to 
attempt adding the CSS property as you mention in your previous 
email.  I considered tabSize as a name as well.  I don’t have a 
preference if we leave things as representing tabs as a number of 
spaces.  But it wouldn’t be too difficult to support units, making it 
mostly compatible with CSS rules.  The way I envision that is having 
two properties, tabSize and tabUnit.


David mentioned javafx.css.SizeUnits… I looked quickly at the java 
docs for it, and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS 
unit like ‘ems’ but meaning the width of a space in the current 
font.  The problem with that is it would leave out the possibility to 
set the tab width to anything relative to the current implementation 
of 8 spaces. (In hindsight it should have been implemented based on 
‘ems’, which for a fixed width font as typically used in a code 
editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this? e.g. 
‘fxsp’  (FX-space - fx prefix to avoid a potential collision with any 
future official CSS units)


// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of 
space in current font


If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile 
benefit to enforcing a maximum.  If we want to store the value in a 
short instead of an int to potentially save a couple bytes, sure.  
Otherwise, if someone wants to set tabs to be 20 spaces or 100, why 
should we prevent it?  If there isn't a performance or memory impact, 
I wouldn’t enforce a maximum.


Ignoring any out of range values rather than clamping seems fine to 
me as well.  I was thinking of what happens if the value was bound to 
another value that strayed out of range. Clamping would get you as 
close as possible to the bound value, rather than stuck at the 
previously observed value.  I just guessed that would be preferred, 
but if ignoring is more consistent with other values, then I agree it 
makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.



Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did 
not pen ..


First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
 if (spaces < 0)
 spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range 
value.
This is practiced elsewhere in FX where illegal values are ignored 
rather than
throwing an exception, although it might be that clamping is quite 
common

in range cases.

The follow on to that is that what is a sensible maximum value.
Currently we have int which is more than we need. For that matter so 
is short.

I think we should have a documented and enforced sensible maximum.
Perhaps it could be "8" meaning we only support reducing tab width as I
don't know if anyone really wants to increase it.
CSS doesn't have a limit that I can see but if we are already only 
supporting
it described as number of spaces, we can have this other limitation 
too.

If you think 8 is too small, then maybe 16 ?
Also remember we can compatibly relax it later but we can't 
compatibly tighten it.



-phil.

On 10/15/19 12:17 PM, Phil

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

Hi,

As I wrote, adding units complicates things.
We would have two linked properties in what you have below and one 
modifies the

interpretation of the other.

If we expose the number of spaces integer property today, I think we'd 
have to add the units as a
separate property, but it might be that this is what you'd want anyway 
unless right now, today
you add some new TabSize class which includes both and perhaps today 
would support

only "spaces" or "ems" or something as the only value of the enum of sizes.
FWIW I assumed that CSS is using "ems" as an explicit name for the 
default if you don't

specify units.

With TabSize there you could later add "pixels".
But I am not sure it is really worth supporting pixels but I raised it 
because
we should have the discussion up front to know if we have a way to add 
it later if it is needed.


If we aren't limiting the value, I see no reason to use short, since any 
value > 100 or thereabouts

pretty much means " line break", or "clip" if there's no line breaking.

However I still prefer a sensible maximum.

Kevin noted off-line that clamping isn't completely off-base for ranges.

-phil.

On 10/17/19 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent with other values, then 
I agree it makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.


Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did not pen ..

First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
 if (spaces < 0)
 spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range value.
This is practiced elsewhere in FX where illegal values are ignored rather than
throwing an exception, although it might be that clamping is quite common
in range cases.

The follow on to that is that what is a sensible maximum value.
Currently we have int which is more than we need. For that matter so is short.
I think we should have a documented and enforced sensible maximum.
Perhaps it could be "8" meaning we only support reducing tab width as I
don't know if anyone really wants to increase it.
CSS doesn't have a limit that I can see but if we are already only supporting
it described as number of spaces, we can have this other limitation too.
If you thi

Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

That's a good question. It feels a bit incomplete without CSS support, 
but might be good enough for most uses. I'd be interested in what other 
developers think.


-- Kevin


On 10/17/2019 11:22 AM, Scott Palmer wrote:

So do we go ahead and implement tabSize without the CSS support?   I’m not sure 
the CSS property should be added before unit issues are worked out, as I think 
the units would be expected in the CSS context.  E.g. CSS implicitly adds ‘px’ 
if no unit is specified.  If our tabSize units are spaces, that breaks.

Scott


On Oct 17, 2019, at 2:16 PM, Kevin Rushforth  wrote:

It might make sense to just add the tabSize property now, and later consider adding a 
tabUnits property in the future if needed. By default, having the units be "number 
of spaces in the current font" is what makes the most sense, so before we could add 
tabUnits we would need to extend it as you suggest. I'm not sure it's needed, though, so 
that would be another reason not to do it now.

It's probably best to have the type of tabSize be double rather than int. We do 
this for most attributes to leave the door open for fractional values. I don't 
know why anyone would want a tab that was 5.7 spaces, but if we ever were to 
add a tabUnits property, I could easily see wanting fractional values for some 
units.

-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent with other values, then 
I agree it makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.


Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did not pen ..

First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
 if (spaces < 0)
 spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range value.
This is practiced elsewhere in FX where illegal values are ignored rather than
throwing an exception, although it might be that clamping is quite common
in range cases.

The follow on to that is that what is a sensible maximum value.
Currently we have int which is more than we need. For that matter so is short.
I think we should have a documented and enforced sensible maximum.
Perhaps it could be "8" meaning we only support reducing tab width as I
don't know if anyone really wants to

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

So do we go ahead and implement tabSize without the CSS support?   I’m not sure 
the CSS property should be added before unit issues are worked out, as I think 
the units would be expected in the CSS context.  E.g. CSS implicitly adds ‘px’ 
if no unit is specified.  If our tabSize units are spaces, that breaks.

Scott

> On Oct 17, 2019, at 2:16 PM, Kevin Rushforth  
> wrote:
> 
> It might make sense to just add the tabSize property now, and later consider 
> adding a tabUnits property in the future if needed. By default, having the 
> units be "number of spaces in the current font" is what makes the most sense, 
> so before we could add tabUnits we would need to extend it as you suggest. 
> I'm not sure it's needed, though, so that would be another reason not to do 
> it now.
> 
> It's probably best to have the type of tabSize be double rather than int. We 
> do this for most attributes to leave the door open for fractional values. I 
> don't know why anyone would want a tab that was 5.7 spaces, but if we ever 
> were to add a tabUnits property, I could easily see wanting fractional values 
> for some units.
> 
> -- Kevin
> 
> On 10/17/2019 10:40 AM, Scott Palmer wrote:
>> Hi Phil,
>> 
>> Thanks for taking a look.  I was going to get back to this soon to attempt 
>> adding the CSS property as you mention in your previous email.  I considered 
>> tabSize as a name as well.  I don’t have a preference if we leave things as 
>> representing tabs as a number of spaces.  But it wouldn’t be too difficult 
>> to support units, making it mostly compatible with CSS rules.  The way I 
>> envision that is having two properties, tabSize and tabUnit.
>> 
>> David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for 
>> it, and it’s basically undocumented .  So I went to 
>> https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit 
>> like ‘ems’ but meaning the width of a space in the current font.  The 
>> problem with that is it would leave out the possibility to set the tab width 
>> to anything relative to the current implementation of 8 spaces. (In 
>> hindsight it should have been implemented based on ‘ems’, which for a fixed 
>> width font as typically used in a code editor would be the same as 8 spaces 
>> anyway)
>> Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
>> (FX-space - fx prefix to avoid a potential collision with any future 
>> official CSS units)
>> 
>> // Two tab-related properties
>> public void setTabSize(int size); // defaults to 8
>> public void setTabUnits(SizeUnits units); // ?? no unit for width of space 
>> in current font
>> 
>> If we did the above, I would consider adding this for convenience:
>> public void setTabWidth(int size, TabUnits units);
>> 
>> As far as setting a range, I’m not sure there is any worthwhile benefit to 
>> enforcing a maximum.  If we want to store the value in a short instead of an 
>> int to potentially save a couple bytes, sure.  Otherwise, if someone wants 
>> to set tabs to be 20 spaces or 100, why should we prevent it?  If there 
>> isn't a performance or memory impact, I wouldn’t enforce a maximum.
>> 
>> Ignoring any out of range values rather than clamping seems fine to me as 
>> well.  I was thinking of what happens if the value was bound to another 
>> value that strayed out of range. Clamping would get you as close as possible 
>> to the bound value, rather than stuck at the previously observed value.  I 
>> just guessed that would be preferred, but if ignoring is more consistent 
>> with other values, then I agree it makes sense.  As long as the behaviour is 
>> documented, I can’t think of any significant downside either way.
>> 
>> 
>> Regards,
>> 
>> Scott
>> 
>> 
>>> On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:
>>> 
>>> Hi,
>>> 
>>> Some more points which I thought about but for whatever reason did not pen 
>>> ..
>>> 
>>> First one minor thing: tabWidth is an OK name, but it does not
>>> conjure up that the value you specify isn't a width in pixels but the
>>> number of discrete space characters to use. FWIW CSS calls it tab-size.
>>> But CSS then supports pixels and ems .. that complicates matters a lot.
>>> 
>>> https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size
>>> 
>>> Thee rest is mostly to do with "legal or sensible values"
>>> 
>>> You have :
>>> if (spaces < 0)
>>> spaces = 0;
>>> 
>>> since negative values are not something we want to support!
>>> But I think instead of clamping you could "ignore", any out of range value.
>>> This is practiced elsewhere in FX where illegal values are ignored rather 
>>> than
>>> throwing an exception, although it might be that clamping is quite common
>>> in range cases.
>>> 
>>> The follow on to that is that what is a sensible maximum value.
>>> Currently we have int which is more than we need. For that matter so is 
>>> short.
>>> I think we should have a documented and enforced sensible maximum.
>>> Perhaps it c

Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

It might make sense to just add the tabSize property now, and later 
consider adding a tabUnits property in the future if needed. By default, 
having the units be "number of spaces in the current font" is what makes 
the most sense, so before we could add tabUnits we would need to extend 
it as you suggest. I'm not sure it's needed, though, so that would be 
another reason not to do it now.


It's probably best to have the type of tabSize be double rather than 
int. We do this for most attributes to leave the door open for 
fractional values. I don't know why anyone would want a tab that was 5.7 
spaces, but if we ever were to add a tabUnits property, I could easily 
see wanting fractional values for some units.


-- Kevin

On 10/17/2019 10:40 AM, Scott Palmer wrote:

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units);

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent with other values, then 
I agree it makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.


Regards,

Scott



On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:

Hi,

Some more points which I thought about but for whatever reason did not pen ..

First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
 if (spaces < 0)
 spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range value.
This is practiced elsewhere in FX where illegal values are ignored rather than
throwing an exception, although it might be that clamping is quite common
in range cases.

The follow on to that is that what is a sensible maximum value.
Currently we have int which is more than we need. For that matter so is short.
I think we should have a documented and enforced sensible maximum.
Perhaps it could be "8" meaning we only support reducing tab width as I
don't know if anyone really wants to increase it.
CSS doesn't have a limit that I can see but if we are already only supporting
it described as number of spaces, we can have this other limitation too.
If you think 8 is too small, then maybe 16 ?
Also remember we can compatibly relax it later but we can't compatibly tighten 
it.


-phil.

On 10/15/19 12:17 PM, Phil Race wrote:

Hi,

I've looked over this and I don't see any issues - meaning gotchas.
It just provides a way to over-ride the hard-coded 8,
whether using a Text node directly or a TextFlow.

Two observations of what one might call limitations
1) This is a renderi

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

Hi Phil,

Thanks for taking a look.  I was going to get back to this soon to attempt 
adding the CSS property as you mention in your previous email.  I considered 
tabSize as a name as well.  I don’t have a preference if we leave things as 
representing tabs as a number of spaces.  But it wouldn’t be too difficult to 
support units, making it mostly compatible with CSS rules.  The way I envision 
that is having two properties, tabSize and tabUnit.

David mentioned javafx.css.SizeUnits… I looked quickly at the java docs for it, 
and it’s basically undocumented .  So I went to 
https://www.w3.org/TR/css-values-3/#lengths and I see there is no CSS unit like 
‘ems’ but meaning the width of a space in the current font.  The problem with 
that is it would leave out the possibility to set the tab width to anything 
relative to the current implementation of 8 spaces. (In hindsight it should 
have been implemented based on ‘ems’, which for a fixed width font as typically 
used in a code editor would be the same as 8 spaces anyway)
Do we add something to SizeUnits so we can work around this?  e.g. ‘fxsp’  
(FX-space - fx prefix to avoid a potential collision with any future official 
CSS units)

// Two tab-related properties
public void setTabSize(int size); // defaults to 8
public void setTabUnits(SizeUnits units); // ?? no unit for width of space in 
current font

If we did the above, I would consider adding this for convenience:
public void setTabWidth(int size, TabUnits units); 

As far as setting a range, I’m not sure there is any worthwhile benefit to 
enforcing a maximum.  If we want to store the value in a short instead of an 
int to potentially save a couple bytes, sure.  Otherwise, if someone wants to 
set tabs to be 20 spaces or 100, why should we prevent it?  If there isn't a 
performance or memory impact, I wouldn’t enforce a maximum.

Ignoring any out of range values rather than clamping seems fine to me as well. 
 I was thinking of what happens if the value was bound to another value that 
strayed out of range. Clamping would get you as close as possible to the bound 
value, rather than stuck at the previously observed value.  I just guessed that 
would be preferred, but if ignoring is more consistent with other values, then 
I agree it makes sense.  As long as the behaviour is documented, I can’t think 
of any significant downside either way.


Regards,

Scott


> On Oct 17, 2019, at 12:45 PM, Phil Race  wrote:
> 
> Hi,
> 
> Some more points which I thought about but for whatever reason did not pen ..
> 
> First one minor thing: tabWidth is an OK name, but it does not
> conjure up that the value you specify isn't a width in pixels but the
> number of discrete space characters to use. FWIW CSS calls it tab-size.
> But CSS then supports pixels and ems .. that complicates matters a lot.
> 
> https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size
> 
> Thee rest is mostly to do with "legal or sensible values"
> 
> You have :
> if (spaces < 0)
> spaces = 0;
> 
> since negative values are not something we want to support!
> But I think instead of clamping you could "ignore", any out of range value.
> This is practiced elsewhere in FX where illegal values are ignored rather than
> throwing an exception, although it might be that clamping is quite common
> in range cases.
> 
> The follow on to that is that what is a sensible maximum value.
> Currently we have int which is more than we need. For that matter so is short.
> I think we should have a documented and enforced sensible maximum.
> Perhaps it could be "8" meaning we only support reducing tab width as I
> don't know if anyone really wants to increase it.
> CSS doesn't have a limit that I can see but if we are already only supporting
> it described as number of spaces, we can have this other limitation too.
> If you think 8 is too small, then maybe 16 ?
> Also remember we can compatibly relax it later but we can't compatibly 
> tighten it.
> 
> 
> -phil.
> 
> On 10/15/19 12:17 PM, Phil Race wrote:
>> Hi,
>> 
>> I've looked over this and I don't see any issues - meaning gotchas.
>> It just provides a way to over-ride the hard-coded 8,
>> whether using a Text node directly or a TextFlow.
>> 
>> Two observations of what one might call limitations
>> 1) This is a rendering time property, which is controlled by the program.
>> A document containing tabs saved and re-rendered somewhere else
>> won't be helped.
>> 
>> 2) This just provides API on the scene graph node types, not any of the UI 
>> controls
>> which use Text nodes. Something like a CSS property may be the way to go if
>> you wanted that.
>> Text has a nested class StyleableProperties for CSS properties with which it
>> plays nice : font, underline, strikethrough, text-alignment
>> 
>> So creating an fx-tabWidth (or similar name) CSS property could be propagated
>> through to there in a similar way.
>> 
>> -phil.
>> 
>> 
>> On 9/30/19 9:48 AM, Scott Palmer wrote:
>>> Okay, cu

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

Hi,

Some more points which I thought about but for whatever reason did not 
pen ..


First one minor thing: tabWidth is an OK name, but it does not
conjure up that the value you specify isn't a width in pixels but the
number of discrete space characters to use. FWIW CSS calls it tab-size.
But CSS then supports pixels and ems .. that complicates matters a lot.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size

Thee rest is mostly to do with "legal or sensible values"

You have :
    if (spaces < 0)
    spaces = 0;

since negative values are not something we want to support!
But I think instead of clamping you could "ignore", any out of range value.
This is practiced elsewhere in FX where illegal values are ignored 
rather than

throwing an exception, although it might be that clamping is quite common
in range cases.

The follow on to that is that what is a sensible maximum value.
Currently we have int which is more than we need. For that matter so is 
short.

I think we should have a documented and enforced sensible maximum.
Perhaps it could be "8" meaning we only support reducing tab width as I
don't know if anyone really wants to increase it.
CSS doesn't have a limit that I can see but if we are already only 
supporting

it described as number of spaces, we can have this other limitation too.
If you think 8 is too small, then maybe 16 ?
Also remember we can compatibly relax it later but we can't compatibly 
tighten it.



-phil.

On 10/15/19 12:17 PM, Phil Race wrote:

Hi,

I've looked over this and I don't see any issues - meaning gotchas.
It just provides a way to over-ride the hard-coded 8,
whether using a Text node directly or a TextFlow.

Two observations of what one might call limitations
1) This is a rendering time property, which is controlled by the program.
A document containing tabs saved and re-rendered somewhere else
won't be helped.

2) This just provides API on the scene graph node types, not any of 
the UI controls
which use Text nodes. Something like a CSS property may be the way to 
go if

you wanted that.
Text has a nested class StyleableProperties for CSS properties with 
which it

plays nice : font, underline, strikethrough, text-alignment

So creating an fx-tabWidth (or similar name) CSS property could be 
propagated

through to there in a similar way.

-phil.


On 9/30/19 9:48 AM, Scott Palmer wrote:

Okay, current work relocated to a clone of the new official Git repo.
My initial implementation is here:
https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f 
 



I would submit it as a pull request but that seems premature since 
there hasn’t been any real discussion of challenges I’ve overlooked.  
All I have are the famous last words, “It works for me.”


I saw in the archives a concern about tab width vs tab stops. For 
some reason I didn’t get the email.  Anyway, I don’t think arbitrary 
tab stop positions, at different intervals that is, are what was 
asked for.  That certainly would require a significantly different 
implementation.


Would love to keep some momentum going on this before I become busy 
with other things and won’t have time for it.


Scott


On Sep 23, 2019, at 3:57 PM, Scott Palmer  wrote:

My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1 
 



While writing a unit test I realized that the StubToolkit isn’t 
really running the Prism layout code, so I’m not sure how that gets 
tested.  I made it work with StubTextLayout for now.


Regards,

Scott


On Sep 20, 2019, at 12:57 PM, Scott Palmer > wrote:


Thanks Kevin.

My current implementation appears to be working for TextFlow and 
Text, with the TextFlow overriding the tabWidth of the child Text 
nodes.  This seems to work out naturally from the way TextFlow 
overrides the TextLayout instance used by the Text node.


If there are tricky corner-cases that I’m missing, I guess figuring 
out all the cases it will need to handle is part of this 
discussion.  It didn’t seem to be that challenging so far, so 
perhaps I am missing something (hopefully not).  I wrote a small 
test app to visually see that things were working as I expected.  I 
have not yet written the unit tests.


Cheers,

Scott

On Sep 20, 2019, at 10:58 AM, Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> 
wrote:


Hi Scott,

I'm sure Phil will have more comments on this. While the API seems 
simple enough on the surface, I suspect that this will be a 
challenging feature to implement correctly for all of the cases it 
will need to handle. It would need careful review and testing as 
well. My only comment on the API itself is that if we do accept 
this feature, it should probably go on both Text and TextFlow, and 
be one of the attribu

Re: JDK-8130738 TextFlow's tab width is static

[Phil Race]

Hi,

I've looked over this and I don't see any issues - meaning gotchas.
It just provides a way to over-ride the hard-coded 8,
whether using a Text node directly or a TextFlow.

Two observations of what one might call limitations
1) This is a rendering time property, which is controlled by the program.
A document containing tabs saved and re-rendered somewhere else
won't be helped.

2) This just provides API on the scene graph node types, not any of the 
UI controls

which use Text nodes. Something like a CSS property may be the way to go if
you wanted that.
Text has a nested class StyleableProperties for CSS properties with which it
plays nice : font, underline, strikethrough, text-alignment

So creating an fx-tabWidth (or similar name) CSS property could be 
propagated

through to there in a similar way.

-phil.


On 9/30/19 9:48 AM, Scott Palmer wrote:

Okay, current work relocated to a clone of the new official Git repo.
My initial implementation is here:
  https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f 


I would submit it as a pull request but that seems premature since there hasn’t 
been any real discussion of challenges I’ve overlooked.  All I have are the 
famous last words, “It works for me.”

I saw in the archives a concern about tab width vs tab stops. For some reason I 
didn’t get the email.  Anyway, I don’t think arbitrary tab stop positions, at 
different intervals that is, are what was asked for.  That certainly would 
require a significantly different implementation.

Would love to keep some momentum going on this before I become busy with other 
things and won’t have time for it.

Scott


On Sep 23, 2019, at 3:57 PM, Scott Palmer  wrote:

My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1
 


While writing a unit test I realized that the StubToolkit isn’t really running 
the Prism layout code, so I’m not sure how that gets tested.  I made it work 
with StubTextLayout for now.

Regards,

Scott



On Sep 20, 2019, at 12:57 PM, Scott Palmer mailto:swpal...@gmail.com>> wrote:

Thanks Kevin.

My current implementation appears to be working for TextFlow and Text, with the 
TextFlow overriding the tabWidth of the child Text nodes.  This seems to work 
out naturally from the way TextFlow overrides the TextLayout instance used by 
the Text node.

If there are tricky corner-cases that I’m missing, I guess figuring out all the 
cases it will need to handle is part of this discussion.  It didn’t seem to be 
that challenging so far, so perhaps I am missing something (hopefully not).  I 
wrote a small test app to visually see that things were working as I expected.  
I have not yet written the unit tests.

Cheers,

Scott


On Sep 20, 2019, at 10:58 AM, Kevin Rushforth mailto:kevin.rushfo...@oracle.com>> wrote:

Hi Scott,

I'm sure Phil will have more comments on this. While the API seems simple 
enough on the surface, I suspect that this will be a challenging feature to 
implement correctly for all of the cases it will need to handle. It would need 
careful review and testing as well. My only comment on the API itself is that 
if we do accept this feature, it should probably go on both Text and TextFlow, 
and be one of the attributes of Text that is ignored / overridden when a Text 
node is in a TextFlow.

-- Kevin


On 9/18/2019 6:14 PM, Scott Palmer wrote:

I would like to implement this feature, being able to adjust the tab size in a TextFlow or 
Text node (JDK-8130738 >).  It involves new public API, 
so I want to start a discussion about it here.  (My motivation is that RichTextFX suggests 
an entirely unacceptable workaround of substituting actual spaces when the tab character is 
typed and cites the lack of this API.)

I’ve already jumped the gun and taken a crack at an implementation.  It is 
currently incomplete as I was just poking around to see if it was going to be 
easy enough to not take up too much of my time.  It boils down to:
TextFlow and Text get a new property for tab width, an integer representing the 
number of spaces taken by a tab. (The value is only used to initialize the tab 
width for the TextLayout when needed.)
TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
PrismTextLayout implements the new setTabWidth API.

I’m not sure that the Text node needs this new property.  I figured it would be 
rarely used on that class, so I had implemented it via an added property in the 
private TextAttributes class.  Maybe it isn’t needed at all.

What’s the next step?

Regards,

Scott

Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

Hi Scott,

Thanks for the update. I haven't had much time lately between the Skara 
switch and trying to keep up with jpackage changes...


The next step is, as you note, to continue discussion any issues around 
the API, including whether and how it might interact with a rich text 
editor that wanted to have a list of variable tab stops (like many word 
processors use). On this point, I wouldn't imagine wanting to implement 
variable tab stops in TextFlow, but I would want to think through how a 
feature layered on top of TextFlow would override the fixed tab width 
that TextFlow has; since we have this problem today, I don't think this 
is making it worse, but some discussion on that point would be good.


Once there is general buy-in on adding this feature, and on the 
approach, it would be good to review the API, although that's quite simple.


Finally, I would want Phil's input before approving the API.

-- Kevin


On 9/30/2019 9:48 AM, Scott Palmer wrote:

Okay, current work relocated to a clone of the new official Git repo.
My initial implementation is here:
https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f 



I would submit it as a pull request but that seems premature since 
there hasn’t been any real discussion of challenges I’ve overlooked. 
 All I have are the famous last words, “It works for me.”


I saw in the archives a concern about tab width vs tab stops. For some 
reason I didn’t get the email.  Anyway, I don’t think arbitrary tab 
stop positions, at different intervals that is, are what was asked 
for.  That certainly would require a significantly different 
implementation.


Would love to keep some momentum going on this before I become busy 
with other things and won’t have time for it.


Scott

On Sep 23, 2019, at 3:57 PM, Scott Palmer > wrote:


My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1 



While writing a unit test I realized that the StubToolkit isn’t 
really running the Prism layout code, so I’m not sure how that gets 
tested.  I made it work with StubTextLayout for now.


Regards,

Scott


On Sep 20, 2019, at 12:57 PM, Scott Palmer > wrote:


Thanks Kevin.

My current implementation appears to be working for TextFlow and 
Text, with the TextFlow overriding the tabWidth of the child Text 
nodes.  This seems to work out naturally from the way TextFlow 
overrides the TextLayout instance used by the Text node.


If there are tricky corner-cases that I’m missing, I guess figuring 
out all the cases it will need to handle is part of this discussion. 
 It didn’t seem to be that challenging so far, so perhaps I am 
missing something (hopefully not).  I wrote a small test app to 
visually see that things were working as I expected.  I have not yet 
written the unit tests.


Cheers,

Scott

On Sep 20, 2019, at 10:58 AM, Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:


Hi Scott,

I'm sure Phil will have more comments on this. While the API seems 
simple enough on the surface, I suspect that this will be a 
challenging feature to implement correctly for all of the cases it 
will need to handle. It would need careful review and testing as 
well. My only comment on the API itself is that if we do accept 
this feature, it should probably go on both Text and TextFlow, and 
be one of the attributes of Text that is ignored / overridden when 
a Text node is in a TextFlow.


-- Kevin


On 9/18/2019 6:14 PM, Scott Palmer wrote:
I would like to implement this feature, being able to adjust the 
tab size in a TextFlow or Text node (JDK-8130738 
).  It involves 
new public API, so I want to start a discussion about it here. 
 (My motivation is that RichTextFX suggests an entirely 
unacceptable workaround of substituting actual spaces when the tab 
character is typed and cites the lack of this API.)


I’ve already jumped the gun and taken a crack at an 
implementation.  It is currently incomplete as I was just poking 
around to see if it was going to be easy enough to not take up too 
much of my time.  It boils down to:
TextFlow and Text get a new property for tab width, an integer 
representing the number of spaces taken by a tab. (The value is 
only used to initialize the tab width for the TextLayout when needed.)
TextLayout interface gets a new method:  boolean setTabWidth(int 
spaces)

TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
PrismTextLayout implements the new setTabWidth API.

I’m not sure that the Text node needs this new property.  I 
figured it would be rarely used on that class, so I had 
implemented it via an added property in the private TextAttributes 
class.  Maybe it isn’t needed at all.


What’s the next step?

Regards,

Scott

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

Okay, current work relocated to a clone of the new official Git repo.
My initial implementation is here:
 
https://github.com/swpalmer/jfx/commit/cc6193451bf8a693093f3ded5dcbe47af2fcbe8f 


I would submit it as a pull request but that seems premature since there hasn’t 
been any real discussion of challenges I’ve overlooked.  All I have are the 
famous last words, “It works for me.”

I saw in the archives a concern about tab width vs tab stops. For some reason I 
didn’t get the email.  Anyway, I don’t think arbitrary tab stop positions, at 
different intervals that is, are what was asked for.  That certainly would 
require a significantly different implementation.

Would love to keep some momentum going on this before I become busy with other 
things and won’t have time for it.

Scott

> On Sep 23, 2019, at 3:57 PM, Scott Palmer  wrote:
> 
> My current work is here 
> https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1
>  
> 
> 
> While writing a unit test I realized that the StubToolkit isn’t really 
> running the Prism layout code, so I’m not sure how that gets tested.  I made 
> it work with StubTextLayout for now.
> 
> Regards,
> 
> Scott
> 
> 
>> On Sep 20, 2019, at 12:57 PM, Scott Palmer > > wrote:
>> 
>> Thanks Kevin.
>> 
>> My current implementation appears to be working for TextFlow and Text, with 
>> the TextFlow overriding the tabWidth of the child Text nodes.  This seems to 
>> work out naturally from the way TextFlow overrides the TextLayout instance 
>> used by the Text node.
>> 
>> If there are tricky corner-cases that I’m missing, I guess figuring out all 
>> the cases it will need to handle is part of this discussion.  It didn’t seem 
>> to be that challenging so far, so perhaps I am missing something (hopefully 
>> not).  I wrote a small test app to visually see that things were working as 
>> I expected.  I have not yet written the unit tests.
>> 
>> Cheers,
>> 
>> Scott
>> 
>>> On Sep 20, 2019, at 10:58 AM, Kevin Rushforth >> > wrote:
>>> 
>>> Hi Scott,
>>> 
>>> I'm sure Phil will have more comments on this. While the API seems simple 
>>> enough on the surface, I suspect that this will be a challenging feature to 
>>> implement correctly for all of the cases it will need to handle. It would 
>>> need careful review and testing as well. My only comment on the API itself 
>>> is that if we do accept this feature, it should probably go on both Text 
>>> and TextFlow, and be one of the attributes of Text that is ignored / 
>>> overridden when a Text node is in a TextFlow.
>>> 
>>> -- Kevin
>>> 
>>> 
>>> On 9/18/2019 6:14 PM, Scott Palmer wrote:
 I would like to implement this feature, being able to adjust the tab size 
 in a TextFlow or Text node (JDK-8130738 
 >).  It involves new 
 public API, so I want to start a discussion about it here.  (My motivation 
 is that RichTextFX suggests an entirely unacceptable workaround of 
 substituting actual spaces when the tab character is typed and cites the 
 lack of this API.)
 
 I’ve already jumped the gun and taken a crack at an implementation.  It is 
 currently incomplete as I was just poking around to see if it was going to 
 be easy enough to not take up too much of my time.  It boils down to:
 TextFlow and Text get a new property for tab width, an integer 
 representing the number of spaces taken by a tab. (The value is only used 
 to initialize the tab width for the TextLayout when needed.)
 TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
 TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
 PrismTextLayout implements the new setTabWidth API.
 
 I’m not sure that the Text node needs this new property.  I figured it 
 would be rarely used on that class, so I had implemented it via an added 
 property in the private TextAttributes class.  Maybe it isn’t needed at 
 all.
 
 What’s the next step?
 
 Regards,
 
 Scott
>>> 
>> 
> 

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1
 


While writing a unit test I realized that the StubToolkit isn’t really running 
the Prism layout code, so I’m not sure how that gets tested.  I made it work 
with StubTextLayout for now.

Regards,

Scott


> On Sep 20, 2019, at 12:57 PM, Scott Palmer  wrote:
> 
> Thanks Kevin.
> 
> My current implementation appears to be working for TextFlow and Text, with 
> the TextFlow overriding the tabWidth of the child Text nodes.  This seems to 
> work out naturally from the way TextFlow overrides the TextLayout instance 
> used by the Text node.
> 
> If there are tricky corner-cases that I’m missing, I guess figuring out all 
> the cases it will need to handle is part of this discussion.  It didn’t seem 
> to be that challenging so far, so perhaps I am missing something (hopefully 
> not).  I wrote a small test app to visually see that things were working as I 
> expected.  I have not yet written the unit tests.
> 
> Cheers,
> 
> Scott
> 
>> On Sep 20, 2019, at 10:58 AM, Kevin Rushforth  
>> wrote:
>> 
>> Hi Scott,
>> 
>> I'm sure Phil will have more comments on this. While the API seems simple 
>> enough on the surface, I suspect that this will be a challenging feature to 
>> implement correctly for all of the cases it will need to handle. It would 
>> need careful review and testing as well. My only comment on the API itself 
>> is that if we do accept this feature, it should probably go on both Text and 
>> TextFlow, and be one of the attributes of Text that is ignored / overridden 
>> when a Text node is in a TextFlow.
>> 
>> -- Kevin
>> 
>> 
>> On 9/18/2019 6:14 PM, Scott Palmer wrote:
>>> I would like to implement this feature, being able to adjust the tab size 
>>> in a TextFlow or Text node (JDK-8130738 
>>> ).  It involves new 
>>> public API, so I want to start a discussion about it here.  (My motivation 
>>> is that RichTextFX suggests an entirely unacceptable workaround of 
>>> substituting actual spaces when the tab character is typed and cites the 
>>> lack of this API.)
>>> 
>>> I’ve already jumped the gun and taken a crack at an implementation.  It is 
>>> currently incomplete as I was just poking around to see if it was going to 
>>> be easy enough to not take up too much of my time.  It boils down to:
>>> TextFlow and Text get a new property for tab width, an integer representing 
>>> the number of spaces taken by a tab. (The value is only used to initialize 
>>> the tab width for the TextLayout when needed.)
>>> TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
>>> TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
>>> PrismTextLayout implements the new setTabWidth API.
>>> 
>>> I’m not sure that the Text node needs this new property.  I figured it 
>>> would be rarely used on that class, so I had implemented it via an added 
>>> property in the private TextAttributes class.  Maybe it isn’t needed at all.
>>> 
>>> What’s the next step?
>>> 
>>> Regards,
>>> 
>>> Scott
>> 
> 

Re: JDK-8130738 TextFlow's tab width is static

[Tom Schindl]

Hi,

Well I'm not sure this API is correct on TextFlow, if it is supposed to
render complex texts like (MS Word) it rather needs a Tab-Stop API, not?

Have we investigated other text-layout controls from other frameworks
like Swing, Qt, WPF, ... ? What API do those expose?

Tom

On 20.09.19 18:57, Scott Palmer wrote:
> Thanks Kevin.
> 
> My current implementation appears to be working for TextFlow and Text, with 
> the TextFlow overriding the tabWidth of the child Text nodes.  This seems to 
> work out naturally from the way TextFlow overrides the TextLayout instance 
> used by the Text node.
> 
> If there are tricky corner-cases that I’m missing, I guess figuring out all 
> the cases it will need to handle is part of this discussion.  It didn’t seem 
> to be that challenging so far, so perhaps I am missing something (hopefully 
> not).  I wrote a small test app to visually see that things were working as I 
> expected.  I have not yet written the unit tests.
> 
> Cheers,
> 
> Scott
> 
>> On Sep 20, 2019, at 10:58 AM, Kevin Rushforth  
>> wrote:
>>
>> Hi Scott,
>>
>> I'm sure Phil will have more comments on this. While the API seems simple 
>> enough on the surface, I suspect that this will be a challenging feature to 
>> implement correctly for all of the cases it will need to handle. It would 
>> need careful review and testing as well. My only comment on the API itself 
>> is that if we do accept this feature, it should probably go on both Text and 
>> TextFlow, and be one of the attributes of Text that is ignored / overridden 
>> when a Text node is in a TextFlow.
>>
>> -- Kevin
>>
>>
>> On 9/18/2019 6:14 PM, Scott Palmer wrote:
>>> I would like to implement this feature, being able to adjust the tab size 
>>> in a TextFlow or Text node (JDK-8130738 
>>> ).  It involves new 
>>> public API, so I want to start a discussion about it here.  (My motivation 
>>> is that RichTextFX suggests an entirely unacceptable workaround of 
>>> substituting actual spaces when the tab character is typed and cites the 
>>> lack of this API.)
>>>
>>> I’ve already jumped the gun and taken a crack at an implementation.  It is 
>>> currently incomplete as I was just poking around to see if it was going to 
>>> be easy enough to not take up too much of my time.  It boils down to:
>>> TextFlow and Text get a new property for tab width, an integer representing 
>>> the number of spaces taken by a tab. (The value is only used to initialize 
>>> the tab width for the TextLayout when needed.)
>>> TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
>>> TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
>>> PrismTextLayout implements the new setTabWidth API.
>>>
>>> I’m not sure that the Text node needs this new property.  I figured it 
>>> would be rarely used on that class, so I had implemented it via an added 
>>> property in the private TextAttributes class.  Maybe it isn’t needed at all.
>>>
>>> What’s the next step?
>>>
>>> Regards,
>>>
>>> Scott
>>
> 

-- 
Tom Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7. A-6020 Innsbruck
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck

Re: JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

Thanks Kevin.

My current implementation appears to be working for TextFlow and Text, with the 
TextFlow overriding the tabWidth of the child Text nodes.  This seems to work 
out naturally from the way TextFlow overrides the TextLayout instance used by 
the Text node.

If there are tricky corner-cases that I’m missing, I guess figuring out all the 
cases it will need to handle is part of this discussion.  It didn’t seem to be 
that challenging so far, so perhaps I am missing something (hopefully not).  I 
wrote a small test app to visually see that things were working as I expected.  
I have not yet written the unit tests.

Cheers,

Scott

> On Sep 20, 2019, at 10:58 AM, Kevin Rushforth  
> wrote:
> 
> Hi Scott,
> 
> I'm sure Phil will have more comments on this. While the API seems simple 
> enough on the surface, I suspect that this will be a challenging feature to 
> implement correctly for all of the cases it will need to handle. It would 
> need careful review and testing as well. My only comment on the API itself is 
> that if we do accept this feature, it should probably go on both Text and 
> TextFlow, and be one of the attributes of Text that is ignored / overridden 
> when a Text node is in a TextFlow.
> 
> -- Kevin
> 
> 
> On 9/18/2019 6:14 PM, Scott Palmer wrote:
>> I would like to implement this feature, being able to adjust the tab size in 
>> a TextFlow or Text node (JDK-8130738 
>> ).  It involves new public 
>> API, so I want to start a discussion about it here.  (My motivation is that 
>> RichTextFX suggests an entirely unacceptable workaround of substituting 
>> actual spaces when the tab character is typed and cites the lack of this 
>> API.)
>> 
>> I’ve already jumped the gun and taken a crack at an implementation.  It is 
>> currently incomplete as I was just poking around to see if it was going to 
>> be easy enough to not take up too much of my time.  It boils down to:
>> TextFlow and Text get a new property for tab width, an integer representing 
>> the number of spaces taken by a tab. (The value is only used to initialize 
>> the tab width for the TextLayout when needed.)
>> TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
>> TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
>> PrismTextLayout implements the new setTabWidth API.
>> 
>> I’m not sure that the Text node needs this new property.  I figured it would 
>> be rarely used on that class, so I had implemented it via an added property 
>> in the private TextAttributes class.  Maybe it isn’t needed at all.
>> 
>> What’s the next step?
>> 
>> Regards,
>> 
>> Scott
> 

Re: JDK-8130738 TextFlow's tab width is static

[Kevin Rushforth]

Hi Scott,

I'm sure Phil will have more comments on this. While the API seems 
simple enough on the surface, I suspect that this will be a challenging 
feature to implement correctly for all of the cases it will need to 
handle. It would need careful review and testing as well. My only 
comment on the API itself is that if we do accept this feature, it 
should probably go on both Text and TextFlow, and be one of the 
attributes of Text that is ignored / overridden when a Text node is in a 
TextFlow.


-- Kevin


On 9/18/2019 6:14 PM, Scott Palmer wrote:

I would like to implement this feature, being able to adjust the tab size in a 
TextFlow or Text node (JDK-8130738 
).  It involves new public 
API, so I want to start a discussion about it here.  (My motivation is that 
RichTextFX suggests an entirely unacceptable workaround of substituting actual spaces 
when the tab character is typed and cites the lack of this API.)

I’ve already jumped the gun and taken a crack at an implementation.  It is 
currently incomplete as I was just poking around to see if it was going to be 
easy enough to not take up too much of my time.  It boils down to:
TextFlow and Text get a new property for tab width, an integer representing the 
number of spaces taken by a tab. (The value is only used to initialize the tab 
width for the TextLayout when needed.)
TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
PrismTextLayout implements the new setTabWidth API.

I’m not sure that the Text node needs this new property.  I figured it would be 
rarely used on that class, so I had implemented it via an added property in the 
private TextAttributes class.  Maybe it isn’t needed at all.

What’s the next step?

Regards,

Scott

JDK-8130738 TextFlow's tab width is static

[Scott Palmer]

I would like to implement this feature, being able to adjust the tab size in a 
TextFlow or Text node (JDK-8130738 
).  It involves new public 
API, so I want to start a discussion about it here.  (My motivation is that 
RichTextFX suggests an entirely unacceptable workaround of substituting actual 
spaces when the tab character is typed and cites the lack of this API.)

I’ve already jumped the gun and taken a crack at an implementation.  It is 
currently incomplete as I was just poking around to see if it was going to be 
easy enough to not take up too much of my time.  It boils down to:
TextFlow and Text get a new property for tab width, an integer representing the 
number of spaces taken by a tab. (The value is only used to initialize the tab 
width for the TextLayout when needed.)
TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
PrismTextLayout implements the new setTabWidth API.

I’m not sure that the Text node needs this new property.  I figured it would be 
rarely used on that class, so I had implemented it via an added property in the 
private TextAttributes class.  Maybe it isn’t needed at all.

What’s the next step?

Regards,

Scott