| Summary: | IXtextBuilderParticipant: deltas contain too much information | ||
|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Victor Noël <victor.noel.irit> |
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | sebastian.zarnekow, sven.efftinge |
| Version: | 1.0.0 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Victor Noël
Hi Victor, can you confirm that delta.hasChanges() returns false for the deltas that you considered superfuous? What do you mean by output directory? /src-gen or /bin? Regards, Sebastian (In reply to comment #1) > can you confirm that delta.hasChanges() returns false for the deltas that you > considered superfuous? It is as you said, delta.haveEObjectDescriptionsChanged() returns false for the deltas concerning other files than the one changed. So this does not seems like a bug, even if I am wondering what is the point of having these deltas, just to know about what was impacted by the change (that's a good point :) But it stills return true for the deltas of the changed file in the output directory. > > What do you mean by output directory? /src-gen or /bin? The /bin one, but it does not happen with every project I have. I have a test project for developing my DSL, and files in the /bin directory are not parsed… But I have also a bigger project (using maven, so the output directory is /target/classes, I don't know if it is important) and for this one, I get the problematic behaviour. Things happens as follow, there is 3 files, common, logic and robot. logic and robot are referencing elements declared in common. common is modified (with something that has no impact on the other files) and saved. 1) IXtextBuilderParticipant's build is called, there is 5 deltas (the boolean is the haveEObjectDescriptionsChanged()): platform:/resource/rosace-agent/target/classes/robot.muadlt: false platform:/resource/rosace-agent/src/main/resources/common.muadlt: true platform:/resource/rosace-agent/src/main/resources/logic.muadlt: false platform:/resource/rosace-agent/target/classes/logic.muadlt: false platform:/resource/rosace-agent/src/main/resources/robot.muadlt: false 2) build is called again, I guess because some code was generated in src-gen so it triggers it or something like that, there is 3 deltas: platform:/resource/rosace-agent/target/classes/logic.muadlt: false platform:/resource/rosace-agent/target/classes/common.muadlt: true platform:/resource/rosace-agent/target/classes/robot.muadlt: false 3) build is called, again, for the same reasons I guess (since the previous one generated some code again), and there is no deltas. I would think that platform:/resource/rosace-agent/target/classes/ should not be read. Thank you (In reply to comment #2) > (In reply to comment #1) > > can you confirm that delta.hasChanges() returns false for the deltas that you > > considered superfuous? > > It is as you said, delta.haveEObjectDescriptionsChanged() returns false for the > deltas concerning other files than the one changed. > So this does not seems like a bug, even if I am wondering what is the point of > having these deltas, just to know about what was impacted by the change (that's > a good point :) I would like to add that it is difficult to rely on delta.haveEObjectDescriptionsChanged() because it does not seem to take into account elements that were not exported (I have the feeling it is based on the name property, but I can't be sure). For example if I modify the content of an element but not the element itself, then the delta is not marked as changed. (In reply to comment #3) > I would like to add that it is difficult to rely on > delta.haveEObjectDescriptionsChanged() because it does not seem to take into > account elements that were not exported (I have the feeling it is based on the > name property, but I can't be sure). That's the intended behavior. We only need to inform dependency if something they reference has changed. The default is name changes, but it can and should be adapted in the org.eclipse.xtext.resource.IResourceDescription.Manager.isAffected(Delta, IResourceDescription) if that is not enough. (In reply to comment #4) > (In reply to comment #3) > > I would like to add that it is difficult to rely on > > delta.haveEObjectDescriptionsChanged() because it does not seem to take into > > account elements that were not exported (I have the feeling it is based on the > > name property, but I can't be sure). > > That's the intended behavior. We only need to inform dependency if something > they reference has changed. The default is name changes, but it can and should > be adapted in the > org.eclipse.xtext.resource.IResourceDescription.Manager.isAffected(Delta, > IResourceDescription) if that is not enough. Ok, but I am not sure this is what I am talking about, the problem here is not about an element in a resource referencing an element present in the delta, but about an element in the delta containing an element that does not have a name but that changed anyway and because of that the delta should return true for haveEObjectDescriptionsChanged(). So it's more about how haveEObjectDescriptionsChanged() is implemented (or about how to compare two elements of the delta) for a delta than about the implication of a delta on another resource. Of course what I say would fall apart if deltas were also internally used to represent changes in elements inside another element, but I didn't had the feeling it was this way… (In reply to comment #5) > Ok, but I am not sure this is what I am talking about, the problem here is not > about an element in a resource referencing an element present in the delta, but > about an element in the delta containing an element that does not have a name > but that changed anyway and because of that the delta should return true for > haveEObjectDescriptionsChanged(). Please use the user data field to store additional information about the relevant state of the object, e.g. a hashCode for all the contents that should be considered in the haveDescriptionsChanged( > Of course what I say would fall apart if deltas were also internally used to > represent changes in elements inside another element, but I didn't had the > feeling it was this way… Deltas are used to compute the affected resources in the builder / among open editors and to provide as much information for listeneres / builder participants as possible. I'ld like to close this one. Any objections? Closed as won't fix. Closing bug which were set to RESOLVED before Eclipse Neon.0. |