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

End of multiline ternary expressions not aligned with colon anymore when using tab indentation #15783

Open
paescuj opened this issue Dec 11, 2023 · 8 comments

Comments

@paescuj
Copy link

paescuj commented Dec 11, 2023

Prettier 3.1.1
Playground link

--parser babel
--use-tabs

Input:

condition ? exprIfTrue : {
  exprIfFalse
};

Output:

Ends with a hard tab and is not aligned with the colon anymore (same goes for truthy expression):

condition
	? exprIfTrue
	: {
			exprIfFalse,
		};

Expected behavior:

Should end with a soft indent to be aligned with colon, as before with prettier@3.1.0:

condition
	? exprIfTrue
	: {
			exprIfFalse,
	  };

Most likely related to #15662.

Thanks for your work ❤️

@ehoogeveen-medweb
Copy link

ehoogeveen-medweb commented Dec 13, 2023

Alternative formatting that would look okay (IMO):

condition
	?	exprIfTrue
	:	{
			exprIfFalse,
		};

There was an experimental change (later reverted) in #15662 that would have produced this formatting, but it went too far, also changing cases like

	condition ? exprIfTrue : exprIfFalse

to

	condition ?	exprIfTrue :	exprIfFalse

As I commented there, I think it's only safe to use tabs after non-tab characters if it's after a single, tab-aligned character (like ? or :).

@ehoogeveen-medweb
Copy link

Technically I guess it's impossible to be fully consistent using tabs, if you consider a tab width of 1 space:

condition
 ? exprIfTrue
 : {
   exprIfFalse,
  };

But I doubt anyone actually uses a tab width of 1, and it works with a tab width of 2 and higher:

condition
  ? exprIfTrue
  : {
      exprIfFalse,
    };

@sosukesuzuki
Copy link
Member

This is fixed by #15662. Please wait next release

Prettier pr-15662
Playground link

--parser babel
--use-tabs

Input:

condition ? exprIfTrue : {
  exprIfFalse
};

Output:

condition
	?	exprIfTrue
	:	{
			exprIfFalse,
		};

@ehoogeveen-medweb
Copy link

I'm confused, didn't that PR make it into Prettier v3.1.1, which this bug report is for? And I see the same output in both playgrounds:

condition
	? exprIfTrue
	: {
			exprIfFalse,
		};

@paescuj
Copy link
Author

paescuj commented Jan 13, 2024

@sosukesuzuki This is still an issue in v3.2.1, could you please reopen? 🙏

@kachkaev
Copy link
Member

kachkaev commented Jan 13, 2024

Thanks for flagging this @paescuj! I can still observe misaligned { } in the playground:

Prettier 3.2.1
Playground link

--parser babel
--use-tabs

Input:

condition ? exprIfTrue : {
  exprIfFalse
};

Output:

condition
	? exprIfTrue
	: {
			exprIfFalse,
		};

Expected output:

condition
	?	exprIfTrue
	:	{
			exprIfFalse,
		};

Reopening for further investigation.

@kachkaev kachkaev reopened this Jan 13, 2024
@sndrrth
Copy link

sndrrth commented Jan 15, 2024

Why is the expected output with tabs instead of spaces after ? and :? In previous versions Prettier used spaces which is much more readable imho even if it results in a mix of tabs and spaces (if tabs are enabled).

condition
	? exprIfTrue
	: {
		exprIfFalse,
	  };

@eamodio
Copy link

eamodio commented Apr 7, 2024

I really wish that #15662 was reverted, or at a minimum be able to turn it off -- it is what is causing us to have to stick on 3.1.0.

useTabs shouldn't, imo, be interpreted to mean spaces can't be used anywhere -- since we still use spaces in TONS of places in code even if we indent with tabs. There is certainly a grey area since for the lines outside the ? and : prefix are technically getting indented, but imo alignment with the ? & : lines are more important than sticking with pure tabs (should still use tabs up until the point where more granular spacing is needed -- which I believe was the case). Forcing a full tab between the ? & : and their conditions is extremely undesirable.

Can this be turned off, opt-out, or reverted?

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

No branches or pull requests

6 participants