JavaScript indentation ignores the IDE indentation settings!

Synopsis: JavaScript indentation (formatting, smart indent) is always 4 characters. Changing the global default
won't affect it. There is no workaround. It was reported by an external user:
http://www.nabble.com/Format-of-a-JS-file-to16556614.html

Ruby users really want indentation 2 for all their files and this isn't possible right now, both for pure JavaScript
files and embedded ERB or HTML files.



Why this must be fixed in 6.1:
I believe this is a must fix for 6.1 because users are very sensitive to indentation settings. If the IDE won't let them
indent the code to the indentation level they prefer, they get very upset - this has been temporarily broken in the
past for Ruby for example with users switching to other tools until it got resolved.


Why did this bug happen?
NetBeans has an IDE-wide indentation setting which defaults to 4. This was not right for Ruby, where nearly everybody 
wants the default indentation to be 2.  I had code in the Ruby indentation formatter and in GSF to try to deal with
this situation such that Ruby can have its own indentation default which works both for tab-spacing in the document
and for indenting. 

Unfortunately, this got carried over into the JavaScript formatter as well. But unlike Ruby, JavaScript doesn't
have an options panel where you set its indentation setting - and this is why there is no way to work around this
bug: the JavaScript formatter is tied to an options object which isn't accessible from the UI.  Furthermore,
JavaScript isn't like Ruby and should be tied to the global IDE options.


Why wasn't this caught until now?
* Automatic testing:  We have lots of unit tests for formatting, using different indentation preferences (2, 4, various
versions of continuation indents) etc. But these are all unit tests with different indentation settings
hardcoded for the test. To actually test what happens in the IDE using the editor options dialog (which ends up
calling various basekit options etc) we'd have to write an integration/gui test which is not there. Thus, the
formatters handle different indentation settings fine but what we don't have a test for is whether changing IDE
options actually affect the language indentation.
* Manual testing: Most of us at Sun prefer the defaults, so we just don't think to run with "weird" formatting
options like indent=8. However, for companies that follow different conventions this is a showstopper.  I have
tested it a couple of times in the past but it was broken in other areas (e.g. see #130644) so I figured the
problem was only elsewhere.


What is the risk of the fix?
The code only modifies indentation-size related settings, and only in the JavaScript area. That means that (barring
NPEs etc.) it can only break the indentation levels for JavaScript files, which are already wrong.  There is also
the risk that the hardcoded default 4, is better than GSF's preference for indentation 2. I'm investigating that
right now.


Diffs + Explanation of the fix:

There are a couple of different components to the fix.

First, for JavaScript, don't return a preferred indent size; return -1 instead. On the GSF side, we will interpret
this return value as an indication that this language doesn't want to override the document default; if don't return
a natural number then we will simply call the super class to get the shift width for this document.

diff -r d7c458cefd44 gsf/src/org/netbeans/modules/gsf/GsfDocument.java
--- a/gsf/src/org/netbeans/modules/gsf/GsfDocument.java	Tue Apr 08 17:16:25 2008 +0400
+++ b/gsf/src/org/netbeans/modules/gsf/GsfDocument.java	Tue Apr 08 11:44:10 2008 -0700
@@ -103,7 +104,10 @@
     public int getShiftWidth() {
         org.netbeans.modules.gsf.api.Formatter f = language.getFormatter();
         if (f != null) {
-            return f.indentSize();
+            int shiftWidth = f.indentSize();
+            if (shiftWidth > 0) {
+                return shiftWidth;
+            }
         }
         return super.getShiftWidth();
     }

diff -r d7c458cefd44 javascript.editing/src/org/netbeans/modules/javascript/editing/JsFormatter.java
--- a/javascript.editing/src/org/netbeans/modules/javascript/editing/JsFormatter.java	Tue Apr 08 17:16:25 2008 +0400
+++ b/javascript.editing/src/org/netbeans/modules/javascript/editing/JsFormatter.java	Tue Apr 08 11:44:38 2008 -0700
     public int indentSize() {
-        return codeStyle.getIndentSize();
+        return -1; // Use IDE defaults
     }
     
     public int hangingIndentSize() {
-        return codeStyle.getContinuationIndentSize();
+        return -1; // Use IDE defaults
     }


Second, in the old code which used to copy the (Ruby) code style preferences into the Document indentation
settings, reverse the logic. Rather than imposing the code style settings on the document, impose the document settings
on the code style.

Since the existing CodeStyle isn't extensible (or mutable), change this by making it non final, and create our own subclass
which basically performs a document lookup rather than a NbPreferences lookup.  

The vital code here is the part which computes the indent size. The logic is copied directly from the editor options
dialog; it looks up the XML editor kit and asks for its tab size. This will be considered the IDE global indentation width
value.


+    private static final class DocSyncedCodeStyle extends CodeStyle {
+        private DocSyncedCodeStyle() {
+            super(null);
+        }
+        
+        private int indentSize = 4;
+        private int continuationIndentSize = 4;
+        private int rightMargin = 80;
+        private EditorKit kit;
+        
+        // Copied from option.editor's org.netbeans.modules.options.indentation.IndentationModel
+        private EditorKit getEditorKit() {
+            if(kit == null)
+                kit = MimeLookup.getLookup(MimePath.parse("text/xml")).lookup(EditorKit.class); // NOI18N
+            return kit;
+        }
+
+        // Copied from option.editor's org.netbeans.modules.options.indentation.IndentationModel
+        private Integer getSpacesPerTab() {
+            Integer sp =
+                    (Integer) Settings.getValue(getEditorKit().getClass(), SettingsNames.SPACES_PER_TAB);
+            return sp;
+        }
+        
+        public void syncToDocument(BaseDocument doc) {
+            Integer sp = getSpacesPerTab();
+            int indent = sp.intValue();
+            if (indent <= 0) {
+                indent = 4;
+            }
+            indentSize = continuationIndentSize = indent;
+        }
+        
+        @Override
+        public int getIndentSize() {
+            return indentSize;
+        }
+
+        @Override
+        public int getContinuationIndentSize() {
+            return continuationIndentSize;
+        }
+
+        @Override
+        public boolean reformatComments() {
+            // This isn't used in JavaScript yet
+            return false;
+        }
+
+        @Override
+        public boolean indentHtml() {
+            // This isn't used in JavaScript yet
+            return false;
+        }
+
+        @Override
+        public int getRightMargin() {
+            return rightMargin;
         }
     }


CodeStyle wasn't extensible, so make it so:

diff -r d7c458cefd44 javascript.editing/src/org/netbeans/modules/javascript/editing/CodeStyle.java
--- a/javascript.editing/src/org/netbeans/modules/javascript/editing/CodeStyle.java	Tue Apr 08 17:16:25 2008 +0400
+++ b/javascript.editing/src/org/netbeans/modules/javascript/editing/CodeStyle.java	Tue Apr 08 11:44:37 2008 -0700
@@ -54,7 +54,7 @@
  * 
  * @author Dusan Balek
  */
-public final class CodeStyle {
+public /*final*/ class CodeStyle {
     
     private static CodeStyle INSTANCE;
 
@@ -64,7 +64,7 @@
     
     private Preferences preferences;
     
-    private CodeStyle(Preferences preferences) {
+    protected CodeStyle(Preferences preferences) {
         this.preferences = preferences;
     }
 


Modify the Formatter to initialize one of our new document-synchronized code styles instead of
the old Preferences-based one when a Formatter is constructed


@@ -98,7 +104,8 @@
     private Stack<StackItem> stack = new Stack<StackItem>();
 
     public JsFormatter() {
-        this.codeStyle = CodeStyle.getDefault(null);
+        this.codeStyle = new DocSyncedCodeStyle();
     }
     
     public JsFormatter(CodeStyle codeStyle, int rightMarginOverride) {


And finally modify the old sync-options to document call to perform the reverse instead:

     /** Compute the initial balance of brackets at the given offset. */
@@ -865,9 +875,69 @@
      * those settings
      */
     private static void syncOptions(BaseDocument doc, CodeStyle style) {
-        org.netbeans.editor.Formatter formatter = doc.getFormatter();
-        if (formatter.getSpacesPerTab() != style.getIndentSize()) {
-            formatter.setSpacesPerTab(style.getIndentSize());
+        if (style instanceof DocSyncedCodeStyle) {
+            ((DocSyncedCodeStyle)style).syncToDocument(doc);
         }
     }



Limitations of the fix:
This fix will fix the indentation widths when you format your code, and for smart indent (e.g. pressing return
and having the caret indent to the next level if necessary).  It will NOT fix the indentation size for
pressing Tab inside the document (go to the next visual column). The insert tab action calls formatter.getSpacesPerTab
directly and I haven't found a safe way to force that value to something kit-type dependent.

The root difficulty with all this (besides the issue of having a global indent setting in the IDE) is that I
cannot initialize editor settings per mime type. This is a well known issue:
http://www.netbeans.org/nonav/issues/show_bug.cgi?id=114747
http://www.netbeans.org/nonav/issues/show_bug.cgi?id=90403
GSF, like Schliemann has a single editor kit implementation serving multiple languages with different indentation
defaults (JavaScript=4, Ruby=2, ...) and there's no way to initialize the associated settings object to different
values since the settings map is stored for each editor kit. Unless Vita can think of a good workaround for now...

The workaround for this is to edit the editor options; this will override the default 2 and set it to whatever
you want.