brodycj / prettierx

a less opinionated fork of Prettier code formatter
MIT License
209 stars 22 forks source link

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

Open brodycj opened 3 years ago

brodycj commented 3 years ago

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 https://github.com/prettierx/prettierx-0.4.x-fork/pull/41 & https://github.com/prettierx/prettierx-0.4.x-fork/pull/46.)

I made a demo in https://github.com/prettier/prettier/issues/4203#issuecomment-847442620 - 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: https://github.com/prettier/prettier/issues/4203#issuecomment-867213164

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 https://github.com/prettier/prettier/issues/4203#issuecomment-847442620:

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 https://github.com/prettier/prettier/issues/4203#issuecomment-867213164 (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.

brodycj commented 3 years ago

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.

brodycj commented 3 years ago

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.