Fix for indentation behaviour

[newsiberian]

Hi, fix for #31 Also added new argument for indentation depth for onTabs, handleReturn;

[coveralls]

Coverage decreased (-1.3%) to 97.917% when pulling bb682a4422bc8489add4d6f5a2624da296b4b020 on newsiberian:fix-31 into c219dc50d42f7db9c69b4b8c9be4b437116e8e6a on SamyPesse:master.

[coveralls]

Coverage decreased (-1.3%) to 97.917% when pulling 9928552e382b10c071347f189809d3e27b706f9a on newsiberian:fix-31 into c219dc50d42f7db9c69b4b8c9be4b437116e8e6a on SamyPesse:master.

[newsiberian]

If you try to press TAB here several times it will indent on 4, then 4, then 8 and 16 spaces, etc. I'm don't understand the logic behind getIndentation

[mxstbr]

The logic behind getIndentation is that not everybody indents with two spaces.

Imagine code like this, | is the cursor position:

function bla2(x) {
  const a = "b";
  |
}

When a user presses tab now, this is what should happen:

function bla2(x) {
  const a = "b";
    |
}

The cursor is indented by two spaces, because they use two spaces for indentation.

Now imagine this code:

function bla4(y) {
    const b = "c";
    |
}

What happens now when a user pressed tab? (note the four spaces indentation) Ideally it indents by four spaces, since the user uses four spaces for indentation:

function bla4(y) {
    const b = "c";
        |
}

I think that's what the getIndentation function was for, detecting that? Maybe? (I wasn't the original author of the package so not 100% on the original intentions) Anyway, that's the way it should work but by hardcoding the 2 there we can't make that work... :confused:

[newsiberian]

Ok, again, if I understand you correctly we could have the following: Let's say the default is 4 spaces case 1:

function () {
    | // <-- in that case we could use default or preset indent

case 2:

function () {
  var x = 1;
  | // <-- we are looking on previous line with 2 spaces and use
// this 2 spaces-indent for the current line

case 3: at the same time

function () {
    var y = 2;
    | // <-- and again here (even if several line above we were using 2
// spaces indent) we are looking on previous line indent and using it for this line

Is this correct?

[mxstbr]

Yep, we just choose some default and then try and smartly guess the indentation preferences of the user based on their previous code in the code block! So rather than the user saying "I want everybody to type with 2 or 4 spaces indentation" we just take care of it. That'd be ideal, right?

[newsiberian]

ok, now I get it, but can we leave new argument? because now we have only 4 spaces by default, which is not cool, so we will have something like:

var DEFAULT_INDENTATION = '    ';

function getIndentation(text, indent) {
  var result = detectIndent(text);
  return result.indent || indent || DEFAULT_INDENTATION;
}

[mxstbr]

Let's just make 2 the default, I prefer that anyway, and then infer from the user if possible.

[andria-dev]

@mxstbr @SamyPesse is there any way we could merge this? Or should I open up a PR that allows a set indentation size while defaulting to inferring the correct indent size?

[newsiberian]

@ChrisBrownie55, hi, check my fork. It is published and probably has a fix for that use case