JBIDE-17911 Make "full tree" vs. "funtions and macro defs." configurable #21
Conversation
for Freemarker outline plus some refactoring to avoid usage of deprecated Prefence APIs.
|
|
||
| @Override | ||
| public void stop(BundleContext context) throws Exception { | ||
| Preferences.getInstance().dispose(); |
There was a problem hiding this comment.
It doesn't feel right. Why it is required?
There was a problem hiding this comment.
It disposes SWT Colors linked in Preferences. I have read somewhere that Colors need to be disposed manually (do they not?) or is this not the right place to invoke Preferences.dispose()?
There was a problem hiding this comment.
I agree, but then it should be
try {
Preferences.getInstance().dispose();
} finally {
super.stop(context);
} There was a problem hiding this comment.
+1 will be added in a new commit in this PR
Editor.addProblemMarker(String, int) and log it rather than ignore
| if (null != file) | ||
| this.lastUpdatedTime = file.getModificationStamp(); | ||
| } | ||
| catch (Exception e) { | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
This catch block is not required, what kind of exception is expected here?
There was a problem hiding this comment.
I do not know because I have not written that code and I had no reason to get acquanted with this particular method. The intention behind the present commit was code formatting and some basic refactoring without any behavior change. Although I agree that catch (Exception e) is generally not a good practice, this particular issue is out of the present scope.
|
@dgolovin, I hope to have addressed all your comments. From my PoV, the last three commits can be squashed together. |
|
released to master |
for Freemarker outline plus some refactoring to avoid usage of
deprecated Prefence APIs.