Bug 507947 - Provide generic editor extension for .target files
Summary: Provide generic editor extension for .target files
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.7   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.7 M5   Edit
Assignee: Nobody - feel free to take it CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2016-11-22 07:39 EST by Nobody - feel free to take it CLA
Modified: 2017-01-24 03:22 EST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nobody - feel free to take it CLA 2016-11-22 07:39:10 EST
First iteration would contain support for simple IU autocomplete and syntax highlighting.
Comment 1 Eclipse Genie CLA 2016-11-29 10:54:11 EST
New Gerrit change created: https://git.eclipse.org/r/85954
Comment 2 Nobody - feel free to take it CLA 2016-11-29 10:57:55 EST
Demo of what the above gerrit can do: https://sopotc.fedorapeople.org/Screencast%20from%2011-28-2016%2005:21:39%20PM.webm
Comment 3 Vikas Chandra CLA 2016-12-19 05:48:54 EST
The feature looks good and in a new target file it seems to work great !!



However with 85954/5, I found the following issues
----------------------------------------------------

1) This doesn't work on old target editor files. The reason is that auto completion expects 

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<?pde version="3.8"?><target name="org.eclipse.orion" s>
</target>

doesn't work but

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<?pde version="3.8"?>
<target name="org.eclipse.orion" s>
</target>

works. When PDE create a target editor, it is in a formatting that resembles the former.

2) Lot of places needs //$NON-NLS-1$ addition for example see below

handyAddition = "</" + tags[i] + ">";


3) Dependencies should have range till upto 
Require-Bundle: org.eclipse.core.runtime,
 org.eclipse.jface.text,
 org.eclipse.ui.genericeditor,
 org.eclipse.pde;bundle-version="3.13.0",
 org.eclipse.pde.ui;bundle-version="3.10.0",
 org.eclipse.equinox.p2.metadata;bundle-version="2.3.100",
 org.eclipse.equinox.p2.core;bundle-version="2.4.100",
 org.eclipse.equinox.p2.repository;bundle-version="2.3.200",
 org.eclipse.ui;bundle-version="3.109.0",

something like
 org.eclipse.ui.forms;bundle-version="[3.2.0,4.0.0)",

Else there will be breakage once once of the plugins upgrade even their micro version number.

4) Setting should be in accordance with rest of pde
.settings\org.eclipse.jdt.ui.prefs
\.settings\org.eclipse.jdt.core.prefs

See org.eclipse.pde.ds.annotations(which was recently added) or any other pde plugin.
Comment 4 Nobody - feel free to take it CLA 2016-12-29 06:06:37 EST
Vikas, thank you for the review. I addressed all your concerns in a new patchset in the same gerrit. Now autocomplete should work even if you want all your target file in one single line.

Please review it there so we don't have to hop back and forth between the bug and the gerrit.

Also we need a decision on the CQ story there. Thanks.
Comment 5 Alexander Kurtakov CLA 2017-01-05 10:27:16 EST
Vikas, would you please answer whether CQ will be needed and if yes filing one so this doesn't get delayed by this too.
Comment 7 Vikas Chandra CLA 2017-01-13 09:23:51 EST
Alex, a CQ is not needed.

Thanks Sopot !

Looking forward to further enhancing this and providing support for other PDE editors too.

Sopot, can you update the N&N for pde.
Comment 8 Eclipse Genie CLA 2017-01-13 11:21:22 EST
New Gerrit change created: https://git.eclipse.org/r/88654
Comment 9 Nobody - feel free to take it CLA 2017-01-13 11:22:43 EST
(In reply to Vikas Chandra from comment #7)

> Thanks Sopot !
Thanks to you and Lars for reviewing.
 
> Sopot, can you update the N&N for pde.

Done, see comment 8.
Comment 10 Vikas Chandra CLA 2017-01-13 11:32:53 EST
The N&N item looks good.
Comment 12 Nobody - feel free to take it CLA 2017-01-13 11:52:02 EST
(In reply to Vikas Chandra from comment #10)
> The N&N item looks good.

Merged.
Comment 13 Vikas Chandra CLA 2017-01-14 04:16:21 EST
Works well on

Version: Oxygen (4.7)
Build id: I20170113-2000
Comment 14 Vikas Chandra CLA 2017-01-24 03:22:50 EST
verified on
Version: Oxygen (4.7)
Build id: I20170123-2000