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

Incorrect indentation for single line if/for/while/etc, multiline chaining statements etc #43244

Open
johanolofsson opened this issue Feb 8, 2018 · 84 comments · May be fixed by #115454
Open
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-autoindent Editor auto indentation issues javascript JavaScript support issues on-unit-test typescript Typescript support issues
Milestone

Comments

@johanolofsson
Copy link

johanolofsson commented Feb 8, 2018

I am experiencing some weird indentation issues. Not using any plugins to prettify or similar.

if (condition) {
    return; // <-- correct indentation
}
if(condition)
return; // <-- incorrect indentation

Some ppl claim this should be avoided. I don't agree. Using pair of { } for simple early exits messes up code. Putting the return on the end of the same line can be hard to read if the condition is long.
This is not exclusive to return. Applies to all single lines.

same problems apply to for, while, etc.

var transformedValues =
originalValues <-- incorrect should be indented one level.
.where(condition) <-- incorrect, should be indented two levels.
.select(transform); <-- incorrect, should be indented two levels. 

function someFunction() {
    callToSomeOtherFunction(
         variableWithLongNameWhichRequiresASeparateLine,
         anotherVariableWithLongNameWhichRequiresASeparateLine);
    }  <-- incorrect should not be indented.
    function ... <-- incorrect the rest of the file is indented. 
@vscodebot vscodebot bot added editor editor-autoindent Editor auto indentation issues labels Feb 8, 2018
@mjbvz mjbvz self-assigned this Feb 8, 2018
@mjbvz mjbvz added the javascript JavaScript support issues label Feb 12, 2018
@johanolofsson
Copy link
Author

Thx for picking this up! Btw, it applies to typescript as well.

class Foo {
    public bar(x: number): void {
        if(x == 0)
        return; // <-- incorrect
    }
}

@Spongman
Copy link

Spongman commented Apr 11, 2018

also related, after typing, the code looks like this:

if (condition)
return;

but after 'Format Document' the indentation is correct:

if (condition)
  return;

but if you put the cursor after the ; and hit ENTER, the cursor is incorrectly indented:

if (condition)
  return;
  |  <-- cursor is here, should be in column zero.

image

@rickrankin
Copy link

This also applies C/C++/D, as well. Probably anything derived from C syntax.

@ahfeel
Copy link

ahfeel commented May 22, 2018

Also applies to PHP, very very frustrating :( Anybody figured out a solution or an extension that fixes this ?

@johanolofsson
Copy link
Author

None as far as I know, the fact that this has not yet been addressed is kind of weird. Seems to me like a very fundamental part of a decent code editor.

I'm starting to wonder if this is actually a case of opinion where the responsible developers simply don't agree with this way of avoiding { } and therefore just happens to forget about it? Microsoft, pls prove me wrong on this!

@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label Sep 11, 2018
@stanislavdavid
Copy link

There is even bug with this code:

if (condition) { some code in 1 line }
    next command wrongly intended

@rebornix rebornix added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Sep 19, 2018
@stanislavdavid
Copy link

when is condition in comment, then it is even not working )

// if (condition) {

@hleVqq
Copy link

hleVqq commented Nov 5, 2018

How is this taking so long to fix? I wanna use VSCode in place of Sublime, but c'mon :/

@Spongman
Copy link

Spongman commented Nov 5, 2018

@rebornix can you explain why you do not consider this a bug?

@Spongman
Copy link

Spongman commented Nov 5, 2018

here's another weirdness.

  1. new file
  2. set filetype to 'JS'
  3. type if(true)<ENTER>

after hitting <ENTER> the cursor drops to line 2, column 1. this is clearly wrong. the cursor at this point should be indented, and only un-indented if the next keystroke is a {.

image

however...

  1. new file
  2. hit <ENTER> several times
  3. go to line 1
  4. type if(true)<ENTER>

now the cursor ends up indented correctly:

image

@fbricon
Copy link
Contributor

fbricon commented Nov 6, 2018

So I opened #62198 for a related issue but it was closed as duplicate of this one (I'm not convinced this is the exact same issue though), so I'll repeat the specifics here:

Create a javascript file containing an if statement without brackets, one line executed if the if statement is valid, then 2 unrelated lines next

var i = 1;
if (i === 1)
    console.log('log from if');

console.log('line one');
console.log('line two');
  1. invert the last 2 lines with alt+up or alt+down
  2. lines are indented under the if statement

indentation-issue-when-moving-lines

And now for the really fun part: on Mac, the same issue exists in 1.28.2, BUT works correctly in 1.29.0-insider:
oct-30-2018 16-24-12

We found the issue while trying to fix a similar problem in vscode-java, after copying the indentation rules from the js/ts server.

@lpaladin
Copy link

It seems that when I turn on the 'editor.formatOnType' setting, this issue will go away. Maybe correct indentation is implemented as a code formatting feature, and relies on specific language support.

@hleVqq
Copy link

hleVqq commented Nov 15, 2018

@zhouhaoyu it still has weird behavior, like:

function Test(x, y)
{
    if (x)
        return y;
    
    if (y)
    {
        
        }
}

The setting also makes no difference in other languages, like C++.

@lpaladin
Copy link

@hleVqq I guess there's a conflict between the auto indent feature and language-specific formatting then.
Hope this could be fixed soon...

@stanislavdavid
Copy link

Why this is labelled as feature request and bug label was removed. It is bug at the first glance

@rickrankin
Copy link

Why this is labelled as feature request and bug label was removed. It is bug at the first glance

@stanislavdavid, I agree that, at least from the outside, this looks like a bug and not a feature request. There's an existing auto-indent feature and it doesn't work correctly, at least in some circumstances. That, to me, is the definition of a bug. I have some speculative reasons why this might be considered a feature request rather than a bug, but it would be really nice to hear from one of the folks to whom this is assigned.

It's been almost a year, now, since this issue was opened with no apparent activity outside of others commenting on new cases where it doesn't work correctly. This is an important enough issue to me that frankly, I'm just about to the point of giving up on VSCode and going back to one of the other editors that auto-indents correctly. That would be a shame, because there's a lot I like about VSCode, but this is really frustrating.

@fbricon
Copy link
Contributor

fbricon commented May 2, 2019

The behavior I described in #43244 (comment) is still broken in VS Code 1.33.1 (and 1.34.0-exploration) but works fine in 1.34.0-insider O_o. So there's a proper behavior that's been there in the insider builds for months but never made it to the actual release

@sd3412
Copy link

sd3412 commented Jul 24, 2019

Are assignees ever going to take ownership of this issue or will VS Code remain a broken editor forever?

@ssigwart
Copy link
Contributor

ssigwart commented Apr 6, 2024

@aiday-mar, is there a setting to disable this? As I mentioned in the main description for PR #136577, always adding an extra tab is annoying because not all if statements are going to be single line ifs.

From PR:

while I like single line if statements without brackets, I don't want VS Code to assume all my if statements are going to be a single line for 2 reasons:

If I do add brackets, it then looks like this:

if (1)
  {
    console.log('');
  }

I don't want to break other people's workflows. It seems more intuitive to hit tab once if you want to do a single line if > versus having to hit backspace if it were to assume it's going to be one line.

I added some screen records of the behavior for JavaScript in VS Code 1.88.0 vs Insider:

vsCode.mov
vsCodeInsiders.mov

I much prefer the current behavior over Insiders. The same applies to Typescript, but this change has already made it into 1.88.0.

@me21
Copy link

me21 commented Apr 6, 2024

Can't it be indented by default, but unindent if an opening brace is typed?

@ssigwart
Copy link
Contributor

ssigwart commented Apr 6, 2024

Can't it be indented by default, but unindent if an opening brace is typed?

I was thinking of that as an option too. It's not my preference though. I'm hoping these can be added as settings so that if people want it auto-indented, they can enable the setting and if they want it to unindent when a brace is added, they can enable that setting.

@ciabaros
Copy link

ciabaros commented Apr 6, 2024

Can't it be indented by default, but unindent if an opening brace is typed?

I agree, we need this (ideally), or a way to turn this off altogether...

Sadly, in its current state (without the above fix), this is much much worse than before, because:

  • For anyone (like me) who puts all their regular braces on new lines, this will now have a much bigger incorrect indentation impact, than the original lack of brace-less single line indentation.
  • If this current new behavior is integrated into the "Format Document" and "Format Selection" options, then all my code gets broken from here on, and I think I would have to switch editors altogether.
  • In any case, we'll have to get used to another even bigger quirk, in exchange.

While I'm thankful for the efforts to try and address this community request, in its current state, I think it needs to be pulled back, unless/until the un-indent brace fix above can be implemented.

@aiday-mar
Copy link
Contributor

aiday-mar commented Apr 8, 2024

Hi everyone thank you for sharing your concerns. By looking at the numbers in this thread, it currently seems that more users prefer indentation on Enter after a braceless if statement than not. I suggest we open an issue and let the community put upvotes to decide how to proceed.

@ssigwart The current implementation of indentation does not allow to add settings to turn specific indentation on or off unfortunately. The indentation rules are specified as regex patterns in so-called language configuration files, and making a setting out of the specific indentation rules, would imply changing the structure of the language configuration file. This would impact VS Code core as well as extension authors.

@me21 that is an interesting idea, unfortunately I believe this is currently not possible. In the language configuration file syntax we have a so-called unIndentedLinePattern which allows to ignore previous indentation. We could define a regex rule so that we unindent when the pair {} is typed, but this could generally cause issues elsewhere. Imagine I am within an object as follows:

{
    a: {}
}

As soon as I type the {} this would unindent that field which is not what we want in this case. Ideally we want to unindent when we notice that the previous line contains an if statement and the current line contains a {}. Currently in language configuration files, we do not support the concept of previousLinePattern for the indentation rules. I will open an issue to myself and discuss with colleagues, to see how we can resolve this issue. Perhaps there is another way around this issue.

@ciabaros The 'Format Document' is a separate feature and this indentation rule will not be merged with this command's behavior.

In order to fix this issue for those who want a different behavior, we can explore the feature of allowing users to define their own indentation rules within the settings to override those defined by the language configuration files. You could then define rules that better suit your needs. I will open an issue about this and discuss with colleagues.

@hleVqq
Copy link

hleVqq commented Apr 8, 2024

This feature (though looks like more of an oversight to me) is needed for various languages with C-like syntax and you cannot assume that the opening brace is not going to go on a separate line.

This needs to work:

if (foo)
    bar();

This needs to (continue to) work:

if (foo) 
{
    bar();
}

And this needs to (continue to) work:

if (foo) {
    bar();
}

@ssigwart
Copy link
Contributor

ssigwart commented Apr 8, 2024

In order to fix this issue for those who want a different behavior, we can explore the feature of allowing users to define their own indentation rules within the settings to override those defined by the language configuration files. You could then define rules that better suit your needs. I will open an issue about this and discuss with colleagues.

Thanks, @aiday-mar. That sounds useful. One thing I was thinking that is kind of along these lines is that there could potentially be multiple language configuration files and VS Code would pick the appropriate one based on the settings the user picks. Though perhaps loading one config file, then manipulating it based on user setting would work too.

@Spongman
Copy link

Spongman commented Apr 9, 2024

@ssigwart no configuration is necessary to distinguish between the 3 examples that @hleVqq gave above.

This whole debacle can simply be solved like this:

when enter is pressed, if a new scope was created, indent accordingly. If the first character entered on the first, indented, otherwise-blank line is a ‘{‘, then un-indent that line first before inserting the ‘{‘.

@svivian
Copy link

svivian commented Apr 9, 2024

when enter is pressed, if a new scope was created, indent accordingly. If the first character entered on the first, indented, otherwise-blank line is a ‘{‘, then un-indent that line first before inserting the ‘{‘.

Yes, this is exactly it. Sublime Text and other editors already did it this way years (decades?) ago.

@aiday-mar
Copy link
Contributor

Yes thank you for the clarification, I have opened a ticket here to track this feature : #209822

@ssigwart
Copy link
Contributor

ssigwart commented Apr 9, 2024

when enter is pressed, if a new scope was created, indent accordingly. If the first character entered on the first, indented, otherwise-blank line is a ‘{‘, then un-indent that line first before inserting the ‘{‘.

@Spongman, that is a good solution, but what I'm trying to point out is that it's not the ideal solution for everyone. For me, I could live with that over the behavior in #43244 (comment), but I personally don't like when the editor adds characters that don't belong. I have this urge to hit backspace first to get rid of the tab instead of assuming that the editor will remove it for me. I feel like I'm fighting the editor.

I'm not suggesting this gets implemented, but an equally valid solution to your unindenting suggestion would be to not indent and automatically indent if any character other than { is pressed.

My ideal solution is that no tab gets added and I can add the tab if needed. I know I'm probably in the minority here, so I'm not advocating for that to be the only option. I know people have different preferences, so the thing I'm advocating for is a setting to control the behavior.

@aiday-mar
Copy link
Contributor

Hi @ssigwart thank you for your input, I understand your preferences. I have discussed the feature of allowing users to configure the indentation rules in the settings, and our team so far is leaning overall towards not providing it for several reasons:

  1. It surfaces complexity that should ideally remain hidden
  2. When the indentation rules are overridden this could cause unforeseeable indentation errors that the user did not take into account

If there are sufficient upvotes on the issue #209805 we could reconsider allowing users to override the indentation rules in the settings.

In the meantime, you can also create a language extension that provides a language configuration file for your language of interest which contains the modified indentation rules. The advantage of developing an extension is that others can also benefit from the same indentation rules.

@ssigwart
Copy link
Contributor

ssigwart commented Apr 9, 2024

Thanks, @aiday-mar.

In the meantime, you can also create a language extension that provides a language configuration file for your language of interest which contains the modified indentation rules. The advantage of developing an extension is that others can also benefit from the same indentation rules.

Yes, that was my fallback plan. I've done that for other languages before.

@FlowIT-JIT
Copy link

FlowIT-JIT commented May 6, 2024

Adding indentation after flow control is a terrible ideer idea. Most people will immediately add curly brackets after a loop or if statement. Unfortunately indentation after flow control seems to be the new default behaviour in 1.88.x (#209802) which results in an absolutely terrible editing experience.

@ygoe
Copy link

ygoe commented May 6, 2024

Did you mean "a terrible idea"?

Can you please explain what the issue is? From my understanding, an opening curly brace should unindent itself again automatically. At least that's the only way it makes sense if you want any automatic indentation. Every other way will cause pain somewhere.

@FlowIT-JIT
Copy link

Did you mean "a terrible idea"?

Yes :-)

Can you please explain what the issue is? From my understanding, an opening curly brace should unindent itself again automatically. At least that's the only way it makes sense if you want any automatic indentation. Every other way will cause pain somewhere.

If only adding curly brackets would outdent the code again, it would be fine. But it doesn't. Notice how I constantly have to fix my code in this video clip:

VSCodeindentationbugs-ezgif com-video-to-gif-converter

@ciabaros
Copy link

ciabaros commented May 6, 2024

@aiday-mar so, unfortunately this fix has introduced an even bigger problem, per comments above (for people who rely on newline scope braces). How can we turn it off, until it is implemented to correctly unindent opening braces?

@ssigwart
Copy link
Contributor

I noticed that the current implementation is a little inconsistent. If you hit Enter after if or else if, it will auto-indent. However, it does not indent after else. Personally, I don't think any of them should indent, but if the first 2 do, I think else should as well to be consistent.

In the meantime, you can also create a language extension that provides a language configuration file for your language of interest which contains the modified indentation rules. The advantage of developing an extension is that others can also benefit from the same indentation rules.

I created the ssigwart.javascript-typescript-language-configuration extension that will remove the auto-indenting after if, for, and while in JavaScript and Typescript.

@FlowIT-JIT
Copy link

FlowIT-JIT commented May 13, 2024

I created the ssigwart.javascript-typescript-language-configuration extension that will remove the auto-indenting after if, for, and while in JavaScript and Typescript.

So cool that you are trying to fix this mistake - much appreciated 💪😊
I do think they need to undo everything they changed in regards to indentation in 1.88 though - it is completely broken, and it's beyond me that they haven't shipped a fix yesterday. This is a major bug breaking the editing experience.

@aiday-mar
Copy link
Contributor

aiday-mar commented May 13, 2024

Hi @ssigwart thank you for pointing that out about the else. That is a bug and needs to be brought up to consistency with the other control flow statements. As I mentioned in the other threads, I believe the best experience to satisfy a majority of users would be to indent by default after an if statement, and when a curly brace is typed on the following line, to outdent automatically. I believe this is how some other editors handle indentation too, and I will explore this further.

@FlowIT-JIT
Copy link

@aiday-mar Are you aware of the outdent issue documented in my video? Watch what happens when adding a line break after the forEach(function(x)
#43244 (comment)
In this case curly brackets must indent the code.

@KyleKolander
Copy link

Yes, please fix this so it continues to work as it always did if you hit ENTER and then an opening curly brace. I spent a long time trying to figure out what I (or an installed extension) did to cause this to break. I didn't even consider the possibility that a change like this would have been intentionally introduced. It's one thing if it's just a bug, but this intentionally broke backward compatibility and deviates from the norm with other editors like VS. Wow...

@aiday-mar
Copy link
Contributor

Hi @FlowIT-JIT I have to look into that thank you.

After a braceless if statement the next line is indented, and when you type an opening curly brace, the line is outdented. This behavior is already in Insiders and will come out in the next stable release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug editor-autoindent Editor auto indentation issues javascript JavaScript support issues on-unit-test typescript Typescript support issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.