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

[prettierx] ternary formatting with objects & tabs needs improvement in upstream Prettier #552

Open
brodybits opened this issue May 25, 2021 · 2 comments
Labels
bug Something isn't working needs-discussion

Comments

@brodybits
Copy link
Owner

brodybits commented May 25, 2021

updated:

While working on PR #492 with simpler balanced ternary formatting, to resolve issue #468, I discovered that the default ternary formatting with objects & tabs coming from upstream Prettier could use some improvement. (This does not seem to be an issue with the now removed & replaced --no-align-ternary-lines option from bangkokjs/prettierx-0.4.x-fork#41 & bangkokjs/prettierx-0.4.x-fork#46.)

I made a demo in prettier/prettier#4203 (comment) - see Case 1 below ... now waiting for feedback whether or not it should be in a new issue in Prettier.

Update June 2021: adding Case 2 below with a demo that I reported in: prettier/prettier#4203 (comment)

Here are some related issues that I found on Prettier itself:

See also:

I hope to find a decent workaround when resolving issue #468 for release 0.19.0 (#493).


Case 1

My demo from prettier/prettier#4203 (comment):

From playground, with 8-space tab formatting which is consistent with default formatting on vim/macOS/Linux:

Prettier 2.3.0
Playground link

--parser babel
--use-tabs
--no-semi

Input:

a = {
	on: this.readonly ? {
			"sync:PremiumStyle": this.update_readonly_tailor_cover,
			"sync:OwnershipStructure": this.update_readonly_tailor_cover,
		} : {
			"change:client_life_ownership_structure:input_value": () =>
				this.life_tpd_trauma_select_logic("client"),
			"change:client_tpd_ownership_structure:input_value": () =>
				this.life_tpd_trauma_select_logic("client"),
		}
}

Output:

a = {
	on: this.readonly
		? {
				"sync:PremiumStyle": this.update_readonly_tailor_cover,
				"sync:OwnershipStructure": this.update_readonly_tailor_cover,
		  }
		: {
				"change:client_life_ownership_structure:input_value": () =>
					this.life_tpd_trauma_select_logic("client"),
				"change:client_tpd_ownership_structure:input_value": () =>
					this.life_tpd_trauma_select_logic("client"),
		  },
}

Proposed expected output:

a = {
	on: this.readonly
		? {
			"sync:PremiumStyle": this.update_readonly_tailor_cover,
			"sync:OwnershipStructure": this.update_readonly_tailor_cover,
		  }
		: {
			"change:client_life_ownership_structure:input_value": () =>
				this.life_tpd_trauma_select_logic("client"),
			"change:client_tpd_ownership_structure:input_value": () =>
				this.life_tpd_trauma_select_logic("client"),
		  },
}

Adapted by replacing each tab character with 4 spaces:

Input:

a = {
    on: this.readonly ? {
            "sync:PremiumStyle": this.update_readonly_tailor_cover,
            "sync:OwnershipStructure": this.update_readonly_tailor_cover,
        } : {
            "change:client_life_ownership_structure:input_value": () =>
                this.life_tpd_trauma_select_logic("client"),
            "change:client_tpd_ownership_structure:input_value": () =>
                this.life_tpd_trauma_select_logic("client"),
        }
}

Output:

a = {
    on: this.readonly
        ? {
                "sync:PremiumStyle": this.update_readonly_tailor_cover,
                "sync:OwnershipStructure": this.update_readonly_tailor_cover,
          }
        : {
                "change:client_life_ownership_structure:input_value": () =>
                    this.life_tpd_trauma_select_logic("client"),
                "change:client_tpd_ownership_structure:input_value": () =>
                    this.life_tpd_trauma_select_logic("client"),
          },
}

Proposed expected output:

a = {
    on: this.readonly
        ? {
            "sync:PremiumStyle": this.update_readonly_tailor_cover,
            "sync:OwnershipStructure": this.update_readonly_tailor_cover,
          }
        : {
            "change:client_life_ownership_structure:input_value": () =>
                this.life_tpd_trauma_select_logic("client"),
            "change:client_tpd_ownership_structure:input_value": () =>
                this.life_tpd_trauma_select_logic("client"),
          },
}

Case 2

from prettier/prettier#4203 (comment) (June 2021):

Prettier 2.3.1
Playground link

--parser babel

Input:

alice
  ? {
      beth: {
          carol: 101
        }
        ? {
            debbie: 202
          }
          ? {
              ellen: 303
            }
          : 404
        : 505
    }
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102
  : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201

Output:

alice
  ? {
      beth: {
        carol: 101,
      }
        ? {
            debbie: 202,
          }
          ? {
              ellen: 303,
            }
          : 404
        : 505,
    }
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102
  : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201;

Expected formatting behavior:

I was expecting better consistency in alignment between the closing braces & conditional operators.


with nested ternary expressions & arrays:

Prettier 2.3.1
Playground link

--parser babel

Input:

alice
  ? [
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101,
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102,
      [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0202,
      ]
        ? [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0301,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0302,
          ]
        : [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0401,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0402,
          ],
    ]
      ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
      : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
  : 1000

Output:

alice
  ? [
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0101,
      loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0102,
      [
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0201,
        loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0202,
      ]
        ? [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0301,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0302,
          ]
        : [
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0401,
            loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0402,
          ],
    ]
    ? loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
    : loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong0501
  : 1000;

Expected formatting behavior:

Better consistency in alignment between the closing brackets & conditional operators.

@brodybits brodybits added bug Something isn't working needs-discussion labels May 25, 2021
@brodybits
Copy link
Owner Author

brodybits commented May 25, 2021

I am thinking the workaround in prettierX should be keep the alignment change from the old (replaced) --no-align-ternary-lines feature in its own option.

@brodybits brodybits changed the title [prettierx] default ternary formatting with objects & tabs needs improvement in upstream Prettier [prettierx] ternary formatting with objects & tabs needs improvement in upstream Prettier Jun 7, 2021
brodybits added a commit that referenced this issue Jun 23, 2021
in ternary formatting with objects & tabs ref:

- #552
@brodybits
Copy link
Owner Author

I am thinking the workaround in prettierX should be keep the alignment change from the old (replaced) --no-align-ternary-lines feature in its own option.

I found another case while working on PR #620, now suspect we may need something more flexible than a boolean option. Further discussion will likely be in issue #585.

brodybits added a commit that referenced this issue Jun 28, 2021
in ternary formatting with objects & tabs ref:

- #552
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-discussion
Projects
None yet
Development

No branches or pull requests

1 participant