Bug 154708 - Allow users to select save options in generated editor
Summary: Allow users to select save options in generated editor
Status: NEW
Alias: None
Product: EMF
Classification: Modeling
Component: Tools (show other bugs)
Version: 2.2   Edit
Hardware: PC Windows XP
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Ed Merks CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-22 11:27 EDT by Kenn Hussey CLA
Modified: 2018-01-22 12:17 EST (History)
1 user (show)

See Also:


Attachments
editor preferences patch (49.61 KB, patch)
2007-02-13 04:53 EST, Jacek Pospychala CLA
no flags Details | Diff
Updates to your patch (77.09 KB, patch)
2007-02-13 15:05 EST, Ed Merks CLA
no flags Details | Diff
patch with preferences hierarchy (159.10 KB, patch)
2007-05-17 12:18 EDT, Jacek Pospychala CLA
no flags Details | Diff
Some tweaks to Jacek's patch (90.55 KB, patch)
2007-05-24 15:11 EDT, Dave Steinberg CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenn Hussey CLA 2006-08-22 11:27:39 EDT
Please provide a mechanism (e.g. prompt to select in a dialog) in the generated editor that allows users to specify which save options are used when saving a resource. Obviously only certain options would make sense; some obvious ones are

XMLResource.OPTION_FORMATTED
XMLResource.OPTION_LINE_WIDTH
XMLResource.OPTION_SAVE_TYPE_INFORMATION
Comment 1 Sven Krause CLA 2006-09-26 02:06:11 EDT
It might be valueable to allow the user to decide the kind of resource references. I like to have an option XMLResource.OPTION_URI_FORMAT (platformResource, pathRelative[default] and pathAbsolute) available.
Comment 2 Jacek Pospychala CLA 2007-02-12 18:23:33 EST
How about putting this options on editor properties page in Window/Preferences? I am working on the page, that will contain listed options, whole feature will be possible to disable from genmodel properties. This will generally provide a place for more editor-oriented options.
Let me know if you have a different ideas.
Comment 3 Jacek Pospychala CLA 2007-02-13 04:53:30 EST
Created attachment 58842 [details]
editor preferences patch

I appreciate all your comments on the patch.

It adds org.eclipse.ui.preferencePages extension to generated editor plugin.xml.
No new plug-in dependencies are added.

Editor is modified in two major places:
1. just before saving Resource to XML, options map is created from preference store: Editor.doSaveAs().new WorkspaceModifyOperation(){ getOptions() }
2. new PreferencePage class is added in editor plugin.

Preference page subclasses FieldEditorPreferencePage so it's easy to add new options with minimum effort now.

Things missing in the patch:
- switching wether preferences should be generated or not, from genmodel properties
Comment 4 Ed Merks CLA 2007-02-13 15:05:56 EST
Created attachment 58890 [details]
Updates to your patch

Jacek,

This looks very cool.  I never even thought about using preferences.  Your patch seemed to have org.eclipse.emf checked out as project, but we normally check out the things under "plugins" as projects, so it was a little bit hard to apply the patch.  The patch I've attached should be easier for us to work with and fixes a few minor problems that I noticed.
Comment 5 Dave Steinberg CLA 2007-02-19 11:26:09 EST
Jacek,

Thanks for the work on this! It's off to a great start.  You've followed our patterns and added the new artifact to the generator almost perfectly. I think using a preference page for this was a really good idea, too, and it's very nice to see how easy it is.

One thing that I think needs some more thought is how best to organize these preferences pages. It seems nasty to dump them at the top level, when other preferences are nicely organized into hierarchies. I imagine that, once you generate a few of them (and we will, just for our included and sample editors), it would get to be a bit of a mess.

My first thought was to put them under General > Editors, but looking at what's there, it doesn't really seem like the right place. Indeed, this is more about the serialization format than the editor, itself. Maybe an XMI category would make sense? Or, if the intent is that most people would add other, non-save related options, maybe not.

Ed also suggested to me that it might make sense to have some kind of cascading arrangement, where you could have defaults for all EMF XMI, and then overrides for each particular model.

Also, instead of generating the full preference page class each time, it might be good to have a framework class with all the basic options. The generated class could extend that, allowing people to modify it as desired.

I'd like to chat with Marcelo about all this when he's back from vacation, and see what he thinks.  I'd also be most interested in your thoughts.
Comment 6 Jacek Pospychala CLA 2007-02-20 06:53:04 EST
(In reply to comment #5)
> One thing that I think needs some more thought is how best to organize these
> preferences pages. 

Right, I've started from the point, that the editor is simple and simple preferences will suit best :) But for better user experience,
preferences should be accessible from two points:
- "Sample Ecore Editor" menu - because there are similar options, like Load Model, Show Properties.
- Preferences - because what we're talking about are preferences for saving documents in the editor. and user knows that preferences are in preferences :)

> My first thought was to put them under General > Editors, but looking at what's
> there, it doesn't really seem like the right place. Indeed, this is more about
> the serialization format than the editor, itself. Maybe an XMI category would
> make sense? Or, if the intent is that most people would add other, non-save
> related options, maybe not.

Good point, but I'm also not sure if it will be common for users, that Editor plug-in has "XMI" menu. They may come one day and ask "I can't save my XML properly, they are saved differently in simple editor than when i do this by hand." So maybe it would be better idea, to clearly mark that preferences belong to editor, and make them like this:

+Ecore editor
 +- SimpleEditor1
 +- SimpleEditor2
...

but it would be perfect if a developer could customize through the plug-in.properties to have this:

+Ecore editor
 +- SimpleEditor1
+Topnotch RCP Editor
 +- Editor2             // (former SimpleEditor1)

> Ed also suggested to me that it might make sense to have some kind of cascading
> arrangement, where you could have defaults for all EMF XMI, and then overrides
> for each particular model.

agree, and top category (either "XMI", "Ecore editor", or else) would be best place to put them.

> Also, instead of generating the full preference page class each time, it might
> be good to have a framework class with all the basic options. The generated
> class could extend that, allowing people to modify it as desired.

yes it's good idea, but i am not sure if there is much to subclass. Right now generated preference page is based on FieldEditorPreferencePage, a framework class for FieldEditor preference pages, so it's very simple. I am not sure what kind of modification do you mean, or in other words, what is generic to all editors properties pages, and what is specific to every one of them.
Do you mean a generic class like this:

GenericPreferencePage {
+GenericPreferencePage(IPreferenceStore) // store is plug-in specific
+addSaveDoctypeOption()
+addSetFormattedOption()
+addSetPreferedWidthOption()
...// other options
+init() // default impl.
-getString() // helper for all those addXXXOption()
}

and then custom implementation calls super(SpecificStore) and adds desired options. The addXXXOption() methods will be two-liners so I am not sure if it's all worth it, but there are also good sides, like the fact that developer will not have to guess what is the appropriate Preference key for certain option. Right now options are very XMLResource oriented.

> I'd like to chat with Marcelo about all this when he's back from vacation, and
> see what he thinks.  I'd also be most interested in your thoughts.

yes, I will also try to update the patch to cover all the points we already agree.

Comment 7 Jacek Pospychala CLA 2007-04-12 12:07:21 EDT
ahh, it's 2 months! but i'm back on the issue and will update you soon.
Comment 8 Jacek Pospychala CLA 2007-05-17 12:18:09 EDT
Created attachment 67674 [details]
patch with preferences hierarchy

updated patch provides default preference page "XMI Editors Preferences" and is a container for specific editors preference pages.

contribution details:
1. adds new generated class to .editor - PreferencePage. It subclasses common GenericEditPreferencePage
2. minor modifications to generated editor, so it reads preferences using two new methods: getPreferences(), getStore()
3. adds new package org.eclipse.emf.edit.ui.preferences to emf.edit.ui: two new classes: 
  GenericEditPreferencePage - super class for all preference classes in EMF.Editor. 
  DefaultEditPreferencePage - Default preferences page (those "XMI Editors Preferences"). So if emf.edit.ui is available, this page would show up in Preferences.

Options available in preference pages are:
XMLResource.OPTION_SAVE_TYPE_INFORMATION
XMLResource.OPTION_SAVE_DOCTYPE
XMLResource.OPTION_FORMATTED
XMLResource.OPTION_LINE_WIDTH
This is for start, any other can be added easily after preference mechanism is approved.

My personal concern: i had to add dependency to org.eclipse.emf.ecore.xmi in emf.edit.ui, to make XMLResource.OPTION_* available. If it's not ok, they should be defined separately in emf.edit.ui.
Comment 9 Jacek Pospychala CLA 2007-05-22 06:14:54 EDT
Dave, how can i ease your work on my patch?

I think it would be good to add a switch in genmodel to enable/disable preferences, so everybody could choose if they want it or not.

Comment 10 Dave Steinberg CLA 2007-05-24 15:11:50 EDT
Created attachment 68645 [details]
Some tweaks to Jacek's patch

Jacek,

First off, I should let you know that it's too late in the cycle to add new UI or other major functionality to EMF 2.3.  So, this will have to go into next year's release, 2.4.

I've looked carefully at your patch. We're making progress, but there's still some way to go.  As I was looking over it and playing with it, I made a few small changes, like applying standard EMF code formatting and (mostly) resolving a naming inconsistency with the platform (switching "preferences" to "preference").

I made a few slightly more substantive changes, too. I switched DefaultEditPreferencePage to use the preference store from its own plug-in, instead of from platform.ui.  It seems quite wrong to use someone else's preference store for one's own preferences, though I could be mistaken about this. Also, I noticed that the mere existence of these preferences were causing non-default options to be used. So, I made sure that GenericEditPreferencePage sets the correct details.  I'm not sure if this is the best way to do things, but something like this must be done. I also made GenericEditPreferencePage set the field labels if unspecified, and I wonder whether specific labels even need to be generated for each model-specific page. When would they ever need to be different from the default?

One big thing that still needs to worked out is the fallback behaviour. Since it's now always setting default, it should never fall back to what is set on the DefaultEditPreferencePage. And I'm convinced that this is an improvement. You can't look individually at preferences that have no notion of being unset and decide whether to fall back or not. What I think we should have is a single checkbox on the generated preference pages that enables the fallback behaviour -- something analogous to the "Enable project specific settings" option on the Java Compiler project properties. A hyperlink to the fallback page would be good, too.

I was discussing all this with Marcelo, too, and one thing we thought about was that people may very well want to move the generated preference page out from under the EMF one, and indeed, use capabilities to hide the global EMF page(s). So, having a hyperlink back to it, if it isn't hidden, would seem most helpful.

Unfortunately, this all connects with broader questions about how else we might use preferences in EMF. If we are able to address EMF usability in the next cycle, as we would like, we might see more EMF preferences, so where your default page and the generated pages fit in would definitely be affected. That said, if you want to continue to work on this now, I'm happy with continuing the current layout of the generated pages nested under one for the default. "EMF Serialization" seems a better label than "XMI Editors Preferences", though I am hesitant to think too much about names before we better nail down how things will be organized.

Another issue Marcelo mentioned, which I think is a good point, is that it would be nicer, rather than just using these options in the editor, to generate a resource factory and resource that use them directly into the model plug-in. That way, the same options would be used universally, including when the instance is first created using the wizard, and in any other tool that may be written. The tricky thing here is that the preference are only available in a UI plug-in, but the model plug-in is non-UI. Marcelo suggested that it might be possible to capture this information from the preferences in metadata and look at that when initializing the resource in the generated resource factory. Maybe there's a more conventional way of addressing this kind of problem, so it seems to be something that could use some investigation.

Finally, I'd better address the two questions you raised. Yes, I definitely think a genmodel option would be good. We may want to consider having it false by default, too, so as to avoid cluttering the preferences with identical, unused pages.  As for the dependency on the ecore.xmi plug-in, I don't personally see that as a major issue, but if others do (or if our switch to fine-grained features made it an issue), I would think that duplicating the constants would be fine, too.
Comment 11 Jacek Pospychala CLA 2007-05-25 01:53:44 EDT
Dave, thanks for detailed comments! sure I will continue work, maybe it's even better there's no rush. I'll follow up with comments as soon as I update patch.