Skip to content

Fix single line if/else and loop indentation for Typescript and PHP #136577

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.

Sign up for GitHub

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

Jump to bottom
Merged
hediet merged 7 commits into from
Jul 7, 2023
Merged

Fix single line if/else and loop indentation for Typescript and PHP #136577

hediet merged 7 commits into from
Jul 7, 2023

Conversation

@ssigwart
Copy link
Contributor

@ssigwart ssigwart commented Nov 6, 2021

This PR addresses #43244. It does not solve the all the issues in there. In particular, 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:

  1. If I do add brackets, it then looks like this:
if (1)
  {
    console.log('');
  }
  1. 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.

Here are the parts addresses:

Below are some sample files to test with.

Typescript

if (1)
	console.log('deindent');
else if (2)
	console.log('deindent');
else
	console.log('deindent');

// Change doesn't make this de-indent where after 2, 3, of 4
if (1)
	if (2)
		if (3)
			if (4)
				console.log('so dangerous');
			else
				console.log('deindent'); // Doesn't decrease indent by 2 after this line, but this is so dangerous of a format I don't want to support it


while (1)
	console.log('deindent');

for (var i = 0; i < 5; i++)
	console.log('deindent');

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

console.log('line two'); // Alt-Down here should not indent
console.log('line one'); // Alt-Up here should not indent

PHP

<?php

if (1)
	echo('deindent');
else if (2)
	echo('deindent');
elseif (3)
	echo('deindent');
else
	echo('deindent');

// Change doesn't make this de-indent where after 2, 3, of 4
if (1)
	if (2)
		if (3)
			if (4)
				echo('so dangerous');
			else
				echo('deindent'); // Doesn't decrease indent by 2 after this line, but this is so dangerous of a format I don't want to support it


while (1)
	echo('deindent');

for ($i = 0; $i < 5; $i++)
	echo('deindent');

foreach ($arr as $val)
	echo $val . "\n";

@ssigwart ssigwart mentioned this pull request Nov 6, 2021
Open
@ssigwart ssigwart mentioned this pull request Nov 7, 2021
Open
@ssigwart ssigwart force-pushed the singleLineDeindent branch from d021de1 to 16f501e Compare June 5, 2022 03:56
@ssigwart
Copy link
Contributor Author

ssigwart commented Jun 5, 2022

I rebased this to fix a merge conflict. It's a fairly simple change that would solve a pretty annoying daily issue. It would be great if this could get merged.

aviden
aviden previously approved these changes Aug 10, 2022
View reviewed changes
@ssigwart
Copy link
Contributor Author

ssigwart commented Oct 27, 2022

@rebornix, is there anything else I need to do to get this PR merged or is it just waiting on someone to have time to review it?

@ssigwart ssigwart mentioned this pull request Nov 18, 2022
Merged
@ssigwart
Copy link
Contributor Author

ssigwart commented Nov 22, 2022

@hediet, if you could look at this one, I'd really appreciate it. Basically, it adds a deindent after single line code for PHP/TS/JS. For example, if you put your cursor where the | is below and hit Enter in a PHP file.

if (1)
  1;|

@hediet hediet assigned hediet and unassigned rebornix Nov 23, 2022
@hediet hediet added this to the November 2022 milestone Nov 23, 2022
@aeschli aeschli modified the milestones: November 2022, December 2022 Nov 29, 2022
@hediet hediet modified the milestones: January 2023, February 2023 Jan 26, 2023
@hediet hediet modified the milestones: February 2023, March 2023 Feb 20, 2023
me21
me21 suggested changes Feb 28, 2023
View reviewed changes
Copy link

@me21 me21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add C/C++ languages support to this PR also?

@ssigwart
Copy link
Contributor Author

ssigwart commented Mar 1, 2023

Could you please add C/C++ languages support to this PR also?

I updated it to include C/C++.

@hediet hediet removed this from the March 2023 milestone Mar 20, 2023
@hediet hediet added this to the April 2023 milestone Mar 20, 2023
@hediet hediet modified the milestones: April 2023, May 2023 Apr 24, 2023
@hediet hediet modified the milestones: May 2023, June 2023 May 26, 2023
@hediet hediet modified the milestones: June 2023, July 2023 Jun 27, 2023
@hediet hediet enabled auto-merge July 7, 2023 18:35
@hediet hediet merged commit 32e968c into microsoft:main Jul 7, 2023
@ssigwart
Copy link
Contributor Author

ssigwart commented Jul 7, 2023

Thank you, @hediet and @TylerLeonhardt!

@ssigwart ssigwart deleted the singleLineDeindent branch July 7, 2023 19:48
@ssigwart ssigwart mentioned this pull request Aug 3, 2023
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@TylerLeonhardt TylerLeonhardt TylerLeonhardt approved these changes

@hediet hediet hediet approved these changes

+2 more reviewers

@me21 me21 me21 approved these changes

@aviden aviden aviden left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@hediet hediet

Labels

None yet

Projects

None yet

Milestone

July 2023

Development

Successfully merging this pull request may close these issues.

7 participants

@ssigwart @me21 @TylerLeonhardt @hediet @aviden @rebornix @aeschli