Wrong indentation for ternary ifs

[sassanh]

Please take a look at this comment and comments following it: pangloss/vim-javascript#618 (comment)

The fix we found there breaks the ternary ifs out of JSXRegions. I guess it's better solved in vim-jsx as there isn't a clean way to solve this in vim-javascript.

[mxw]

Can you summarize the issue? I can't repro this—is my vim-javascript too old?

[sassanh]

Consider these lines:

const a = <div
    style={{
        paddingTop: '.2em',
        position: 'relative',
        minHeight: 'calc(3.2em - .2em)',
        pointerEvents: 'none',
        ...(Radium.getState(this.state, this.props.id, ':hover')
            ? {
                zIndex: '1',
                backgroundColor: this.props.selected
                    ? '#a0c4da'
                    : '#cccccc', // <---------- Another line which currently has correct indentation
                border: '1px solid #999999',
            }
                : {}    // <----------------------------   Wrong Indentation
        ),
    }}
    >
    123
</div>
const c = true
    ? 1
    : 2;  // <----------------------------------- Another test case

The goal is to make all lines marked above indent correctly. It started to indent wrong for me around 10 days ago updating vim-javascript. I'm using develop branch of vim-javascript so if you're using master you shouldn't be able to reproduce it. To use develop branch as you may know it's as easy as:

Plug 'pangloss/vim-javascript', { 'branch': 'develop' }

If you're using vim-plugged. Other package managers should provide similar functionality.

You can consider using branch jsx instead of developas there's a new fix for jsx files there.

[bounceme]

thats intended behaviour and not related to vim-jsx,
...(Radium.getState(this.state, this.props.id, ':hover') opens a new 'scope'. lines contained in the paren started in this line will be indented one sw . : is an operator and increases that by one sw if it isn't the very next line after the start of an opened 'scope' (to prevent double indent,which would look strange)

you can customize this using the global variables of the js indent script

[sassanh]

@bounceme I just installed Atom to see how it indents this and it indented all 3 marked lines as desired (except that it doesn't add an indentation level for ternary if, it may be in its configuration, but its result is consistent with its own rules.) The above jsx code is just a valid jsx code, it should indent without setting any configuration. Even if it needs some config to get indented, the config shouldn't break anything else (that's why this config that doesn't break anything else can be included in the core module.) I thought it's related to vim-jsx but if you're sure it's not related to vim-jsx and you know about a pure js code sample that doesn't gets indented as desired, then I guess we should fix this in vim-javascript itself.

I think in this particular example we should decrease indent level after the scope of ? is terminated so that when : increases it again it gets same level of indent as ?. I guess currently the scope of ? contains the scope of : too, but I think they should get separated.

[sassanh]

After all I think to fix the problem with this sample:

const a = true
    ? <div>
        1234
    </div>
    : <span>4321</span>;

We need to handle it in vim-jsx because as far as I understood vim-javascript adds an indentation level after > and another for : and then reduces 2 indents to one to avoid "strange" double indent. Even if we solve the problem of : in vim-javascript, > still adds an indentation level which I guess can't be avoided in vim-javascript easily because it doesn't understand the html tag and it should be an exception handled in vim-jsx like it already has some exceptions in GetJSXIndent().

[bounceme]

(sorry @mxw for the off topic)
my result, ft=javascript

// sw=8
const a = true
        ? <div>
        1234
        </div>
        : <span>4321</span>;

what i mean with strange indent is slightly conceptual. if I were to change the behaviour to have 'strange' results, things like this could result from that logic

//sw=2
(((function(){
        // 4 two space indents
})))

and your jsx example would change to something like this(increased indent in the ternary):

//sw=4
const a = <div
    style={{
        paddingTop: '.2em',
        position: 'relative',
        minHeight: 'calc(3.2em - .2em)',
        pointerEvents: 'none',
        ...(Radium.getState(this.state, this.props.id, ':hover')
                ? {
                    zIndex: '1',
                    backgroundColor: this.props.selected
                        ? '#a0c4da'
                        : '#cccccc', // <---------- Another line which currently has correct indentation
                    border: '1px solid #999999',
                }
                : {}    // <----------------------------   Wrong Indentation
        ),
    }}
    >
    123
</div>
const c = true
    ? 1
    : 2;  // <----------------------------------- Another test case

your other issues all are fixable by customizing the continuation regex (if i understand right)

[bounceme]

Unless if your issues are occurring only with jsx, in which case i'll (or vim-javascript) not fix it

[sassanh]

@mxw I'm sorry for the parts of the conversation that are off-topic too, but parts of it are related to vim-jsx too.

@bounceme I don't think things like these are should be configured, you can configure the sw size, it'd be nice if there were a configuration to determine if the user wants indentation for ternary if at all for example or things like that. But the above snippet is either correct or incorrect. I guess no one likes to have a configuration that indents : one level ahead of ?. So I think this should be avoided at all, without any config requirement.

I didn't get it, after all do you think my code snippet should be indented * by default * the way I marked "Wrong" or should the behavior change?

If it should change, then we should determine in which plugin the change should happen, if you think it's alright the way it is then I was mistaken about what vim-javascript is trying to do. I thought mission of vim-javascript's indentation module is to indent the code the way all other editors do and is generally accepted. If you as the repo owner think it should indent in another way then I totally respect that and I still appreciate your work as it already is helpful a lot for me.

[bounceme]

editors with js indentation are all going to differ, but any that are to be considered correct must indent(or be configurable to indent) when an expression spans more than one line. There is no other logical way to deal with expressions. If you would like to illustrate 'every other editors' indentation please do.

One reference for an awful approach to indentation is emacs js2-mode . It is thousands of lines, built on top of an outdated Rhino parser and the emacs c-logic (so all things newer than es5 are basically imprecise elisp hacks), and still manages to make bad indentation using the assumptions of it's parser ( which are mostly hard coded and not configurable).
http://steve-yegge.blogspot.com/2008/03/js2-mode-new-javascript-mode-for-emacs.html
syl20bnr/spacemacs#4082
https://github.com/thomblake/js3-mode

Lastly, i'm not the repo owner at vim-javascript, and i'm glad to hear you have profited from my toil atleast a little

[mxw]

No worries—I'm not really paying attention :P

Can someone summarize? Is this "issue" with vim-jsx specifically, or vim-javascript more generally?

[bounceme]

sorry, I don't really know haha.

I didn't get it, after all do you think my code snippet should be indented * by default * the way I marked "Wrong" or should the behavior change?

yes what you marked wrong should not be changed. Maybe that resolves this issue?

[sassanh]

Well as you already said emacs' indentation is awful, lets not reference awful ones as we want to have the perfect one, or at least move towards having it. After downloading eclipse, komodIDE, sublime and WebStorm. I can confirm that vim-jsx and vim-javascript are among bests. Out of these ides only Atom indents jsx alright. My vim's indentation is absolutely best after Atom.

But after all I guess no one expects his editor to indent ternary if like this:

const a = true
    ? 1
        : 2;

or this:

const a = true
        ? 1
        : 2;

vim doesn't indent above specific codes incorrect but in some contexts (like the code I provided above) it indents incorrectly (incorrect as no one wants it that way.)

[mxw]

I'm closing this issue because I have yet to get a clear explanation of the problem.

If this is a problem with vim-javascript, please file an issue there. If this is a problem with vim-jsx, please also provide a comparison with the behavior you see outside of a JSX region (but perhaps with a similar degree of nesting).

If this is a problem with vim-jsx only with the dev branch of vim-javascript, then please ping this issue (or create a new issue) once the dev branch has become the master branch. vim-jsx doesn't have a dev branch (and doesn't need one, being so simple), so I have no reasonable way of targeting a beta dependency.

[sassanh]

@mxw OK, I guess we should reopen this with this explanation:

Using vim-jsx and vim-javascript both master branch, this is how it gets indented in vim:

const a = true
    ? 1
    : 2;
const b = <div>
    {true
        ? 1
            : 2
    }
</div>;

So normal js is indented alright (similar degree of nesting.) while jsxRegion is not indented correctly.