Cleanup Script Editor #51
Conversation
Add javadoc, enhance variable naming, add some `final` modifiers where possible and format code. Signed-off-by: squareys <squareys@googlemail.com>
Add javadoc, reorder if clause for more clarity, add final modifier. Signed-off-by: squareys <squareys@googlemail.com>
Now accepts `Collection` instead of `Vector`, add Javadoc. Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Both of the afformentioned methods were able to return null (and did exactly one when the TextEditor is first instanciated). To avoid NullPointerExceptions, their return values were checked against null if needed in exactly that case (all undocumented, though). In all other cases the nullcheck is missing. There was two ways to go about this, either adding all the nullchecks (est. > 30 additional nullchecks) or make `getTab` and `getEditorPane` nullsafe, hence never return null and remove all existing nullchecks. Signed-off-by: squareys <squareys@googlemail.com>
Avoid some nullchecks and make sure the constructor argument is allways not null. Signed-off-by: squareys <squareys@googlemail.com>
New name is more descriptive and avoids shadowing in some EditorPane methods. Signed-off-by: squareys <squareys@googlemail.com>
Provide access to the required one via getters. Signed-off-by: squareys <squareys@googlemail.com>
|
I'm curious about the rationale for 5ee745f. Would be good to write into the commit message why you are making the change—e.g., does it avoid a race condition or something? If so, would be good to paste the relevant stack trace(s) into the commit message, so the problem being solved is more clear. |
Actually I just thought it might be better style. As I mentioned, creating an Editor in a Scripting Window (TextEditor class) which is I'm having the feeling you might have missed that that code was synchronized before via a synchronized method. I just inlined it, since it was merely one method call. (I therefore did not change synchronization. But the inlining in the end was pretty unnecessary, because of the synchronized blocks.) |
|
OK, I see what you mean. I disagree that inlining is better style. Quite the contrary. If you have code in multiple places that does the same thing, it should be a dedicated method. That is the DRY principle: "Don't Repeat Yourself." So I definitely think the |
|
What if the (I never meant to say inlining is good style in general.) |
|
The
That is three separate steps, which are always performed together like that. So it should be a method. I understand that it bothers you that a method called |
|
@ctrueden I worked hard to decouple EditorPane from TextEditor, which means those The decoupling is very important for us to be able to reuse EditorPane in the KNIP ScirptEditor, as you originally suggested. As for the discussion we had about inlining: We were talking past each other. If we ever meet in person on a hackathon again, I'd be glad to explain. I was not able to address imagej/scijava-issue-shuttle#1 just yet. I will do so in a separate pullrequest. As soon as you merge this, I can file a pullrequest for the codeassist/autocomplete. Greetings, Squareys. |
Signed-off-by: squareys <squareys@googlemail.com>
More clear formatting and apply eclipse "Code Cleanup". Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
They are not used outside of that package. Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Original fix fa4cb46 was not good enough, since it did not work for multiple new unsaved files. This commit fixes the source instead of the symptom: Avoid using `SwingUtils.invokeLater` for light-weight code in two places to prevent accessing tabs when they are all closed for application shutdown. Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
This is step one for decoupling `EditorPane` from `TextEditor` for potential use in other applications. Also this avoids the necessity of having the synchronized blocks in EditorPane and halves the `setTitle` calls on file open and ScriptEditor startup. Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
Signed-off-by: squareys <squareys@googlemail.com>
…ditor Signed-off-by: squareys <squareys@googlemail.com>
The `updateGitDirectory() call in `setLanguage()` is not required, since the file it is updated for always resides in the same directoy as the file before which means that git directory will not change. Signed-off-by: squareys <squareys@googlemail.com>
|
@ctrueden I merged the "Remove obsolete TODO" commit with the one where it was fixed (and created). The synchronization issue turned obsolete after I refactored to decouple EditorPane from TextEditor. As I mentioned above, If you are still curious about why I thought inlining was a good idea, we may discuss that in person at some hackathon. As I said: We were talking past each other. All together, toggling bookmarks doesn't seem to work, but it turns out, they don't work on master either. You might want to double check, though. All in all, This is ready for merge, and I already finished the autocompletion/codeassist, so I'd be glad, if this PR could be concluded soon, since that will depend on this one. Greetings, Squareys. |
Important if EditorPane should be used outside of the TextEditor. Signed-off-by: squareys <squareys@googlemail.com>
Overhaul the script editor with cleaned up style, javadoc, and better separation of concerns via decoupling the TextEditor and EditorPane.
|
Thanks @Squareys |
|
👍! And just in time for the ImageJ conference too! |
Hello everybody,
as part of #40, I cleaned up the existing script editor code, extracted some classes for readability and added a bunch of javadoc. This involved code formatting, which I tried to keep in separate commits. Everything is done in small and (hopefully) clear steps.
In general this is all refactoring and should not result in any visible changes for the user, but does break the API every now and then (changing member access to private and providing a getter for example).
Greetings,
Squareys