This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 416399 - [Model] Deprecate MInputPart
Summary: [Model] Deprecate MInputPart
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.3   Edit
Hardware: PC Linux
: P3 normal with 1 vote (vote)
Target Milestone: 4.4 M4   Edit
Assignee: Platform UI Triaged CLA
QA Contact: Eric Moffatt CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-03 04:10 EDT by Lars Vogel CLA
Modified: 2013-12-10 15:34 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2013-09-03 04:10:55 EDT
Currently MInputPart is not used in the Eclipse IDE. As the IDE solves everything with MPart and its persistent data I suggest to deprecate MInputPart to reduce the level of choice for new implementation. 

We also have MPartDescriptor (with the multi flaog) but no equivalent for MINputPart which make MPart easier to use.
Comment 1 Thomas Schindl CLA 2013-09-03 04:36:02 EDT
-1
Comment 2 Lars Vogel CLA 2013-09-03 04:43:15 EDT
@Tom, could you explain why? From a discussion with Eric I had the impression what he would favor this approach so content input would be helpful.
Comment 3 Thomas Schindl CLA 2013-09-03 04:50:45 EDT
Simply because the IDE does not use it, does not mean that it is not useable/used at all in pure e4 Apps/IDEs. How is your pure e4 editor story? To me it absolutely makes sense that an editor has an InputURI abstracting the resource.
Comment 4 Lars Vogel CLA 2013-09-03 04:55:59 EDT
Tom, I agree to having an editor story. But after a discussion with Eric I learned that persistedState offers a more flexible way. You can also send "inputURI" as key via persistedState to a MPart. And we have MPartDescriptors as copy templates for new editors.

But I have no strong opinion on this topic, lets hear what Eric think.
Comment 5 Eric Moffatt CLA 2013-09-03 10:41:01 EDT
Tom, it's not that we don't use it in the IDE, it's more that it was unnecessary...

MInputPart adds only the one string attribute in order to support the 'inputURI' and this can most certainly be achieved using the 'perstistedData' map (which didn't exist when we did the first version of the model). The same pattern could be used if we find that we need any extra info for a particular type of editor.

Why do you think we should keep it ? IMO it unnecessarily pollutes the model (BTW, I'm also working on removing / deprecating a lot of the extra menu/tb elements as well and for the same reason).

The reason to remove it is that we've introduced the MCompositePart but I'd rather remove MInputPart than create an MCompositeInputPart. Why not just stick to MPart with the flavors being handled through the maps ?
Comment 6 Thomas Schindl CLA 2013-09-03 11:41:47 EDT
I have no problem with removing the menu-model cruft which if needed should really should be part of an LegacyIDE.ecore, including makeing the menu-renderers for pure e4 much easier!

To me the solution of persistedState sounds more like an abuse of it than a good design, if we model stuff we should make things explicit and discoverable where a key in peristedState is not.

I can only say that I used InputPart a lot. Yes it is an advanced feature but I'm still in favor of it. I know I could simply provide my own extension .ecore which holds it.

The reason we yet don't have an MInputPartDescriptor is that we've not yet found out what meta-information we need. How do you guys envision creating pure e4 editors? 

To me MPartDescriptor & MPart are the replacement for the view-extension-point and MInputPartDescriptor & MInputPart editor-extension-point with the difference to e3 that editor (input-part) is a specialization of view (part).

Anyways if you all think it is better to remove it, then do it. I'll provide a replacement as part of e(fx)clipse.
Comment 7 Eric Moffatt CLA 2013-09-03 14:54:45 EDT
How about a compromise ? We could put the 'inputURI' directly onto MPart; Tom's correct in his comment about discoverabilty. The new attribute would be documented as providing support for parts that need some external input in order to work.

Frankly, even *having* MInputPart is an indication that we've failed in our goal of simplifying the concepts behind the UI. If we're going to have two model elements we should just have named them MView and MEditor and been done with it. 

Somehow a few years ago that didn't seem like a good idea..;-).

Now, the question as to how e4 supports 'editor' behavior is much more complex than just having an 'input'. Editors are all about associations; file suffixes are the most basic of these. This has proven to be insufficient in the real world because we had 'plugin.xml' that needed a different editor from that used to edit 'normal' XML files and so we introduced the idea of 'content type'...

Now, how do we best express this is e4 ? If it were only file suffixes then it's pretty easy but with content types we need to be backed by runnable code that determines the 'type' so we'd also need to model the content inspectors. There would still need to be some internal logic to find the correct descriptor for a given input.

Again, here I would prefer to extend the existing MPartDescriptor to contain extra fields rather than to explicitly re-introduce the concept of a view / editor split.
Comment 8 Lars Vogel CLA 2013-09-04 04:55:50 EDT
> We could put the 'inputURI' directly onto MPart; 

+1
Comment 9 Brian de Alwis CLA 2013-09-04 10:12:25 EDT
(In reply to Thomas Schindl from comment #6)
> I can only say that I used InputPart a lot. Yes it is an advanced feature
> but I'm still in favor of it. I know I could simply provide my own extension
> .ecore which holds it.

I've used MInputPart to represent an editable resource.  It was a mild pain to structure my identifiers as a URI though.

I wonder if we wouldn't be better off leaving this to an app to decide by creating their own MXXXEditor type — after all, they may want other fields in addition to an inputURI, or something that isn't a URI at all.
Comment 10 Eric Moffatt CLA 2013-09-04 11:38:15 EDT
Brian, I think it's OK to have the simple 'input' field in the MPart (since that's the 90% case) but based on your comment perhaps we should rename it to omit the 'URI', perhaps 'inputSpec' ? 

It's not always the case that editors are based on files / resources; the format of the input specification is defined by a particular editor's implementation. 

It should parse the input specification during creation and create the input (or if it fails to resolve the input throw a PartInitException (or e4 equivalent).
Comment 11 Brian de Alwis CLA 2013-09-04 13:45:57 EDT
My concern is that by providing these fields, or encouraging people to use persistedState, we'll likely end up with developers shoehorning their state requirements as a single string rather than defining meaningful new types for their application.  (Which is exactly what I did with Kizby.)

For example, say I'm building a mapping app where it makes sense to have a editor  with a bounding box and a projection.  As Tom points out, it's likely better that I create an MGeoEditor with specific fields for those interesting values.  But if I see there's an inputURI/inputSpec field, I'll likely assume I should use that and instead spend effort to encode and decode my settings to/from inputURI/inputSpec.

The nice thing about pushing it to the app developers is that it should help ensure that we don't inadvertently become a closed system, tying ourselves to particular types (like MPart and MInputPart).  And a sort-of related question: How does EModelService#createModelElement() handle extensions generated outside of the E4-defined types?

(I wonder if we could somehow surface these custom part-types through the compat layer.  So the compat layer could have an MViewPart and MEditorPart.  JDT could have MJavaCompilationUnitParts and MClassFilePart.  Etc.)
Comment 12 Eric Moffatt CLA 2013-09-06 10:16:41 EDT
This is a little off topic but interesting...

Note that there are many easy ways to format strings (we use XMLMemeno). I expect that *very* few developers are as informed as you are and would rather not extend the model (see below) if they can get the existing one to work for them using the existing mechanisms.

We're currently not set up to handle extended models. Currently I would expect that if you have an extended model then you would also need to extend both the renderer factory and the model service (to extend 'createModelElement'). This would seem to be an excellent area for some research to see if there are ways to factor the code to help (i.e. CSS defined renderers...).
Comment 13 Eric Moffatt CLA 2013-09-09 10:55:32 EDT
I'll let this soak until M3 to get more...er...input...;-).
Comment 14 Mihael Schmidt CLA 2013-10-21 05:18:12 EDT
I second that one should not assume that the resource (to be edited) is a file or easily represented by an URI.

For example my application displays the content of a spooled file from a remote server in an editor and also the content of a userspace object. Both are neither a file nor easily represented in an URI.
Comment 15 Eric Moffatt CLA 2013-10-28 15:35:19 EDT
But you would be able to encode the necessary info into a string correct ? If not then you won't be able to re-open the editor on a restart...
Comment 16 Mihael Schmidt CLA 2013-10-29 03:59:58 EDT
In my cases encoding the necessary information into a string is possible.
Comment 17 Eric Moffatt CLA 2013-12-10 15:34:44 EST
Already fixed...
Comment 18 Eric Moffatt CLA 2013-12-10 15:34:59 EST
Verified in 4.4.0.I20131209-2000.