Indentation issue when using tabs and tabSize

[josdejong]

I came across an issue when using a document indented with tabs: the first indentation level doesn't respect the configured tabSize when using codemirror-wrapped-line-indent

You can try it out in this demo: https://stackblitz.com/edit/codemirror-wrapped-line-indent-issue-with-tabs?file=main.js&terminal=dev

Basically:

import { basicSetup, EditorView } from 'codemirror';
import { EditorState } from '@codemirror/state';
import { wrappedLineIndent } from 'codemirror-wrapped-line-indent';
import './style.css';

const doc = `{
\t"nested": {
\t\t"value": 42
\t} 
}
`;

const tabSize = 8;

window.editor = new EditorView({
  doc,
  extensions: [
    basicSetup,
    EditorState.tabSize.of(tabSize),
    wrappedLineIndent, // remove this extension -> first indentation is correct then
  ],
  parent: document.getElementById('app'),
});

With codemirror-wrapped-line-indent enabled, the first indentation level is wrong:

When disabling the extension, the indentation is correct:

[fauzi9331]

Hi @josdejong thank you for reporting this.

I see that there are 2 issues causing this:

First, the function getIndentSize doesn't accurately calculate the indent size because \t is counted as 1 character. This is fixable.

Second, text-indent doesn't work if the whitespaces at the start of the text contain \t. I'm still trying to find how to fix this. At worst, this is unfixable.

Do you already have workaround at your end? maybe have option to disable this extension? If not, I can help with that

[josdejong]

Thanks for looking into it!

I don't have a workaround yet, but it is only an esthetic problem so no showstopper. As a workaround I could for example disable disable codemirror-wrapped-line-indent when tabs are used.

[josdejong]

I did some testing, and it looks like this will be solved by just fixing getIndentSize: when I configure my editor with indentation: '\t' and tabSize: 8, and I change getIndentSize (hard coded) to multiply with tabSize, all works like a charm as far as I can see:

    getIndentSize(line) {
        return (line.text.length - line.text.trimStart().length) * tabSize;
    }

Now, of course the method needs to check whether the indentation is tabs and only then multiply with the actual tabSize, but besides that all seems to work fine.

[fauzi9331]

@josdejong thank you for trying it out. I tried to implement it. But I still see that the issue still persists.

https://stackblitz.com/edit/codemirror-wrapped-line-indent-issue-with-tabs-jrekfw?file=main.js

[josdejong]

Ahh, that is interesting. I was testing in Firefox, there your example works as it should:

So, apparently, this is a Chrome specific issue?

[fauzi9331]

I think so, text-indent works differently between firefox and chrome. It actually complicate things. Firefox will still have issue if indentUnit and tabSize are different.
This is how it looks like in firefox with indentUnit = 2 and tabSize = 4

[josdejong]

Isn't that because getIndentSize doesn't yet have logic to only multiply with tabSize when the indentation is \t?

[fauzi9331]

hmm, I don't think so, because the doc is still using \t for all its indentation. the getIndentSize should still calculate correctly

[josdejong]

If you mix config and actual indentation, things will always look weird. I think you shouldn't mix it:

• If the document contains \t, the indentUnit should be \t
• If the document contains say 4 space indentation, the indentUnit should be ' '.

I was testing with something like this:

import { basicSetup, EditorView } from "codemirror";
import { EditorState } from '@codemirror/state';
import { indentUnit } from '@codemirror/language';
import { wrappedLineIndent } from "../src/index";

const indentation = '    '  // 4 spaces
// const indentation = '\t' // tab
const tabSize = 8;

const data = {
  "deep": {
    "nested": {
      "value": "verrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrryyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyylooooooooooooonnggg"
    }
  }
}

const doc = JSON.stringify(data, null, indentation)

const editor = new EditorView({
  doc,
  extensions: [
    basicSetup,
    EditorState.tabSize.of(tabSize),
    indentUnit.of(indentation),
    wrappedLineIndent
  ],
  parent: document.getElementById("app")!,
});

[fauzi9331]

That's actually a nice constraint. I have published the fix that can works with \t' or ' ' but firefox only for now. the version is 1.0.5

https://stackblitz.com/edit/codemirror-wrapped-line-indent-issue-with-tabs-jrekfw?file=main.js

I will update again when I found how to fix it for chrome

[josdejong]

Thanks!

[josdejong]

I've now updated to v1.0.5 and have been doing testing in both Firefox and Chrome, but I can't reproduce an issue anymore 🤔. Do you still have a case that fails in Chrome?

[fauzi9331]

@josdejong I see the issue on both browser 🥲

chrome:

firefox:

I modified the parser a little so it add indent with \t instead of space

[fauzi9331]

after a closer look I noticed that the default config has different value between indentation and tabSize

after equalizing the values, the issue disappears in firefox, but still appears in chrome

[josdejong]

Ahh, yes.

So to reproduce:

1. Open the example /src/routes/examples/basic_usage_one_way_binding/

2. Configure indentation to be a tab:

   <JSONEditor {content} onChange={handleChange} indentation={'\t'} /> 

3. Start the dev server, and open the example in Chrome, there, the editor is jumpy and it looks like the editor is not sure whether the tab size is 4 or 8 characters wide:

   

4. When I configure the editor with tabSize={8}, all works like a charm. Can it be that we somewhere do not correctly propagate the configured tabSize?
   EDIT: hm, but if that is the case, it should happen in both Chrome and Firefox 🤔

[fauzi9331]

@josdejong I applied browser engine detection to do different calculation on the text-indent value. It looks good so far, could you try it at your end. the version is 1.0.8

[josdejong]

Thanks for working on a fix! It's unfortunate if there is a switch for different browsers needed. I had hoped we had that time behind us now that IE is retired 😅.

I did some testing on Firefox, Chrome, and Edge (wasn't able to test on Safari). On Chrome and Edge there still is some flaky behavior, see the following gif: