Editor refactor fixup #5528
Editor refactor fixup #5528
Conversation
The file was being deleted but the tab was still there
|
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-5528-BUILD-609-linux32.tar.xz ℹ️ The |
| ContributedPlatform referencedPlatform = indexer.getContributedPlaform(referenced); | ||
| if (referencedPlatform != null) | ||
| requiredTools.addAll(referencedPlatform.getResolvedTools()); | ||
| if (referenced != null) { |
There was a problem hiding this comment.
Shouldn't an error be given when a referenced core does not exist?
There was a problem hiding this comment.
Mmmh, good idea 😄
Compilation would fail anyway, so the problem should be solved by the maintainer (Arrow SAMD boards and Win10 IoT core are affected). However a message is ok, I'm pushing it tomorrow
There was a problem hiding this comment.
When first creating tabs, they are already sorted. Can we not re-use the sort criteria from there? Adding an additional comparator just for resorting the tabs on a change makes me think that these sorting criteria could diverge?
Also, is it really needed to resort whenever the tab menu is rebuilt? Is that the right place for the call to reorderTabs().
Finally, is the SketchFile object in the Sketch also inserted in the sorted position? If so, can we not reuse that position when creating the tab? If not, shouldn't the list of SketchFiles be synchronized with the list of tabs? I vaguely recall that there might be code that assumes the tab and sketch index are the same (though I probably tried to remove that as much as possible with my refactor).
|
@facchinm, I looked over the code and added some comments. Other than that, the commits look good to me. Thanks for diving into these! |
|
The tab reordering code is not optimal; sketch list and tabs list are completely independent, but the first one gets ordered every time we add a tab, while the second one does not. |
|
If the list of sketch files is already kept sorted, can't we use the position of a sketch file in the sketch file list as the position when creating a new tab? Or, whenever the list of sketch files changes, re-create the list of tabs or something? |
@facchinm, I confirm those issues are fixed with the test build(609). Thanks! |
Previously it was selected always the last tab because the action sequence was: - create the new tab (in the last position) - select the new tab index (last) - sort the tabs (the new tab is now in the middle but the selected is always the last) instead the correct action sequence is - create the new tab (in the last position) - sort the tabs (now the new tab is in the middle) - select the new tab index (now the correct index is selected)
|
|
✅ Build completed. Please test this code using one of the following: ⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-5528-BUILD-613-linux32.tar.xz ℹ️ The |
|
A bit late, but the extra commits look good to me :-) |
Fixes some of the bugs emerged with recent Editor refactor.
@matthijskooijman @cmaglie take a look if you have some spare minutes (most fixes are trivial)
@per1234 could you test the build that @ArduinoBot will produce to confirm that #5402 and #5415 are gone? Thanks