Re: [External] : Re: Eager Evaluation of Invalidation Listeners

2021-10-05 Thread Nir Lisker
I would also answer "no" to both points. This is also what the docs say.

So the question is: how likely do we think that changing this behavior will
> break existing applications?
>

That's the main question. I tested the JavaFX code with the new behavior
and some tests break immediately, though a few I've looked at seemed to be
testing the invalidation listener behavior itself (in their own context). I
don't know what would break outside of the tests. If we go this route, we
might want to create PRs to fix the tests before we actually change
the behavior (in contrast to doing it all in the same PR). I think that
tests fail in several modules and it might require several people to fix
these tests depending on their areas of expertise. Then we would need to
test runtime behavior to see what breaks outside of the tests.

In my own codebase nothing breaks, but it's relatively small.

On the related question, I like the idea of nulling out the current value
> when a property is bound.
>

I can pick up from where the older PR stopped. It's a simple change.

On Wed, Oct 6, 2021 at 3:15 AM Kevin Rushforth 
wrote:

> Given that the intention of InvalidationListener was to be a light-weight
> way to listen to properties without causing a binding chain to be
> revalidated, it does seem reasonable to me that the listener is only fired
> on the valid --> invalid transition, which is the specified behavior, even
> if the implementation doesn't currently do that in all cases.
>
> The two related questions then are:
>
> 1. Should adding an invalidation listener to property do an immediate
> get(), which will ensure that the property is then valid? This will force
> an eager evaluation of the property and "arm" the property to fire an
> invalidation even the next time it is invalidated.
>
> 2. Should adding an invalidation listener to a currently invalid property
> cause the listener to be called (either immediately or the next time the
> object is invalidated)?
>
> I think the ideal answer to both questions is "no", although I share
> John's concern that changing this behavior for InvalidationListeners could
> break applications. So the question is: how likely do we think that
> changing this behavior will break existing applications?
>
> I think it's something we can and should consider changing. Unless there
> are serious concerns, I would support changing this behavior as part of
> avoiding eager evaluation when using invalidation listeners. It would need
> to be well tested (of course), and would need a CSR describing the
> compatibility risk, and should probably get a release note.
>
> Any concerns in doing this?
>
> On the related question, I like the idea of nulling out the current value
> when a property is bound.
>
> -- Kevin
>
>
> On 9/11/2021 9:41 AM, Nir Lisker wrote:
>
> I think that we need your input on this to move it forward.
>
> On Fri, Sep 3, 2021 at 7:49 AM Nir Lisker  wrote:
>
>> so the value field should perhaps be nulled out when bound.
>>
>>
>> There was a PR for something like that in the old repo:
>> https://github.com/javafxports/openjdk-jfx/pull/110
>> 
>>
>> On Fri, Sep 3, 2021 at 5:35 AM John Hendrikx  wrote:
>>
>>>
>>>
>>> On 02/09/2021 11:57, Nir Lisker wrote:
>>> > So in order
>>> > to make sure that a new interested invalidation listener does not
>>> miss
>>> > the fact that a property was *already* invalid, the easiest
>>> solution
>>> > might have been to revalidate it upon registration of a new
>>> listener
>>> >
>>> >
>>> > But why does an invalidation listener need to know the past state of
>>> the
>>> > property? It is only interested in the valid -> invalid transition. If
>>> > the property was invalid then the listener (in theory) shouldn't
>>> receive
>>> > any events anyway on subsequent invalidations. (I understand that you
>>> > don't justify this, I'm posing it as a general question.)
>>>
>>> Strictly speaking, no, if users are using InvalidationListener correctly
>>> then this is definitely correct behavior. I'm not really advocating a
>>> change, and I'd even prefer that it be brought in line with the
>>> documentation.
>>>
>>> I think however that many users are not using it correctly and expect an
>>> invalidation event always the next time the value changes (and their
>>> listener will read that value, validating it again), making it act like
>>> a light-weight ChangeListener. I know that I probably have written code
>>> that made that assumption, and would in the past not even think twice
>>> about replacing a change with an invalidation listener or vice versa if
>>> that happened to be a better fit. Which is sort of what happened as well
>>> in the bidirectional binding PR, and the issue slipped past the author
>>> and two reviewers.
>>>
>>> > I suggest that we split the problem 

Re: [External] : Re: Eager Evaluation of Invalidation Listeners

2021-10-05 Thread Kevin Rushforth
Given that the intention of InvalidationListener was to be a 
light-weight way to listen to properties without causing a binding chain 
to be revalidated, it does seem reasonable to me that the listener is 
only fired on the valid --> invalid transition, which is the specified 
behavior, even if the implementation doesn't currently do that in all cases.


The two related questions then are:

1. Should adding an invalidation listener to property do an immediate 
get(), which will ensure that the property is then valid? This will 
force an eager evaluation of the property and "arm" the property to fire 
an invalidation even the next time it is invalidated.


2. Should adding an invalidation listener to a currently invalid 
property cause the listener to be called (either immediately or the next 
time the object is invalidated)?


I think the ideal answer to both questions is "no", although I share 
John's concern that changing this behavior for InvalidationListeners 
could break applications. So the question is: how likely do we think 
that changing this behavior will break existing applications?


I think it's something we can and should consider changing. Unless there 
are serious concerns, I would support changing this behavior as part of 
avoiding eager evaluation when using invalidation listeners. It would 
need to be well tested (of course), and would need a CSR describing the 
compatibility risk, and should probably get a release note.


Any concerns in doing this?

On the related question, I like the idea of nulling out the current 
value when a property is bound.


-- Kevin


On 9/11/2021 9:41 AM, Nir Lisker wrote:

I think that we need your input on this to move it forward.

On Fri, Sep 3, 2021 at 7:49 AM Nir Lisker > wrote:


so the value field should perhaps be nulled out when bound.


There was a PR for something like that in the old repo:
https://github.com/javafxports/openjdk-jfx/pull/110



On Fri, Sep 3, 2021 at 5:35 AM John Hendrikx mailto:hj...@xs4all.nl>> wrote:



On 02/09/2021 11:57, Nir Lisker wrote:
>     So in order
>     to make sure that a new interested invalidation listener
does not miss
>     the fact that a property was *already* invalid, the
easiest solution
>     might have been to revalidate it upon registration of a
new listener
>
>
> But why does an invalidation listener need to know the past
state of the
> property? It is only interested in the valid -> invalid
transition. If
> the property was invalid then the listener (in theory)
shouldn't receive
> any events anyway on subsequent invalidations. (I understand
that you
> don't justify this, I'm posing it as a general question.)

Strictly speaking, no, if users are using InvalidationListener
correctly
then this is definitely correct behavior. I'm not really
advocating a
change, and I'd even prefer that it be brought in line with the
documentation.

I think however that many users are not using it correctly and
expect an
invalidation event always the next time the value changes (and
their
listener will read that value, validating it again), making it
act like
a light-weight ChangeListener. I know that I probably have
written code
that made that assumption, and would in the past not even
think twice
about replacing a change with an invalidation listener or vice
versa if
that happened to be a better fit. Which is sort of what
happened as well
in the bidirectional binding PR, and the issue slipped past
the author
and two reviewers.

> I suggest that we split the problem into 2: one is the case
where the
> property was valid when the listener was attached, and the
other is when
> it was invalid.
> * A valid starting state. In this case attaching a listener
shouldn't
> need to do anything. A subsequent invalidation event will be
sent
> regardless. Currently, it is calling get() redundantly.

True, the call to get is redundant in this case. Ugly too,
calling get
and discarding its result, while the intention is to force the
property
to become valid.

> * An invalid starting state. In this case the documentation
says that
> nothing needs to happen, but get() is called anyway. Here, the
> difference is that a subsequent invalidation event is sent
in one case
> and not in the other. The only way to advance here is to
   

Gtk4 on openjfx-sandbox

2021-10-05 Thread Thiago Milczarek Sayão
Hi,

I have done some porting to gtk4:

https://github.com/openjdk/jfx-sandbox/tree/tsayao_gtk4_playground

It currently compiles, but does not show any windows (I suspect of an
threading issue). Some parts such as DND are commented out.

To use it, pass: -Djdk.gtk.version=4 -Djdk.gtk.verbose=true

-- Thiago.


Re: RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses [v7]

2021-10-05 Thread Michael Strauß
On Fri, 20 Aug 2021 05:15:49 GMT, Michael Strauß  wrote:

>> This PR adds the `Node.focusVisible` and `Node.focusWithin` properties, as 
>> well as the corresponding `:focus-visible` and `:focus-within` CSS 
>> pseudo-classes.
>
> Michael Strauß has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixed undeterministic test failures

I've created a [CSR](https://bugs.openjdk.java.net/browse/JDK-8274798) for this 
feature.

-

PR: https://git.openjdk.java.net/jfx/pull/475


Re: Proof of concept for fluent bindings for ObservableValue

2021-10-05 Thread John Hendrikx

I made the title more specific :)

--John

On 05/10/2021 17:58, Nir Lisker wrote:

Looks good. I assume we will add more bindings in the future like
conditionOn or anything else that I would put under "extras", so maybe
the title could be more specific since there will be more fluent
bindings in the future?

On Tue, Oct 5, 2021 at 1:34 PM John Hendrikx mailto:hj...@xs4all.nl>> wrote:

I've created https://bugs.openjdk.java.net/browse/JDK-8274771

Please have a look.

--John

On 04/10/2021 17:51, Nir Lisker wrote:
> I think that a PR can be created. The only point we need to decide
is about
> the subscription models we talked about above. ReactFX uses something
> different for each, you use the same. That can determine if we need
> different classes for each binding type.
>
> I can create the JBS issue for this one and a draft CSR if you want.
>
> On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker mailto:nlis...@gmail.com>> wrote:
>
>> The OrElseBinding is incorrect
>>>
>>
>> Yes, that was a typo with the order of the arguments in the ternary
>> statement.
>>
>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>>> think the tests would become pretty unreadable and less useful /
>>> thourough otherwise).
>>>
>>> What are the options?
>>>
>>
>> This is a bigger question. Kevin will have to answer that.
>>
>> As for the subscription model: flat map has a more complicated
one, but
>>> orElse, orElseGet and map all have the same one.
>>>
>>
>> From what I saw, ReactFX uses a different subscription model for
these. I
>> could have misread it.
>>
>> The messages will need to be written from the perspective of the
Binding
>>> class then IMHO.
>>>
>>
>> That's fine.
>>
>> As for the Optional methods, I'll have a look in my code base and
see if
>> the places I would like to use them at will become irrelevant
anyway with
>> the fluent bindings. I'm fine with proceeding with the current
names of the
>> proposed methods.
>>
>> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx mailto:hj...@xs4all.nl>> wrote:
>>
>>> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
>>> please see the tests here:
>>>
>>>
>>>

https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
>>>
>>> The OrElseBinding is incorrect, the compute value should read:
>>>
>>>  protected T computeValue() {
>>>T value = source.getValue();
>>>return value == null ? constant : value;
>>>  }
>>>
>>> Similar for OrElseGetBinding.
>>>
>>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>>> think the tests would become pretty unreadable and less useful /
>>> thourough otherwise).
>>>
>>> What are the options?
>>>
>>> - Integrate a nested runner (there is an Apache 2.0 licensed one
>>> available)
>>> - Create our own nested runner (about 200 lines of code)
>>> - Can we introduce JUnit 5?
>>> - Rewrite to plain JUnit 4?
>>>
>>> Let me know, and I can integrate them.
>>>
>>> --John
>>>
>>> On 19/09/2021 02:12, Nir Lisker wrote:
 I've played around with the implementation and pushed a modified
 prototype to the sandbox [1]. I ended up with something similar to
>>> ReactFX:
 the orElse and orElseGet methods have their own binding classes
that
 extend LazyObjectBinding, just like MapBinding and
FlatMapBinding. The
 reason being that both their compute value and their
subscription models
 are slightly different. While they can be matched to MapBinding
like you
 did, it entails a bit of a roundabout way in my opinion. Creating a
 supplier for the constant value of orElse somewhat defeats the
purpose I
 think.
>>>
>>> I have no strong opinion here, just wanted to keep the MR small. The
>>> supplier should be an inline candidate, but creating a separate
class is
>>> fine too.
>>>
>>> As for the subscription model: flat map has a more complicated
one, but
>>> orElse, orElseGet and map all have the same one.
>>>
 In addition, I think that it's fine to move the arguments' null
checks
>>> to
 the binding classes themselves, even if it's a couple of levels
deeper
>>> on
 the stack, while adding a message in the 2nd argument of
 Objects.requireNonNull for clarity. These classes are
"self-contained"
>>> so
 it makes sense for them to check their arguments. They might be
used in
 other places, perhaps in the public Bindings class.
>>>
>>> The messages will need to be written from the perspective of the

RFR: 8232812: [MacOS] Double click title bar does not restore window size

2021-10-05 Thread Martin Fox
The test case for JDK-8160241 creates a window in a zoomed state (as defined by 
macOS). When the OS later goes to unzoom the window it will try to shrink it 
down to 1 point wide. This was entered as JDK-8163137 but the fix for that bug 
inadvertently disabled unzooming altogether. This PR fixes 8163137 in a 
slightly different way.

Access to the zoom/unzoom feature has changed with newer versions of macOS. To 
reproduce this you have to change `System Preferences > Dock & Menu Bar > 
Double-click a window's title bar` to `zoom`. Then use double-clicks in the 
title bar to test the feature. The green button in the title bar no longer has 
anything to do with zoom/unzoom.

-

Commit messages:
 - Unzooming windows using double-clicks works again.

Changes: https://git.openjdk.java.net/jfx/pull/639/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=639=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8232812
  Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/639.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/639/head:pull/639

PR: https://git.openjdk.java.net/jfx/pull/639


Re: RFR: 8272870: Add convenience factory methods for border and background [v4]

2021-10-05 Thread Ambarish Rapte
On Sun, 19 Sep 2021 01:40:11 GMT, Nir Lisker  wrote:

>> Added convenience factory factory methods for Background and Border.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added null tests and changed doc as per comments

Marked as reviewed by arapte (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/610


Re: Proof of concept for fluent bindings for ObservableValue

2021-10-05 Thread Nir Lisker
Looks good. I assume we will add more bindings in the future like
conditionOn or anything else that I would put under "extras", so maybe the
title could be more specific since there will be more fluent bindings in
the future?

On Tue, Oct 5, 2021 at 1:34 PM John Hendrikx  wrote:

> I've created https://bugs.openjdk.java.net/browse/JDK-8274771
>
> Please have a look.
>
> --John
>
> On 04/10/2021 17:51, Nir Lisker wrote:
> > I think that a PR can be created. The only point we need to decide is
> about
> > the subscription models we talked about above. ReactFX uses something
> > different for each, you use the same. That can determine if we need
> > different classes for each binding type.
> >
> > I can create the JBS issue for this one and a draft CSR if you want.
> >
> > On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker  wrote:
> >
> >> The OrElseBinding is incorrect
> >>>
> >>
> >> Yes, that was a typo with the order of the arguments in the ternary
> >> statement.
> >>
> >> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> >>> think the tests would become pretty unreadable and less useful /
> >>> thourough otherwise).
> >>>
> >>> What are the options?
> >>>
> >>
> >> This is a bigger question. Kevin will have to answer that.
> >>
> >> As for the subscription model: flat map has a more complicated one, but
> >>> orElse, orElseGet and map all have the same one.
> >>>
> >>
> >> From what I saw, ReactFX uses a different subscription model for these.
> I
> >> could have misread it.
> >>
> >> The messages will need to be written from the perspective of the Binding
> >>> class then IMHO.
> >>>
> >>
> >> That's fine.
> >>
> >> As for the Optional methods, I'll have a look in my code base and see if
> >> the places I would like to use them at will become irrelevant anyway
> with
> >> the fluent bindings. I'm fine with proceeding with the current names of
> the
> >> proposed methods.
> >>
> >> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx  wrote:
> >>
> >>> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
> >>> please see the tests here:
> >>>
> >>>
> >>>
> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
> >>>
> >>> The OrElseBinding is incorrect, the compute value should read:
> >>>
> >>>  protected T computeValue() {
> >>>T value = source.getValue();
> >>>return value == null ? constant : value;
> >>>  }
> >>>
> >>> Similar for OrElseGetBinding.
> >>>
> >>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> >>> think the tests would become pretty unreadable and less useful /
> >>> thourough otherwise).
> >>>
> >>> What are the options?
> >>>
> >>> - Integrate a nested runner (there is an Apache 2.0 licensed one
> >>> available)
> >>> - Create our own nested runner (about 200 lines of code)
> >>> - Can we introduce JUnit 5?
> >>> - Rewrite to plain JUnit 4?
> >>>
> >>> Let me know, and I can integrate them.
> >>>
> >>> --John
> >>>
> >>> On 19/09/2021 02:12, Nir Lisker wrote:
>  I've played around with the implementation and pushed a modified
>  prototype to the sandbox [1]. I ended up with something similar to
> >>> ReactFX:
>  the orElse and orElseGet methods have their own binding classes that
>  extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
>  reason being that both their compute value and their subscription
> models
>  are slightly different. While they can be matched to MapBinding like
> you
>  did, it entails a bit of a roundabout way in my opinion. Creating a
>  supplier for the constant value of orElse somewhat defeats the
> purpose I
>  think.
> >>>
> >>> I have no strong opinion here, just wanted to keep the MR small. The
> >>> supplier should be an inline candidate, but creating a separate class
> is
> >>> fine too.
> >>>
> >>> As for the subscription model: flat map has a more complicated one, but
> >>> orElse, orElseGet and map all have the same one.
> >>>
>  In addition, I think that it's fine to move the arguments' null checks
> >>> to
>  the binding classes themselves, even if it's a couple of levels deeper
> >>> on
>  the stack, while adding a message in the 2nd argument of
>  Objects.requireNonNull for clarity. These classes are "self-contained"
> >>> so
>  it makes sense for them to check their arguments. They might be used
> in
>  other places, perhaps in the public Bindings class.
> >>>
> >>> The messages will need to be written from the perspective of the
> Binding
> >>> class then IMHO.
> >>>
>  I also moved the subscription-related methods from ObservableValue to
>  static methods in Subscription, at least for now, to defer the API
> >>> related
>  to Subscription.
> >>>
> >>> Sounds good.
> >>>
>  The branch doesn't compile because I didn't finish working on the
>  visibility issue, but it's close enough to what I envision it like for
> >>> the
>  first 

Re: RFR: 8272870: Add convenience factory methods for border and background [v4]

2021-10-05 Thread Kevin Rushforth
On Sun, 19 Sep 2021 01:40:11 GMT, Nir Lisker  wrote:

>> Added convenience factory factory methods for Background and Border.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added null tests and changed doc as per comments

Looks good. I'll also review the CSR, and then you can Finalize.

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/610


RFR: 8187474: Tree-/TableCell, TreeCell: editingCell/Item not updated in cell.startEdit

2021-10-05 Thread Jeanette Winzenburg
cell startEdit is supposed to update the editing location on its associated 
control - was done in ListCell, not in Tree-/TableCell nor TreeCell.

Fix was to add control.edit(..). Note that ListCell was also touched to use the 
exact same method call pattern as the fixed cell types.

Added/unignored cell tests that are failing/passing before/after the fix.

-

Commit messages:
 - 8187474: Tree-/TableCell, TreeCell: editingCell/Item not updated in

Changes: https://git.openjdk.java.net/jfx/pull/638/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=638=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8187474
  Stats: 76 lines in 7 files changed: 46 ins; 21 del; 9 mod
  Patch: https://git.openjdk.java.net/jfx/pull/638.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/638/head:pull/638

PR: https://git.openjdk.java.net/jfx/pull/638


Re: Proof of concept for fluent bindings for ObservableValue

2021-10-05 Thread John Hendrikx

I've created https://bugs.openjdk.java.net/browse/JDK-8274771

Please have a look.

--John

On 04/10/2021 17:51, Nir Lisker wrote:

I think that a PR can be created. The only point we need to decide is about
the subscription models we talked about above. ReactFX uses something
different for each, you use the same. That can determine if we need
different classes for each binding type.

I can create the JBS issue for this one and a draft CSR if you want.

On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker  wrote:


The OrElseBinding is incorrect




Yes, that was a typo with the order of the arguments in the ternary
statement.

I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I

think the tests would become pretty unreadable and less useful /
thourough otherwise).

What are the options?



This is a bigger question. Kevin will have to answer that.

As for the subscription model: flat map has a more complicated one, but

orElse, orElseGet and map all have the same one.



From what I saw, ReactFX uses a different subscription model for these. I
could have misread it.

The messages will need to be written from the perspective of the Binding

class then IMHO.



That's fine.

As for the Optional methods, I'll have a look in my code base and see if
the places I would like to use them at will become irrelevant anyway with
the fluent bindings. I'm fine with proceeding with the current names of the
proposed methods.

On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx  wrote:


I need to get you the tests I wrote, unfortunately, they're JUnit 5,
please see the tests here:


https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value

The OrElseBinding is incorrect, the compute value should read:

 protected T computeValue() {
   T value = source.getValue();
   return value == null ? constant : value;
 }

Similar for OrElseGetBinding.

I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
think the tests would become pretty unreadable and less useful /
thourough otherwise).

What are the options?

- Integrate a nested runner (there is an Apache 2.0 licensed one
available)
- Create our own nested runner (about 200 lines of code)
- Can we introduce JUnit 5?
- Rewrite to plain JUnit 4?

Let me know, and I can integrate them.

--John

On 19/09/2021 02:12, Nir Lisker wrote:

I've played around with the implementation and pushed a modified
prototype to the sandbox [1]. I ended up with something similar to

ReactFX:

the orElse and orElseGet methods have their own binding classes that
extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
reason being that both their compute value and their subscription models
are slightly different. While they can be matched to MapBinding like you
did, it entails a bit of a roundabout way in my opinion. Creating a
supplier for the constant value of orElse somewhat defeats the purpose I
think.


I have no strong opinion here, just wanted to keep the MR small. The
supplier should be an inline candidate, but creating a separate class is
fine too.

As for the subscription model: flat map has a more complicated one, but
orElse, orElseGet and map all have the same one.


In addition, I think that it's fine to move the arguments' null checks

to

the binding classes themselves, even if it's a couple of levels deeper

on

the stack, while adding a message in the 2nd argument of
Objects.requireNonNull for clarity. These classes are "self-contained"

so

it makes sense for them to check their arguments. They might be used in
other places, perhaps in the public Bindings class.


The messages will need to be written from the perspective of the Binding
class then IMHO.


I also moved the subscription-related methods from ObservableValue to
static methods in Subscription, at least for now, to defer the API

related

to Subscription.


Sounds good.


The branch doesn't compile because I didn't finish working on the
visibility issue, but it's close enough to what I envision it like for

the

first PR.


I've ported the changes over to my branch and ran the tests on it, they
all pass apart from the above mentioned problem in the OrElse bindings.


As for the java,util.Optional methods, I indeed did something like
`x.orElse(5).getValue()` in the past in other cases, but this approach
creates a new binding just to get the wrapped value out, which is very
inefficient. (In one case I did booleanValue.isNull().get(), which

creates

a boolean binding, because isPresent() does not exist). I would like to

see

what others think about the Optional methods, The binding method

variants

are much more important, but I want to avoid a name clash if the

Optional

ones will have support.


Okay, some more things to consider:

ObservableValue is not an Optional, their get methods respond
differently with Optional#get throwing an exception when null -- the
#orElse is a necessity; this is not the case for