Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Help test dprint #16

Closed
luisherranz opened this issue May 17, 2022 · 10 comments
Closed

Help test dprint #16

luisherranz opened this issue May 17, 2022 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@luisherranz
Copy link
Member

@gziolo proposed fully adopting prettier (and dropping the WordPress spacing standard) in January: https://make.wordpress.org/core/2022/01/05/proposal-changes-to-javascript-coding-standards-for-full-prettier-compatibility/

Even though the proposal received a lot of support, @ntwb said in Slack that the consensus at the moment is a "No" from the core team. We've been unable to locate where that core team conversation took place, and I'm not sure why Gutenberg contributors are not considered Core contributors, but the proposal is currently stuck because of that.

I'll keep trying to contact @ntwb and see if he can finally share more information about how, where and why that decision was made, but until then, I was testing dprint, as an alternative, just in case prettier doesn't finally work out.

As I'm not the only one using this repository anymore, it'd be great to know your impressions with dprint. My main issue right now is that you cannot use a local node_modules installation, and therefore people need to figure out how to install it globally. Also, I was unable to make the VS Code extension work by default with the global installation of dprint and I had to manually add a dprint.path in the VS Code settings.

If you've also tested it out while working on this repository, please share your impressions 🙂

@luisherranz luisherranz added the help wanted Extra attention is needed label May 17, 2022
@luisherranz
Copy link
Member Author

My main issue right now is that you cannot use a local node_modules installation, and therefore people need to figure out how to install it globally. Also, I was unable to make the VS Code extension work by default with the global installation of dprint and I had to manually add a dprint.path in the VS Code settings.

I was able to fix those issues, which were happening because my local folder contains spaces: dprint/dprint-vscode#33

Actually, support for local node_modules was added three days ago 😄 🎉

@luisherranz
Copy link
Member Author

luisherranz commented May 17, 2022

I've just found out that the latest version of the JS/TS dprint plugin supports a new option called spaceAround that mimics (some of the?) WordPress's coding standards: dprint/dprint-plugin-typescript#341

I've added a PR so we can test it in this repository: #17

@luisherranz
Copy link
Member Author

I've been carefully reading the WP JavaScript coding standards and trying to match them with dprint options.

You can see the result in the playground.

TLDR;
Most things work with the current options.

Things that don't work

  • Spaces when accessing properties/items of objects/arrays

    const x = a["b"];
    const x = a[0];
    
    // It should be:
    const x = a[ "b" ];
    const x = a[ 0 ];

    It should be easy to add a new option for this if they agree.

  • Context indentation in chains

    x.
      a( 1 )
      .aa( 2 )
      b( 3 )
    
    // It should be:
    x.
      a( 1 )
        .aa( 2 )
      b( 3 )

    I don't think this will ever work because it can be inferred from the code.

  • Using individual lines in variable declarations.

    const a, b, c = 1, d;
    
    // It should be:
    const a, b,
      c = 1,
      d;

    I'm not sure how easy it would be to add this functionality. It probably can be done, but not sure if it'll be worth the effort.

  • Automatically adding a new line above comments

    const a = 1;
    // Comment.
    const b = 2;
    
    // Instead it should be:
    const a = 1;
    
    // Comment.
    const b = 2;

    This is not enforced or automatically transformed, but the developer can add the new line manually, so it's not a deal-breaker.

    I guess this can be added, although I'm not sure if it will conflict with other rules, like no new line at the start of a block.

  • Automatically transform object props to dot notation whenever possible

    x['a'];
    
    // Instead t should be:
    x.a;

    This is not enforced or automatically transformed, but the developer can add the new line manually, so it's not a deal-breaker.

    I guess this can be added to dprint. Prettier already does it.

Bugs that affect WP coding standards

  • Spaces on functions without arguments

    x(  ) === true
    
    // It should be:
    x() === true

    I've already created a PR to fix it.

  • No spaces in the inner parenthesis

    if ( x && (y && z) ) a = 1;
    
    // It should be:
    if ( x && ( y && z ) ) a = 1;

    It should be easy to fix.


You can see the configuration in the dprint.json file.

{
	"useTabs": true,
	"lineWidth": 80,
	"typescript": {
		"quoteStyle": "alwaysSingle",
		"useBraces": "always",
		"singleBodyPosition": "nextLine",
		"operatorPosition": "sameLine",
		"preferSingleLine": true,
		"memberExpression.preferSingleLine": false,
		"binaryExpression.linePerExpression": true,
		"memberExpression.linePerExpression": true,
		"spaceAround": true
	}
}

@gziolo
Copy link
Member

gziolo commented May 19, 2022

@luisherranz, I think you should be comparing with what wp-prettier does with the current config from @wordpress/prettier-config:

https://github.com/WordPress/gutenberg/blob/trunk/packages/prettier-config/lib/index.js

I'm not 100% sure that all WP JS coding standards can be achieved with automated formatting so we tried to bring it as close as possible. We might miss some changes as well in the process.

It probably would be best to run wp-prettier with the config I shared on the files formatted with dprint to learn about differences.

@luisherranz
Copy link
Member Author

I think you should be comparing with what wp-prettier does with the current config from @wordpress/prettier-config

Oh, ok. I'll try to match the options you are currently using with prettier that are not present in the coding standards, like jsxBracketSameLine: false and arrowParens: 'always'. Thanks 🙂

It probably would be best to run wp-prettier with the config I shared on the files formatted with dprint to learn about differences.

I'll do that as well, good idea.

I think there are aesthetic differences between both in the formatting, but nothing that affects the coding standards.

If we ever move Gutenberg to dprint, we'd need to do one of those "git blame ignore" commits, though.

@luisherranz
Copy link
Member Author

For the record, I've been trying to set up @prettier/plugin-php with VS Code and hasn't been straightforward either. I still haven't managed to get it working with markdown php snippets 😕

@ockham
Copy link
Collaborator

ockham commented Jun 16, 2022

I'm probably Doing It Wrong:tm:, but FWIW, the VS Code extension doesn't seem to work for me:

dprint

Maybe we can add auto-formatting to the git commit hook? I think GB does something like that (or maybe just linting).

@luisherranz
Copy link
Member Author

Oh, that's a shame 😩

Does the VS Code prettier extension work for you in that project?

@luisherranz
Copy link
Member Author

I'm going to stop this experiment and move back to Prettier to start testing the prettier-php plugin as well.

@luisherranz
Copy link
Member Author

Done in #43.

I'll add prettier-php in a future PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants