Comment 1 Angelo ZERR 2013-10-02 04:44:20 EDT

(In reply to Paul Webster from comment #0)
> org.eclipse.e4.tools.orion.editor.swt.OrionEditorControl.
> addDirtyListener(IDirtyListener) throws an NPE when the incoming listener is
> null.
> 
> We should either specify null is a NO-OP in the javadoc and just return, or
> we should throw something like an IllegalArgumentException (their bad) as
> opposed to an NPE (our bad).
> 
> PW

When I have implemented addDirtyListener(IDirtyListener) to use ListenerList, I have copied/pasted the code from org.eclipse.core.commands.Command#addExecutionListener and replace Execution with Dirty.

So I think we must add a Javadoc comments like Command Javadoc : 

----------------------------------------------------------------
@param dirtyListener The listener to be added; must not be null.
----------------------------------------------------------------

Regards Angelo

Comment 2 Paul Webster 2013-10-02 08:52:43 EDT

(In reply to Angelo ZERR from comment #1)
> When I have implemented addDirtyListener(IDirtyListener) to use
> ListenerList, I have copied/pasted the code from
> org.eclipse.core.commands.Command#addExecutionListener and replace Execution
> with Dirty.

Yes, unfortunately that's one of our examples of bad code.  You can either throw an IllegalArgumentException or probably better to use org.eclipse.core.runtime.Assert.isNotNull(Object, String)

> 
> So I think we must add a Javadoc comments like Command Javadoc : 
> 
> ----------------------------------------------------------------
> @param dirtyListener The listener to be added; must not be null.

Yes, that's good.

PW

Comment 3 Patrik Suzzi 2013-10-06 18:47:15 EDT

Created attachment 236160 [details]
img explaining the missing dependency

The image represents an error in codebase

Comment 4 Angelo ZERR 2013-10-06 18:52:57 EDT

(In reply to Patrik Suzzi from comment #3)
> Created attachment 236160 [details]
> img explaining the missing dependency
> 
> The image represents an error in codebase
I don't undertand your error, OrionEditorControl have a dependency with org.apache.commons.lang 2.6.0 and E4 hosts this bundle. 

Which version of Eclipse do you use?

Comment 5 Patrik Suzzi 2013-10-06 18:55:40 EDT

Comment on attachment 236160 [details]
img explaining the missing dependency

I'm using Eclipse 4.3 - 

I tried to download the codebase, but I'm not able to fix the missing dependency. Is t

Here is explained the procedure followed: 
- git-clone the http://git.eclipse.org/gitroot/e4/org.eclipse.e4.tools.git

install plugin prerequisites for e4 tooling 

from http://download.eclipse.org/e4/downloads/ I got the latest Stream integration build update site
http://download.eclipse.org/e4/downloads/drops/I20131005-2200/repository/
from the update site I installed: 
->   Eclipse e4 Tools (Incubation)


from the latest Xtext update site
http://download.eclipse.org/modeling/tmf/xtext/updates/composite/latest/
-> installed XText SDK


At the End I got the error you can see

Comment 6 Angelo ZERR 2013-10-06 19:23:55 EDT

(In reply to Patrik Suzzi from comment #5)
> Comment on attachment 236160 [details]
> img explaining the missing dependency
> 
> I'm using Eclipse 4.3 - 
> 
> I tried to download the codebase, but I'm not able to fix the missing
> dependency. Is t
> 
> Here is explained the procedure followed: 
> - git-clone the http://git.eclipse.org/gitroot/e4/org.eclipse.e4.tools.git
> 
> install plugin prerequisites for e4 tooling 
> 
> from http://download.eclipse.org/e4/downloads/ I got the latest Stream
> integration build update site
> http://download.eclipse.org/e4/downloads/drops/I20131005-2200/repository/
> from the update site I installed: 
> ->   Eclipse e4 Tools (Incubation)
> 
> 
> from the latest Xtext update site
> http://download.eclipse.org/modeling/tmf/xtext/updates/composite/latest/
> -> installed XText SDK
> 
> 
> At the End I got the error you can see

It's a really strange error? My Eclipse E4 4.3 (for Windows) hosts org.apache.commons.lang 2.6.0. Have you tried to remove the version 2.6.0?

Perhaps you should rebuild the project?

Comment 7 Patrik Suzzi 2013-10-06 19:28:51 EDT

Thanks, rebuilding the whole working set fixed the problem.

Comment 8 Eclipse Genie 2015-05-03 05:30:17 EDT

New Gerrit change created: https://git.eclipse.org/r/46988

Comment 9 Robert Roth 2015-05-03 05:34:06 EDT

Proposed patch in context of the #greatfix initiative, please assign me and add the greatfix keyword.

Comment 10 Wim Jongman 2015-05-05 12:32:27 EDT

Thanks Robert.

Comment 11 Eclipse Genie 2015-05-05 12:36:30 EDT

Gerrit change https://git.eclipse.org/r/46988 was merged to [master].
Commit: http://git.eclipse.org/c/e4/org.eclipse.e4.tools.git/commit/?id=2c38ed75c178b8fdbc7aedb3d68bcf688f7ff299

Comment 12 Lars Vogel 2015-05-05 12:41:06 EDT

.