tamzinblake / js3-mode

A chimeric fork of js2-mode and js-mode
GNU General Public License v3.0
181 stars 13 forks source link

Continued assignment expression indentation #84

Open 0xc22b opened 10 years ago

0xc22b commented 10 years ago

There are (at least) 2 types of continued expressions.

goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function(el,
    opt_pos) {
  var corrected = this.correctedWords_ &&
      this.correctedWords_.contains(this.activeWord_);
}

One is at the second line which is inside parentheses, in this case. We can use syntax-ppss to get the position of the opening parenthesis and back-to-indentation to get the starting point of indentation (current-column).

Another type is the line before the last one. We could not use the syntax-ppss as the starting point should be at 'var', not 'goog'.

Proposed solution

  1. Make sure the line is a continued expression (js3-continued-expression-p is t)
  2. Try to find the position of '='
  3. Compare the position of '=' with the position of the opening parenthesis from syntax-ppss
  4. If the '=' is closer, it means this is the second type, find starting point of indentation from that line.

Please see the code here. https://github.com/witterk/js3-mode/commit/05ea437419b69c0e978f32344f3c7ba1d5bdf1a2

I called a function (js3-assignment-expr-indentation) to find an indentation of a continued assignment expression twice. It would make the program slow.

I'm not sure I should check js3-continued-expression-p is true in the function as it's kind of a prerequisite.

Please feel free to advise.

tamzinblake commented 10 years ago

I'm not sure what problem you're trying to solve here. The general model of js3-mode is to use the AST to perform indentation. Maybe with some samples of what you're seeing and what you'd like to see instead?

0xc22b commented 10 years ago

What I'm seeing now is

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function() {
2.   var corrected = this.correctedWords_ &&
3.     this.correctedWords_.contains(this.activeWord_);
4. }

What I'd like to see is

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function() {
2.   var corrected = this.correctedWords_ &&
3.       this.correctedWords_.contains(this.activeWord_);
4. }

The difference is the indentation at line no. 3. It's an continued expression.

I set the value of js3-continued-expr-mult to be 1, js3-indent-level is 2, js3-expr-indent-offset is 2 which mean, in my understanding, the indentation of line no. 3 should be 4 spaces from the previous line (Please correct me if I'm wrong).

Now the indentation is only 2 spaces instead of 4 spaces.

tamzinblake commented 10 years ago

js3-expr-indent-offset is what determines the offset between var and this. js3-indent-level is only used to indent var. So when calculating where to indent this, it goes back to indentation for the previous expression (goog), adds js3-indent-level to get to where var is, and adds js3-expr-indent-offset to offset this from var.

Off the top of my head, anyway. Going to have to mess with some test cases.

0xc22b commented 10 years ago

Let me try again.

Here is what I'm seeing now.

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function(el,
2.     opt_pos) {
3.   var corrected = this.correctedWords_ &&
4.     this.correctedWords_.contains(this.activeWord_);
5. }

What I'd like to see is

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function(el,
2.   opt_pos) {
3.   var corrected = this.correctedWords_ &&
4.     this.correctedWords_.contains(this.activeWord_);
5. }

Line no. 2 and line no. 4 are continued expressions. With the setup above, they both have the same indentation which is 4 spaces.

I guessed that the indentation of line no. 2 should have 2 spaces instead.

tamzinblake commented 10 years ago

If you use the default settings and then set js3-boring-indentation to t, then you should get exactly what you have in the second example. Can you post your custom settings for js3-mode?

0xc22b commented 10 years ago

Here my custom settings.

(custom-set-variables
 ;; custom-set-variables was added by Custom.
 ;; If you edit it by hand, you could mess it up, so be careful.
 ;; Your init file should contain only one such instance.
 ;; If there is more than one, they won't work right.
 '(js3-auto-indent-p nil)
 '(js3-auto-insert-catch-block t)
 '(js3-boring-indentation t)
 '(js3-cleanup-whitespace nil)
 '(js3-compact nil)
 '(js3-consistent-level-indent-inner-bracket t)
 '(js3-continued-expr-mult 1)
 '(js3-curly-indent-offset 0)
 '(js3-enter-indents-newline nil)
 '(js3-expr-indent-offset 2)
 '(js3-indent-on-enter-key nil)
 '(js3-lazy-commas nil)
 '(js3-lazy-dots nil)
 '(js3-lazy-operators nil)
 '(js3-paren-indent-offset 2)
 '(js3-pretty-lazy-vars nil)
 '(js3-pretty-vars nil)
 '(js3-reparse-on-indent nil)
 '(js3-square-indent-offset 0)
 '(js3-strict-missing-semi-warning t)
 '(js3-strict-var-hides-function-arg-warning t))
tamzinblake commented 10 years ago

Well, it's being caused primarily by js3-paren-indent-offset being set to 2. That adds 2 additional spaces to the indentation of parenthesized lists.

0xc22b commented 10 years ago

I've just found out that the line no. 2 in the example above is not considered to be a continued expression. I'm sorry I gave you the wrong example.

Let me give you another one.

Now here is what I'm seeing.

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function(el,
2.   opt_pos) {
3.   var corrected = this.correctedWords_ &&
4.     this.correctedWords_.contains(this.activeWord_);
5.   if (corrected.substring(0, 7) !== 'http://' &&
6.       corrected.substring(0, 8) != 'https://') {
7.     corrected = 'http://' + corrected;
8.   }
9. }

What I'd like to see is

1. goog.ui.AbstractSpellChecker.prototype.showSuggestionsMenu = function(el,
2.   opt_pos) {
3.   var corrected = this.correctedWords_ &&
4.     this.correctedWords_.contains(this.activeWord_);
5.   if (corrected.substring(0, 7) !== 'http://' &&
6.     corrected.substring(0, 8) != 'https://') {
7.     corrected = 'http://' + corrected;
8.   }
9. }

The difference is the indentation at line no. 6.

In my opinion, as both line no. 4 and line no. 6 are continued expressions, they should have the same indentation.

I've set the value of js3-paren-indent-offset to be 0 but as I debugged, it's not being used in this case.

I couldn't figure it out how to set it up or it was designed to be like this. Could you help?