Bug 132363 - JavaScript indentation ignores the IDE indentation settings!
JavaScript indentation ignores the IDE indentation settings!
Status: RESOLVED FIXED
Product: javascript
Classification: Unclassified
Component: Editor
6.x
Macintosh All
: P1 (vote)
: 6.x
Assigned To: Torbjorn Norbye
issues@javascript
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-08 20:11 UTC by Torbjorn Norbye
Modified: 2009-02-27 14:26 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Patch against release61 (5.68 KB, patch)
2008-04-08 20:14 UTC, Torbjorn Norbye
Details | Diff
Built release61 versions of the gsf and javascript editing modules (since trunk has changed a lot from release61, you don't want to verify there) (785.50 KB, application/x-compressed)
2008-04-08 20:17 UTC, Torbjorn Norbye
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Torbjorn Norbye 2008-04-08 20:11:12 UTC
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.
Comment 1 Torbjorn Norbye 2008-04-08 20:14:08 UTC
Created attachment 59862 [details]
Patch against release61
Comment 2 Torbjorn Norbye 2008-04-08 20:17:12 UTC
Created attachment 59863 [details]
Built release61 versions of the gsf and javascript editing modules (since trunk has changed a lot from release61, you don't want to verify there)
Comment 3 Torbjorn Norbye 2008-04-08 20:23:48 UTC
This is fixed in the trunk with changeset a70c6d847c5e.

However, use the diff attached to this issue rather than the above changeset to read the diffs since the formatter code
in the trunk is slightly different than what is in release61, so the patch isn't identical.
Comment 4 Martin Schovanek 2008-04-08 23:36:14 UTC
verified in release61
Comment 5 Martin Schovanek 2008-04-09 09:27:37 UTC
Tor can you integrate in the release61 branch please.
Comment 6 Torbjorn Norbye 2008-04-09 14:55:19 UTC
Fixed in 6.1 with changeset 7314b2c39ba9.
Comment 7 Martin Schovanek 2008-04-09 22:35:21 UTC
Verified in release61.
Comment 8 Torbjorn Norbye 2008-04-10 15:16:50 UTC
*** Issue 132411 has been marked as a duplicate of this issue. ***
Comment 9 Marian Mirilovic 2008-08-01 11:20:14 UTC
move back to consistent state RESOLVED/FIXED


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo