Bug 552134 - org.eclipse.rcp misses dependency to org.eclipse.jface.text
Summary: org.eclipse.rcp misses dependency to org.eclipse.jface.text
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.13   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.16 RC1   Edit
Assignee: Matthias Becker CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2019-10-16 01:03 EDT by Stefan Weiser CLA
Modified: 2020-05-27 06:55 EDT (History)
9 users (show)

See Also:


Attachments
Contains a project with an invalid product because of this bug (2.06 KB, application/x-zip-compressed)
2019-10-16 01:03 EDT, Stefan Weiser CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Weiser CLA 2019-10-16 01:03:31 EDT
Created attachment 280270 [details]
Contains a project with an invalid product because of this bug

With fix 549229 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=549229) the dependency "org.eclipse.jface.text" was added to "org.eclipse.ui.workbench". This leads to the problem that the feature "org.eclipse.rcp" also has a dependency to the plugin "org.eclipse.jface.text" (because "org.eclipse.ui.workbench" is included. 

Feature based products using org.eclipse.rcp (without platform) are now missing the 2 plugins:
- org.eclipse.jface.text
- org.eclipse.text

This problem can be reproduced by validating the product file within the attachement.

This problem may be solved by including the missing plugins in "org.eclipse.rcp".

See also https://github.com/aposin/MergeProcessor/pull/26
Comment 1 Andrey Loskutov CLA 2019-10-16 01:33:41 EDT
(In reply to Stefan Weiser from comment #0)
> With fix 549229 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=549229) the
> dependency "org.eclipse.jface.text" was added to "org.eclipse.ui.workbench".

Wrong bug? I don't see related changes in bug 549229.
Comment 2 Stefan Weiser CLA 2019-10-17 01:06:09 EDT
Sorry, wrong bug referenced.
This is the correct one.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=546251
Comment 3 Andrey Loskutov CLA 2019-10-17 02:39:52 EDT
Matthias, please check this one.
Comment 4 Eclipse Genie CLA 2019-10-17 03:27:48 EDT
New Gerrit change created: https://git.eclipse.org/r/151210
Comment 5 Matthias Becker CLA 2019-10-17 03:29:47 EDT
(In reply to Andrey Loskutov from comment #3)
> Matthias, please check this one.

See my gerrit. Can you please review and give your ok for that change?
Comment 6 Andrey Loskutov CLA 2019-10-17 03:50:06 EDT
(In reply to Matthias Becker from comment #5)
> (In reply to Andrey Loskutov from comment #3)
> > Matthias, please check this one.
> 
> See my gerrit. Can you please review and give your ok for that change?

I'm not experienced with feature / product definitions, but for me it looks like the org.eclipse.jface.text / org.eclipse.text should go to the org.eclipse.e4.rcp feature, that already contains org.eclipse.jface and org.eclipse.jface.databinding.

But I would ask someone else who has an idea why do we have two rcp features one including another.

@All on CC: could you please check this bug / patch https://git.eclipse.org/r/#/c/151210/ ?
Comment 7 Lars Vogel CLA 2019-10-17 04:04:23 EDT
As e4 does not depend on org.eclipse.ui, adding the text bundles to the e4 feature would be wrong.
Comment 8 Lars Vogel CLA 2019-10-17 04:08:46 EDT
Preferred would be a solution without dependency from UI to text. Looks like this dependency was introduced to use a bold style provider. Can this be implemented with the normal JFace provider or can the provider be moved to JFace?

RCP should not depend on text
Comment 9 Eclipse Genie CLA 2019-10-17 05:28:54 EDT
New Gerrit change created: https://git.eclipse.org/r/151222
Comment 10 Eclipse Genie CLA 2019-10-17 05:28:59 EDT
New Gerrit change created: https://git.eclipse.org/r/151223
Comment 11 Matthias Becker CLA 2019-10-17 05:30:39 EDT
(In reply to Lars Vogel from comment #8)
> Preferred would be a solution without dependency from UI to text. Looks like
> this dependency was introduced to use a bold style provider. Can this be
> implemented with the normal JFace provider or can the provider be moved to
> JFace?
> 
> RCP should not depend on text

See my next to gerrits. Are they okay?
Comment 12 Lars Vogel CLA 2019-10-17 06:53:27 EDT
(In reply to Matthias Becker from comment #11)
> (In reply to Lars Vogel from comment #8)
> > Preferred would be a solution without dependency from UI to text. Looks like
> > this dependency was introduced to use a bold style provider. Can this be
> > implemented with the normal JFace provider or can the provider be moved to
> > JFace?
> > 
> > RCP should not depend on text
> 
> See my next to gerrits. Are they okay?

Thanks. They look good to me.
Comment 13 Matthias Becker CLA 2019-10-17 07:08:33 EDT
(In reply to Lars Vogel from comment #12)
> (In reply to Matthias Becker from comment #11)
> > (In reply to Lars Vogel from comment #8)
> > > Preferred would be a solution without dependency from UI to text. Looks like
> > > this dependency was introduced to use a bold style provider. Can this be
> > > implemented with the normal JFace provider or can the provider be moved to
> > > JFace?
> > > 
> > > RCP should not depend on text
> > 
> > See my next to gerrits. Are they okay?
> 
> Thanks. They look good to me.

Is the deprecation-patch also okay? For:
{@link org.eclipse.jface.viewer.BoldStylerProvider )}
I get the warning:
"Javadoc: org.eclipse.jface.viewer cannot be resolved to a type"

Why?
Comment 15 Matthias Becker CLA 2019-10-21 07:27:49 EDT
(In reply to Matthias Becker from comment #13)
> Is the deprecation-patch also okay? For:
> {@link org.eclipse.jface.viewer.BoldStylerProvider )}
> I get the warning:
> "Javadoc: org.eclipse.jface.viewer cannot be resolved to a type"
> 
> Why?
Comment 17 Lars Vogel CLA 2019-10-28 05:56:08 EDT
Thanks, Matthias.
Comment 18 Dawid Pakula CLA 2020-01-03 09:08:05 EST
BoldStylerProvider is now deprecated, so there is any clean alternative to ICompletionProposalExtension7?
Comment 19 Dani Megert CLA 2020-01-03 09:17:15 EST
(In reply to Dawid Pakula from comment #18)
> BoldStylerProvider is now deprecated, so there is any clean alternative to
> ICompletionProposalExtension7?
That deprecation was not correct - at least not without providing a valid alternative to org.eclipse.jface.text.contentassist.ICompletionProposalExtension7.getStyledDisplayString(IDocument, int, BoldStylerProvider)
Comment 20 Matthias Becker CLA 2020-02-25 06:21:19 EST
(In reply to Dani Megert from comment #19)
> (In reply to Dawid Pakula from comment #18)
> > BoldStylerProvider is now deprecated, so there is any clean alternative to
> > ICompletionProposalExtension7?
> That deprecation was not correct - at least not without providing a valid
> alternative to
> org.eclipse.jface.text.contentassist.ICompletionProposalExtension7.
> getStyledDisplayString(IDocument, int, BoldStylerProvider)

So what need to be done here?
Comment 21 Dani Megert CLA 2020-03-15 12:34:44 EDT
(In reply to Matthias Becker from comment #20)
> (In reply to Dani Megert from comment #19)
> > (In reply to Dawid Pakula from comment #18)
> > > BoldStylerProvider is now deprecated, so there is any clean alternative to
> > > ICompletionProposalExtension7?
> > That deprecation was not correct - at least not without providing a valid
> > alternative to
> > org.eclipse.jface.text.contentassist.ICompletionProposalExtension7.
> > getStyledDisplayString(IDocument, int, BoldStylerProvider)
> 
> So what need to be done here?
Either remove the deprecation or provide an alternative.
Comment 22 Dani Megert CLA 2020-04-18 10:20:59 EDT
Ping!
Comment 23 Eclipse Genie CLA 2020-05-12 08:40:24 EDT
New Gerrit change created: https://git.eclipse.org/r/162880
Comment 24 Matthias Becker CLA 2020-05-12 08:40:49 EDT
(In reply to Dani Megert from comment #22)
> Ping!

see my revert
Comment 25 Eclipse Genie CLA 2020-05-13 12:47:31 EDT
New Gerrit change created: https://git.eclipse.org/r/162976
Comment 27 Andrey Loskutov CLA 2020-05-26 03:47:49 EDT
(In reply to Eclipse Genie from comment #26)
> Gerrit change https://git.eclipse.org/r/162880 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> ?id=06fb93d52c43fe3a4334024b5a2ca80973dbaab0

I see API error after this commit. on org.eclipse.jface.text:

The minor version should be incremented in version 3.16.300, since new APIs have been added since version 3.16.200
Comment 28 Matthias Becker CLA 2020-05-26 03:50:30 EDT
(In reply to Andrey Loskutov from comment #27)
> (In reply to Eclipse Genie from comment #26)
> > Gerrit change https://git.eclipse.org/r/162880 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> > ?id=06fb93d52c43fe3a4334024b5a2ca80973dbaab0
> 
> I see API error after this commit. on org.eclipse.jface.text:
> 
> The minor version should be incremented in version 3.16.300, since new APIs
> have been added since version 3.16.200

I already asked this in the gerrit: https://git.eclipse.org/r/#/c/162880/

I don't know why Dani has merged it.
Comment 29 Alexander Kurtakov CLA 2020-05-26 04:08:26 EDT
It doesn't need version increment as it's removal of deprecation. I'll add api filter.
Comment 30 Matthias Becker CLA 2020-05-26 04:11:04 EDT
(In reply to Alexander Kurtakov from comment #29)
> It doesn't need version increment as it's removal of deprecation. I'll add
> api filter.

Thank you.
Comment 31 Eclipse Genie CLA 2020-05-26 04:12:09 EDT
New Gerrit change created: https://git.eclipse.org/r/163579
Comment 33 Alexander Kurtakov CLA 2020-05-26 08:32:15 EDT
Unless someone has some other pending issue I'll resolve this one tomorrow.
Comment 34 Matthias Becker CLA 2020-05-26 08:33:37 EDT
(In reply to Alexander Kurtakov from comment #33)
> Unless someone has some other pending issue I'll resolve this one tomorrow.

okay for me