Skip to content

'reindent all lines' and formatter have different formatting strategies #19140

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

Closed
aeschli opened this issue Jan 24, 2017 · 9 comments
Closed
Assignees
Labels
editor-autoindent Editor auto indentation issues *out-of-scope Posted issue is not in scope of VS Code typescript Typescript support issues under-discussion Issue is under discussion for relevance, priority, approach

Comments

@aeschli
Copy link
Contributor

aeschli commented Jan 24, 2017

Testing #19091
In a typescript file:

class B {
	public foo() {
		console.log('hello');
		switch (parseInt(this.toString())) {
			case 1: 
				break;
			case 2:
				break;
		}
	}
}
  • use 'Reindent All Lines'
  • Indenting of the 'break' statements is changed to align to the case statements
  • Use the formatter: break statements are indented again

It would be better if formatter and the reindent command use the same indentation strategy.

@rebornix
Copy link
Member

This is because that our indentation rules for TypeScript is not complete, the one we are using is

decreaseIndentPattern: /^(.*\*\/)?\s*\}.*$/,
increaseIndentPattern: /^.*\{[^}"']*$/

The one TextMate or other editors are using is like

decreaseIndentPattern: /^\s*((?!\S.*\/[*]).*[*]\/\s*)?[})\]]|^\s*(case\b.*|default):\s*(\/\/.*|\/[*].*[*]\/\s*)?$/,
increaseIndentPattern: /(\{[^}"']*|\([^)"']*|\[[^\]"']*|^\s*(\{\}|\(\)|\[\]|(case\b.*|default):))\s*(\/\/.*|\/[*].*[*]\/\s*)?$/,
indentNextLinePattern: /^\s*(for|while|if|else)\b(?!.*[;{}]\s*(\/\/.*|\/[*].*[*]\/\s*)?$)/,
unIndentedLinePattern: /^(?!.*([;{}]|\S:)\s*(\/\/.*|\/[*].*[*]\/\s*)?$)(?!.*(\{[^}"']*|\([^)"']*|\[[^\]"']*|^\s*(\{\}|\(\)|\[\]|(case\b.*|default):))\s*(\/\/.*|\/[*].*[*]\/\s*)?$)(?!^\s*((?!\S.*\/[*]).*[*]\/\s*)?[})\]]|^\s*(case\b.*|default):\s*(\/\/.*|\/[*].*[*]\/\s*)?$)(?!^\s*(for|while|if|else)\b(?!.*[;{}]\s*(\/\/.*|\/[*].*[*]\/\s*)?$))/

But we still have good experience with TypeScript as we allow extensions to customize onEnterRules, the indentation adjusts correctly for most of the case when users press enter.

Right now Reindent lines only read indentation rules and doesn't take onEnter rules into consideration as at least for TypeScript there are a few overlap between the second indentation rules set and onEnterRules.

I suggest to make ReIndent Lines honor onEnterRules and remove unnecessary onEnterRules from TypeScript. cc @mjbvz . Since it's kind of breaking change I'd like to postpone it to following iteration and work together with Matt.

@Nimzozo
Copy link

Nimzozo commented Mar 25, 2017

A case label is not a statement and should not be indented, but aligned with the switch.

@rebornix rebornix added the under-discussion Issue is under discussion for relevance, priority, approach label Apr 19, 2017
@rebornix rebornix added the editor-autoindent Editor auto indentation issues label Jun 7, 2017
@alexdima alexdima removed the editor label Nov 23, 2017
@wuhkuh
Copy link

wuhkuh commented Mar 30, 2019

Other issues have been closed in favor of this one, but this issue has gone stale. Could you give us an update?

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 12, 2019

Changing switch/case indentation for the JS/ts formatter is tracked by microsoft/TypeScript#18682

@mjbvz mjbvz added the *out-of-scope Posted issue is not in scope of VS Code label Oct 11, 2019
@vscodebot
Copy link

vscodebot bot commented Oct 11, 2019

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that are not going to be addressed in the foreseeable future: We look at the number of votes the issue has received and the number of duplicate issues filed. More details here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!

@vscodebot vscodebot bot closed this as completed Oct 11, 2019
@derek-pavao
Copy link

Why was this issue closed and points to an issue about switch/case indentation specifically. This is an issue that effects more than just switch. Jsdoc comments are formatted differently by reindent lines vs format, as well as jsx.

I expect all indentation commands to indent the same

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 24, 2019

There is pretty much zero reason to use Reindent all lines in js/ts files; use format document. Reindent all lines has a lot of bugs and limitations and we do not plan on investing in it further given its complexity. Using Format document is the solution for js/ts files

@derek-pavao
Copy link

So, every couple of months I keep coming back to vscode hoping to be able to use it. There is a lot to like with vscode. However, the one thing that keeps getting to me is how indentation works. I stumbled back upon this issue I commented on a couple of months ago.

While I agree Reindent all lines is probably useless in favor of Format document, what I want to do is have reindent current line indent correctly.

Let me explain where I'm coming from on this. I am an avid emacs user, and in emacs the tab key will auto indent the current line. I changed my tab key in my vscode config to execute reindent current line expecting to achieve this behavior, but given the unexpected result, it doesn't work for me. I guess what I really wish is there was a format current line in addition to the already existing format selection and format document

@derek-pavao
Copy link

my fault, it looks like reindent current line comes from an extension. I appologize

@microsoft microsoft locked as resolved and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-autoindent Editor auto indentation issues *out-of-scope Posted issue is not in scope of VS Code typescript Typescript support issues under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

7 participants