John,
Comments below.
On 29/08/2013 6:37 PM, John Arthorne wrote:
You raise good arguments as always Ed. I will attempt to summarize and
respond to some of the comments raised in this thread.
Q: Should JFace be generified at all.
As with most library generifications there are pros and cons that we
can probably debate all day. I still think on balance it will be a net
benefit for consumers. Some will not have homogeneous element types
and will be stuck with Object, and therefore be no better or worse off
than before. TreeViewer is the worst case, although I think it would
be difficult and awkward to generify some viewer types but not others.
Yes, it would be odd.
My experience with most generification is that 90% of usage is adapted
relatively painlessly and eliminates a tonne of casting, resulting in
cleaner code.
Looking at the existing implementations, I don't see that a tonne of
casting will be eliminated. The instanceof checks will remain...
There are always some rough edges, especially where arrays are found,
and suppressing warnings is sometimes necessary.
Did you miss my point that suppressing the warning results in wrong code?
public E[] getElements(I inputElement) {
return (E[])inputElement.toArray();
}
There is certainly work involved for clients to adopt it, although I
don't find that alone a compelling argument to do nothing at all. A
good place to continue this general discussion would be the master bug
report: https://bugs.eclipse.org/bugs/show_bug.cgi?id=402445
That announcement would have been better made here first.
Q: Should we release incomplete work to a milestone
I'm torn on this one. Certainly in the past our project adopted a "we
won't accept your contribution until it is perfect" approach.
Sorry, but this is the core JFace API that we all use. It better remain
perfect.
There is no doubt this benefits consumers, but I think it has strongly
discouraged all but the most die-hard contributors to the project.
I expect the platform team to be particularly vigilant, and if that
discourages people, that's unfortunate.
Given the number of remaining active contributors, I strongly believe
we need to find ways to make progress using the contributors we have,
at the rate they are capable of contributing.
Sorry, but these APIs are not a playground or a research project.
Undoubtedly this is a slower and bumpier road, but I think the risk is
better than the alternative of stagnating because there are no
contributors who can keep up with the old pace expected/demanded by
the community.
How many hundreds of defects could be addressed? Why do tabs not
repaint properly in Kepler? Why do the back buttons not work properly?
Certainly if there was risk of data loss or severe problems with tool
usability, we would have avoided releasing in a milestone. In this
case the worst case is compiler warnings which can be disabled if
desired.
This attitude of "just ignore it" is so unacceptable.
I understand there are some who consider warnings to be
catastrophically bad,
Yes, a sign of poor code qualityand poor attention to detail.
but the community compiling against our milestones is a tiny fraction
of the overall community. I believe all changes released were binary
compatible and not changing behaviour.
That may be, but we're not in a position to respond to them because they
are incomplete, and in some cases, incorrect, and in the case of generic
arrays, highly questionable.
In the end I admit we got this one wrong. We misjudged the rate of
progress on the work, and many committers being on holiday bogged down
the review process. We should have kept the work in a branch and
waited until at least after M1 to release. Some damage is already
done, but we are now investigating moving the work into a branch. This
will actually introduce a much worse problem for any client that had
reacted to changes already, since it will result in compile errors.
We'll send a separate warning note about this once we are sure on this
path, so it doesn't get lost in this long-winded message.
Q: I have some specific technical concerns about the approach taken so
far.
This is excellent feedback and I'm sure it will be incorporated into
the work going forward. I wonder if we would have had such feedback if
we hadn't released anything to master ;)
How difficult is it to set of an Eclipse development environment for a
specific branch?
Q: Your communication sucks.
Yes it does. I think normally we wait until things are closer to
completion before making big announcements. It is quite often that we
have feature work in progress appearing in a milestone, that we hold
off on announcing until a future milestone when it is more polished.
In this case because the interim state already had significant source
level impact this should have been communicated more widely.
John
From: Ed Merks <[email protected]>
To: [email protected],
Date: 08/29/2013 03:52 AM
Subject: Re: [cross-project-issues-dev] JFace Generics
Sent by: [email protected]
------------------------------------------------------------------------
Hi,
It's difficult to avoid an emotional outburst at this type of thing.
I'm completely shocked that sweeping changes of this nature go
unannounced on this mailing list. Sorry, a blog doesn't cut it...
It's clear the current state is woefully incomplete. I.e., we have
IStructuredContentProvider<E, I> but ITreeContentProvider is still
raw. Of course it's clear that a tree rarely has uniformly typed
children, so what is the plan for the completion of JFace's APIs? I
question all this being committed to master in incremental stages like
this...
EMF is a sea of warnings with these changes, and eliminating those is
weeks of work; work I can't begin because the changes are incomplete.
And of course this affects all users of JFace, not just EMF, so the
overall impact to the community is hard to calculate. The most
disturbing part of all this is that I question whether it has even
been well thought out. For example, the following change is highly
disturbing:
public interface IStructuredContentProvider<E,I> extends
IContentProvider<I> {
public E[] getElements(I inputElement);
}
Suppose I used it like this:
public class GenericContentProvider<E, I extends List<E>> implements
IStructuredContentProvider<E, I> {
@Override
public void dispose() {
}
@Override
public void inputChanged(Viewer<I> viewer, I oldInput, I newInput) {
}
@Override
public E[] getElements(I inputElement) {
return (E[])inputElement.toArray();
}
}
I.e., I have a generic content provider implementation class that for
a List<E> input returns the elements of that list. But there is a
warning in the code for "E[])inputElement.toArray();" and it's not
something one can blithely ignore. It will *never *be possible to
create a generic subclass of IContentProvider that leaves E
unsubstituted by a concrete implementation class, because it will
never be possible to create an E[] array. If you question this
assertion, stop and ask yourself why java.util.Collection.toArray() if
of type Object[] and not of type E[]? It's because it would not be
possible to implement generic Collection implementation classes.
I can't emphasize enough how disturbing I find the approach being
taken here. We're all familiar with using generics, but implementing
generic classes properly remains complex and tricky and what's being
done in JFace doesn't just impact the use of generics, it forces us
all to revisit our implementation classes. For example, perhaps
someone can explain how org.eclipse.jface.viewers.ArrayContentProvider
will be updated? Probably just to "class ArrayContentProvider
implements IStructuredContentProvider<Object, Object>" I would
imagine, but that's not terribly useful is it? I imagine the overall
impact on the community is to make sweeping pointless changes of
precisely this nature. But suppose I even have a nice implementation
class where I wanted IStructuredContentProvider<Foo, Bar>, my current
implementation of getElements is probably wrong and would need to
change to return Foo[]. But of what value is that? ContentProviders
are generally just passed to a generic viewer, which uses it in a
context where the types don't matter. So what's the benefit?
Sorry to be so extreme in my opinion, but I would go further and argue
that it's hard to imagine a significant set of scenarios where the new
APIs are helpful even if this generic array issue wasn't just plain
wrong or a horribly bad idea... I could go on and on, but as I said,
it's hard to remain unemotional about this...
Regards,
Ed
On 29/08/2013 7:46 AM, Lars Vogel wrote:
Hi Eike,
this is a GSoC done by Hendrik (cc) and was announced on
PlanetEclipse. See
_http://blog.vogella.com/2013/06/17/google-summer-of-code-at-eclipse-2/_.
John Arthone and I are the mentors for this project
The work is still ongoing so far the ComboViewer and the TableViewer
have been converted as well several basic classes. We currently don't
know if we can generify IStructuredSelection.
Input is very welcome, the umbrella bug is
_https://bugs.eclipse.org/bugs/show_bug.cgi?id=402445_and every code
change is pushed to Gerrit. Look in
_https://git.eclipse.org/r/#/q/platform/eclipse.platform.ui,n,z_for
Hendrik (so don't know how to narrow the query down to Hendrik only).
Best regards, Lars
2013/8/29 Eike Stepper <[email protected]_ <mailto:[email protected]>>
Hi,
After updating my target platform to Luna I noticed that my workspace
is full of raw type warnings caused by changes in JFace to generify
its APIs, for example Viewer. But the changes look incomplete, e.g.,
ViewerDropAdapter.getViewer() is still a raw type. Can we expect more
such changes, e.g., IStructuredSelection?
Has there been an announcement for these changes? Is there any advice
on how to adjust our code?
What about other Eclipse APIs, such as IAdaptable.getAdapter(Class),
will it be changed to <T> T getAdapter(Class<T>)?
Cheers
/Eike
----_
__http://www.esc-net.de_ <http://www.esc-net.de/>_
__http://thegordian.blogspot.com_ <http://thegordian.blogspot.com/>_
__http://twitter.com/eikestepper_
_______________________________________________
cross-project-issues-dev mailing list_
[email protected]_
<mailto:[email protected]>_
__https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev_
_______________________________________________
cross-project-issues-dev mailing list
[email protected]_
<mailto:[email protected]>
_https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev_
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
_______________________________________________
cross-project-issues-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/cross-project-issues-dev
