Downgrade ReadOnly Exception to warning #3299
Conversation
org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/DeltaProcessor.java
Outdated
Show resolved
Hide resolved
| @@ -2022,7 +2023,9 @@ public void resetProjectCaches() { | |||
| */ | |||
| public void registerJavaModelDelta(IJavaElementDelta delta) { | |||
| if (JavaModelManager.isReadOnly()) { | |||
| throw new IllegalStateException("Its not allow to modify JavaModel during ReadOnly action. delta=" + delta); //$NON-NLS-1$ | |||
| ILog.get().warn("JavaModel change during readOnly Operation", new IllegalStateException( //$NON-NLS-1$ | |||
There was a problem hiding this comment.
Sorry, that must be consistent: readOnly Operation -> 'read only' operation
|
@jukzi : I assume we want this in 4.34 RC2? If so, @trancexpress please also review this PR. Background: the feature eclipse-jdt/eclipse.jdt.ui#933 that added So instead of hard failing with exception (and breaking caller code) this PR now allows operation but logs a warning. |
| @@ -2022,7 +2023,9 @@ public void resetProjectCaches() { | |||
| */ | |||
| public void registerJavaModelDelta(IJavaElementDelta delta) { | |||
| if (JavaModelManager.isReadOnly()) { | |||
| throw new IllegalStateException("Its not allow to modify JavaModel during ReadOnly action. delta=" + delta); //$NON-NLS-1$ | |||
| ILog.get().warn("JavaModel change during read only Operation", new IllegalStateException( //$NON-NLS-1$ | |||
There was a problem hiding this comment.
Operation - > operation. Aller guten Dinge sind drei :-)
yes |
|
Jörg, the rules for capitalization rules in English are a bit different to German. TL;DR: first word in a sentence is capitalized, the rest is not, except in menus. |
Throwing an Exception here currently causes more pain then just continuing. Ideally one would rework the getter JavaProject.getResolvedClasspath() to never modify the javamodel, but it's not clear how todo that in a compatible way. eclipse-jdt/eclipse.jdt.ui#1742
| throw new IllegalStateException("Its not allow to modify JavaModel during ReadOnly action. delta=" + delta); //$NON-NLS-1$ | ||
| ILog.get().warn("JavaModel change during read only operation", new IllegalStateException( //$NON-NLS-1$ | ||
| "JavaModel modified during 'read only' operation. Consider to report this warning to https://github.com/eclipse-jdt/eclipse.jdt.core/issues. delta=" //$NON-NLS-1$ | ||
| + delta)); |
There was a problem hiding this comment.
Do we tell the user to report errors to github anywhere else in logged messages? if not I would omit this. Especially on a warning.
Maybe this should be an error though, considering we are expecting trouble to follow after this log (I assume caching somehow will go wrong?).
There was a problem hiding this comment.
Do we tell the user to report errors to github anywhere else in logged messages? if not I would omit this. Especially on a warning.
In other places we've suggested to report to bugzilla, but that time is over :-)
And yes, it would make sense if users would report these warnings, so we can fix inappropriate code.
Throwing an Exception here currently causes more pain then just continuing.
Ideally one would rework the getter JavaProject.getResolvedClasspath() to never modify the javamodel, but it's not clear how todo that in a compatible way.
eclipse-jdt/eclipse.jdt.ui#1742