Community
Participate
Working Groups
Hi Colleagues, The bug impacts XSD editor, based on XSD EMF model. The editor undo/redo is broken because of this issue. This means loss of data as well beacuse undo breaks the source of the opened XSD, and can not be redo. Here is the info requested for Hotbug request. 1.Affiliation - SAP 2.Be clear on which release you want this bug to be fixed in, or "last workable" date, if requesting a patch. - "last workable" 3.Justify why this is a hotbug. Note that "our users don't like the bug" is not the type of reason that gets much attention. The motivation we are looking for to justify a P1-like priority is, basically, "this bug blocks our adoption of WTP" (explaining why, of course). Use case: - We have a XSD editor based on XSD EMF model. The editor represents XSD file as UI tree and text source. The editor does use EMF commands to make undo/redo over XSD EMF model. This functionality is broken by this bug. The problem: - Making Undo of EMF command does leave formatting characters into XSD source text. These characters make impossible the next undo command, because it is a text one. Text commands do work on characters ranges, and after undo of EMF command, these ranges are not valid anymore. The problem is allocated in org.eclipse.xsd.impl.XSDConcreteComponentImpl.forceNiceInsertBefore(Node, Node, Node), which adds formatting whitespaces. Possible solution: - Do not add formatting spaces into forceNiceInsertBefore(…) method. Please do note that absolutely the same issue exists in WSDL EMF API. Please do see the bug 315951 (there is a proposed solution). I have tested the proposed fix for bug 315951 with you XSD API, and it worked. I would say that the formatting algorithms in the the following methods are quite the same: org.eclipse.wst.wsdl.internal.impl.WSDLElementImpl.forceNiceRemoveChild(Node, Node) and org.eclipse.xsd.impl.XSDConcreteComponentImpl.forceNiceRemoveChild(Node, Node) org.eclipse.wst.wsdl.internal.impl.WSDLElementImpl.forceNiceInsertBefore(Node, Node, Node) and org.eclipse.xsd.impl.XSDConcreteComponentImpl.forceNiceInsertBefore(Node, Node, Node) I will try to adopt the JUnit from the bug 315951 for XSD API, and will attach it later on. Best regards, Dimitar
Created attachment 174301 [details] Junit reproducing the bug. Junit reproducing the bug.
Hi Colleagues, I have created a Junit (junit.TestBug319816.testBug319816()), and attached it in the bugzilla. It is a plugin project which I have exported as archive. It has dependency to EMF Transaction API, so please make sure that you have it: http://download.eclipse.org/modeling/emf/updates/releases/ EMF TRANSACTION SDK 1.3.1 In case you need more information, please do contact me. Best regards, Dimitar
My knee jerk reaction is that there can be no absolute guarantee that making a model change and undoing it will restore every character of the DOM/text-representation to its original state. I.e., I don't see how mixing the two ways of editing is a viable approach to designing an editor. Of course if there are changes that generally improve behavior, I'd consider those, but I don't want to make a blanket commitment that undo/redo of model changes preserve every last character of text.
Hi Ed, Actually the current state is that undoing model changes do undo all characters, except the ones caused by formatting bugs described as bug 315951, and this one. There is no need for you to guarantee anything. The bug is obvious. In case that XSD API does add formatting characters, it does has to remove them on undo. Otherwise it is not a undo, but some partial undo. It is quite nice to edit models via UI, but user has to have freedom to edit source as well. In our opinion this will make the editor more powerfull and usefull. Best regards, Dimitar
Looking closely at the details, we could most certainly do a nicer job on the formatting. We're just assuming that 4 spaces are used for indenting, but the example here uses just one tab. We're also not matching the line feed conventions of the source when inserting new lines. I imagine we could look at the white space of the reference child to match the indentation better. When there isn't such a child, we could look at the whitespace of the parent and look at the delta between that parent and its parent to determine the desired indentation; of course at the root we'd have to choose an indentation convention. I'm sure that approach could make your test case work, but here's a case that still wouldn't work. Imagine you have one extra space before the "type" attribute: <xsd:element name="in" type="xsd:string"/> Now imagine deleting that element and then undoing the delete. The result is <xsd:element name="in" type="xsd:string"/> I.e., note that white space within the attributes of the DOM element aren't preserved. So you can see that in general we cannot guarantee that every character of textual formatting will be preserved. You need to change your expectations or your design. Improving the formatting behavior I see as an enhancement request to be done in XSD 2.7, not a bug to be fixed in a maintenance stream.
I'll change this to an enhancement request. I'll investigate better formatting heuristics that will try to match the formatting in the existing DOM, i.e., to use the appropriate number of tabs or spaces as well as matching line conventions. A client should never assume that undo/redo of model-based commands will preserve all non-semantic aspects of the model, i.e., attribute order and white space.
