Clean up sketch loading

Previously, the Sketch constructor called its `load()` function, which
called the `SketchData.load()` function and `Editor.sketchLoaded()` to
initialize the GUI with the loaded files. When external editing  was
enabled, `Sketch.load()` was called when activating the Arduino app, to
reload the entire sketch.

With this commit, the `Sketch.load()` function is removed, and
`SketchData.load()` is called from the SketchData constructor. Instead
of Sketch calling `EditorTab.sketchLoaded()`, that method is renamed
to `createTabs()` and called by `Editor.HandleOpenInternal()` directly
after creating the Sketch object.

Handling of external editor mode has also changed. When the Arduino
application is activated, instead of fully reloading the sketch (through
the now-absent `Sketch.load()` method), the new `SketchData.reload()`
method is called to reload thie list of files in the sketch. If it
changed, all tabs are re-created. If not, the current tab is reloaded.
When the user switches from one tab to another, that tab is also
reloaded. This ensures that the visible  tab is always up-to-date,
without needlessly reloading all tabs all the time. When external
editing mode is enabled or disabled, all tabs are reloaded too, to make
sure they are up-to-date.

When re-creating all tabs, no attempt is made to preserve the currently
selected tab. Since adding or removing files happens rarely, this should
not be a problem. When files are changed, the currently selected tab is
implicitly preserved. The caret (and thus scroll) position is preserved
by temporarily changing the caret update policy, so the caret does not
move while the text is swapped out. This happens in
`EditorTab.setText()` now, so other users can also profit from it.

To support checking for a changed list of files in
`SketchData.reload()`, a `SketchCode.equals()` method is added, that
just checks if the filenames are equal.

In external editor mode, to ensure that during compilation the version
from disk is always used instead of the in-memory version, EditorTab
detaches itself from its SketchCode, so SketchCode has no access to the
(possibly outdated) in-memory contents of the file.

Author: matthijskooijman

app/src/processing/app/Base.java

@@ -610,11 +610,11 @@ protected void handleActivated(Editor whichEditor) {
     activeEditor.rebuildRecentSketchesMenu();
     if (PreferencesData.getBoolean("editor.external")) {
       try {
-        int previousCaretPosition = activeEditor.getCurrentTab().getTextArea().getCaretPosition();
-        activeEditor.getSketch().load(true);
-        if (previousCaretPosition < activeEditor.getCurrentTab().getText().length()) {
-          activeEditor.getCurrentTab().getTextArea().setCaretPosition(previousCaretPosition);
-        }
+        // If the list of files on disk changed, recreate the tabs for them
+        if (activeEditor.getSketch().reload())
+          activeEditor.createTabs();
+        else // Let the current tab know it was activated, so it can reload
+          activeEditor.getCurrentTab().activated();
       } catch (IOException e) {
         System.err.println(e);
       }

app/src/processing/app/Editor.java

@@ -1604,6 +1604,7 @@ public void selectTab(final int index) {
     redoAction.updateRedoState();
     updateTitle();
     header.rebuild();
+    getCurrentTab().activated();
 
     // This must be run in the GUI thread
     SwingUtilities.invokeLater(() -> {
@@ -1655,7 +1656,11 @@ public int findTabIndex(final String name) {
     return -1;
   }
 
-  public void sketchLoaded(Sketch sketch) {
+  /**
+   * Create tabs for each of the current sketch's files, removing any existing
+   * tabs.
+   */
+  public void createTabs() {
     tabs.clear();
     currentTabIndex = -1;
     tabs.ensureCapacity(sketch.getCodeCount());
@@ -1667,6 +1672,7 @@ public void sketchLoaded(Sketch sketch) {
         System.err.println(e);
       }
     }
+    selectTab(0);
   }
 
   /**
@@ -1981,9 +1987,8 @@ protected boolean handleOpenInternal(File sketchFile) {
       Base.showWarning(tr("Error"), tr("Could not create the sketch."), e);
       return false;
     }
+    createTabs();
 
-    header.rebuild();
-    updateTitle();
     // Disable untitled setting from previous document, if any
     untitled = false;
 

app/src/processing/app/EditorTab.java

@@ -46,6 +46,7 @@
 import javax.swing.text.BadLocationException;
 import javax.swing.text.PlainDocument;
 import javax.swing.undo.UndoManager;
+import javax.swing.text.DefaultCaret;
 
 import org.fife.ui.rsyntaxtextarea.RSyntaxDocument;
 import org.fife.ui.rsyntaxtextarea.RSyntaxTextAreaEditorKit;
@@ -68,6 +69,8 @@ public class EditorTab extends JPanel implements SketchCode.TextStorage {
   protected RTextScrollPane scrollPane;
   protected SketchCode code;
   protected boolean modified;
+  /** Is external editing mode currently enabled? */
+  protected boolean external;
 
   /**
    * Create a new EditorTab
@@ -99,12 +102,12 @@ public EditorTab(Editor editor, SketchCode code, String contents)
     RSyntaxDocument document = createDocument(contents);
     this.textarea = createTextArea(document);
     this.scrollPane = createScrollPane(this.textarea);
+    code.setStorage(this);
     applyPreferences();
     add(this.scrollPane, BorderLayout.CENTER);
 
     UndoManager undo = new LastUndoableEditAwareUndoManager(this.textarea, this.editor);
     ((RSyntaxDocument)textarea.getDocument()).addUndoableEditListener(undo);
-    code.setStorage(this);
   }
 
   private RSyntaxDocument createDocument(String contents) {
@@ -277,19 +280,29 @@ public void applyPreferences() {
     scrollPane.setFoldIndicatorEnabled(PreferencesData.getBoolean("editor.code_folding"));
     scrollPane.setLineNumbersEnabled(PreferencesData.getBoolean("editor.linenumbers"));
 
-    // apply the setting for 'use external editor'
-    if (PreferencesData.getBoolean("editor.external")) {
-      // disable line highlight and turn off the caret when disabling
-      textarea.setBackground(Theme.getColor("editor.external.bgcolor"));
-      textarea.setHighlightCurrentLine(false);
-      textarea.setEditable(false);
-
-    } else {
-      textarea.setBackground(Theme.getColor("editor.bgcolor"));
-      textarea.setHighlightCurrentLine(Theme.getBoolean("editor.linehighlight"));
-      textarea.setEditable(true);
+    // apply the setting for 'use external editor', but only if it changed
+    if (external != PreferencesData.getBoolean("editor.external")) {
+      external = !external;
+      if (external) {
+        // disable line highlight and turn off the caret when disabling
+        textarea.setBackground(Theme.getColor("editor.external.bgcolor"));
+        textarea.setHighlightCurrentLine(false);
+        textarea.setEditable(false);
+        // Detach from the code, since we are no longer the authoritative source
+        // for file contents.
+        code.setStorage(null);
+        // Reload, in case the file contents already changed.
+        reload();
+      } else {
+        textarea.setBackground(Theme.getColor("editor.bgcolor"));
+        textarea.setHighlightCurrentLine(Theme.getBoolean("editor.linehighlight"));
+        textarea.setEditable(true);
+        code.setStorage(this);
+        // Reload once just before disabling external mode, to ensure we have
+        // the latest contents.
+        reload();
+      }
     }
-
     // apply changes to the font size for the editor
     textarea.setFont(PreferencesData.getFont("editor.font"));
   }
@@ -302,7 +315,34 @@ public void updateKeywords(PdeKeywords keywords) {
     document.setTokenMakerFactory(new ArduinoTokenMakerFactory(keywords));
     document.setSyntaxStyle(RSyntaxDocument.SYNTAX_STYLE_CPLUSPLUS);
   }
-  
+
+  /**
+   * Called when this tab is made the current one, or when it is the current one
+   * and the window is activated.
+   */
+  public void activated() {
+    // When external editing is enabled, reload the text whenever we get activated.
+    if (external) {
+      reload();
+    }
+  }
+
+  /**
+   * Reload the contents of our file.
+   */
+  private void reload() {
+    String text;
+    try {
+      text = code.load();
+    } catch (IOException e) {
+      System.err.println(I18n.format("Warning: Failed to reload file: \"{0}\"",
+                                     code.getFileName()));
+      return;
+    }
+    setText(text);
+    setModified(false);
+  }
+
   /**
    * Get the TextArea object for use (not recommended). This should only
    * be used in obscure cases that really need to hack the internals of the
@@ -339,7 +379,18 @@ public String getText() {
    * Replace the entire contents of this tab.
    */
   public void setText(String what) {
+    // Set the caret update policy to NEVER_UPDATE while completely replacing
+    // the current text. Normally, the caret tracks inserts and deletions, but
+    // replacing the entire text will always make the caret end up at the end,
+    // which isn't really useful. With NEVER_UPDATE, the caret will just keep
+    // its absolute position (number of characters from the start), which isn't
+    // always perfect, but the best we can do without making a diff of the old
+    // and new text and some guesswork.
+    DefaultCaret caret = (DefaultCaret) textarea.getCaret();
+    int policy = caret.getUpdatePolicy();
+    caret.setUpdatePolicy(DefaultCaret.NEVER_UPDATE);
     textarea.setText(what);
+    caret.setUpdatePolicy(policy);
   }
 
   /**

app/src/processing/app/Sketch.java

@@ -67,42 +67,8 @@ public class Sketch {
   public Sketch(Editor _editor, File file) throws IOException {
     editor = _editor;
     data = new SketchData(file);
-    load();
   }
 
-
-  /**
-   * Build the list of files.
-   * <P>
-   * Generally this is only done once, rather than
-   * each time a change is made, because otherwise it gets to be
-   * a nightmare to keep track of what files went where, because
-   * not all the data will be saved to disk.
-   * <P>
-   * This also gets called when the main sketch file is renamed,
-   * because the sketch has to be reloaded from a different folder.
-   * <P>
-   * Another exception is when an external editor is in use,
-   * in which case the load happens each time "run" is hit.
-   */
-  private void load() throws IOException {
-    load(false);
-  }
-
-  protected void load(boolean forceUpdate) throws IOException {
-    data.load();
-
-    // set the main file to be the current tab
-    if (editor != null) {
-      int current = editor.getCurrentTabIndex();
-      if (current < 0)
-        current = 0;
-      editor.sketchLoaded(this);
-      editor.selectTab(current);
-    }
-  }
-
-
   private boolean renamingCode;
 
   /**
@@ -948,24 +914,6 @@ private void importLibrary(File jarPath) throws IOException {
   public void prepare() throws IOException {
     // make sure the user didn't hide the sketch folder
     ensureExistence();
-
-    // TODO record history here
-    //current.history.record(program, SketchHistory.RUN);
-
-    // if an external editor is being used, need to grab the
-    // latest version of the code from the file.
-    if (PreferencesData.getBoolean("editor.external")) {
-      // history gets screwed by the open..
-      //String historySaved = history.lastRecorded;
-      //handleOpen(sketch);
-      //history.lastRecorded = historySaved;
-
-      // nuke previous files and settings, just get things loaded
-      load(true);
-    }
-
-//    // handle preprocessing the main file's code
-//    return build(tempBuildFolder.getAbsolutePath());
   }
 
   /**
@@ -1274,6 +1222,9 @@ public boolean isUntitled() {
     return editor.untitled;
   }
 
+  public boolean reload() throws IOException {
+    return data.reload();
+  }
 
   // .................................................................
 

arduino-core/src/processing/app/BaseNoGui.java

@@ -466,14 +466,13 @@ static public void init(String[] args) throws Exception {
         boolean success = false;
         try {
           // Editor constructor loads the sketch with handleOpenInternal() that
-          // creates a new Sketch that, in trun, calls load() inside its constructor
+          // creates a new Sketch that, in turn, builds a SketchData
+          // inside its constructor.
           // This translates here as:
           //   SketchData data = new SketchData(file);
           //   File tempBuildFolder = getBuildFolder();
-          //   data.load();
           SketchData data = new SketchData(absoluteFile(parser.getFilenames().get(0)));
           File tempBuildFolder = getBuildFolder(data);
-          data.load();
 
           // Sketch.exportApplet()
           //  - calls Sketch.prepare() that calls Sketch.ensureExistence()
@@ -520,7 +519,6 @@ static public void init(String[] args) throws Exception {
             //   data.load();
             SketchData data = new SketchData(absoluteFile(path));
             File tempBuildFolder = getBuildFolder(data);
-            data.load();
 
             // Sketch.prepare() calls Sketch.ensureExistence()
             // Sketch.build(verbose) calls Sketch.ensureExistence() and set progressListener and, finally, calls Compiler.build()

arduino-core/src/processing/app/SketchCode.java

@@ -91,7 +91,8 @@ public SketchCode(File file, boolean primary) {
 
   /**
    * Set an in-memory storage for this file's text, that will be queried
-   * on compile, save, and whenever the text is needed.
+   * on compile, save, and whenever the text is needed. null can be
+   * passed to detach any attached storage.
    */
   public void setStorage(TextStorage text) {
     this.storage = text;
@@ -199,6 +200,9 @@ public boolean isModified() {
     return false;
   }
 
+  public boolean equals(Object o) {
+    return (o instanceof SketchCode) && file.equals(((SketchCode) o).file);
+  }
 
   /**
    * Load this piece of code from a file and return the contents. This