Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TSX indentation is broken due to update of 'xml' filetype indentation logic #224

Open
rhysd opened this issue Nov 23, 2020 · 2 comments
Open

Comments

@rhysd
Copy link
Contributor

rhysd commented Nov 23, 2020

Repro

Let's say to open the following lines with filetype=typescriptreact:

class MyComponent {
    public render() {
    return (
    <Table disableHeader={false}
        key1="123123"
        key2="423123"
    headerHeight={50}
    rowCount={this.state.data.length}
    ref="Table"
    rowGetter={(row) => this.state.data[row.index]}
    rowHeight={30}>
    <Column
    label="Age"
        rowHeight={30}
    width={90}
    height={100} />
    <Column
    width={210}
    label="Title"
    height={100}
    />
    <Column
    width={210}
        label="Title"
    height={100} />

    </Table>
    );
        }
}

This was taken from test case in test/tsx.indent.vader.

And type gg=G to apply indentation.

Expected behavior

Formatted as follows:

class MyComponent {
    public render() {
        return (
            <Table disableHeader={false}
                key1="123123"
                key2="423123"
                headerHeight={50}
                rowCount={this.state.data.length}
                ref="Table"
                rowGetter={(row) => this.state.data[row.index]}
                rowHeight={30}>
            <Column
                label="Age"
                rowHeight={30}
                width={90}
                height={100} />
            <Column
                width={210}
                label="Title"
                height={100}
            />
            <Column
            width={210}
            label="Title"
            height={100} />

            </Table>
        );
    }
}

Actual behavior

Formatted as follows:

class MyComponent {
    public render() {
        return (
            <Table disableHeader={false}
            key1="123123"
            key2="423123"
            headerHeight={50}
            rowCount={this.state.data.length}
            ref="Table"
            rowGetter={(row) => this.state.data[row.index]}
            rowHeight={30}>
            <Column
            label="Age"
            rowHeight={30}
            width={90}
            height={100} />
            <Column
            width={210}
            label="Title"
            height={100}
        />
                <Column
                width={210}
                label="Title"
                height={100} />

            </Table>
        );
    }
}

Analysis

As you see, the line after start of XML element does not increase indent. For example, the indent level at key1="123123" should be increased but is not increased actually.

I tried to fix this but found that it is hard to fix. I think continuing the current implementation is not possible. Let me explain.

yats.vim uses xml filetype logic for indenting XML part of TSX. It calls XmlIndentGet() which is defined in vim/runtime/indent/xml.vim:

https://github.com/vim/vim/blob/792f786aad8409ca9ab895392742643a5b6aed8f/runtime/indent/xml.vim#L103

When it is called with the line key1="123123" for example, it finally calls the below line of s:IsXMLContinuation():

https://github.com/vim/vim/blob/792f786aad8409ca9ab895392742643a5b6aed8f/runtime/indent/xml.vim#L152

As you see, this line only considers the case where filetype is xml. It means that setting typescriptreact to filetype does not work and the XML indentation logic can no longer recognize the start of XML element with continuation (like <foo). The &ft is# 'xml' check was added 12mo ago (though I don't know the reason):

vim/vim@4ceaa3a#diff-166891d5fe142e1f046368eb5a2150188d8d63eb415a08a46b6ed1ed89aa9994L151-R152

To fix this, we need to change runtime/indent/xml.vim. However, xml.vim is logic for XML and does not consider to be used by any other filetype logics. I don't think the &ft is# 'xml' line can be changed only for fixing yats.vim.

IMO, typescriptreact indentation should not depend on XML indentation logic. For example, vim-jsx-pretty has its own logic to indent XML part. yats.vim also can do the similar thing. Reusing vim-jsx-pretty's logic might be good idea. I created this issue instead of making PR directly because the fix would make large changes and some decision making would be necessary.

@rhysd
Copy link
Contributor Author

rhysd commented Nov 23, 2020

Note: I guess Travis CI does not fail because older Vim is used for testing. (It's Ubuntu 16.04)

@timoxley
Copy link

timoxley commented Feb 9, 2021

Bump, this is very broken right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants