doctrine / migrations

Doctrine Database Migrations Library
https://www.doctrine-project.org/projects/migrations.html
MIT License
4.65k stars 386 forks source link

Add nowdoc format for auto-generated migrations #1388

Closed ghost closed 4 months ago

ghost commented 6 months ago

Feature Request

Add heredoc format for auto-generated migrations

Q A
New Feature yes
RFC no
BC Break no

Summary

e.g. https://github.com/doctrine/migrations/blob/3.7.x/lib/Doctrine/Migrations/Tools/Console/Command/DiffCommand.php

            ->addOption(
                'nowdoc',
                null,
                InputOption::VALUE_NONE,
                'Use nowdoc format for SQL statements.',
            )

Without the new option

    public function up(Schema $schema): void
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->addSql('CREATE TABLE...');
    }

With the new option

    public function up(Schema $schema): void
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->addSql(<<<'SQL'
                CREATE TABLE...
        SQL);
    }
greg0ire commented 6 months ago

Two things:

ghost commented 6 months ago

@greg0ire Yes, nowdoc too, why not 😉

Benefits

greg0ire commented 6 months ago

More readable SQL statements

How exactly would they become more readable?

More readable SQL git diffs

This is not obvious to me either

Easier copy-paste for local testing

Because you can copy entire lines, or for another reason?

ghost commented 6 months ago

The --formatted option already does part of the job, but quote escaping makes it more difficult to read/compare.

Yes, using nowdoc or heredoc, you can copy-paste and test SQL queries without any modification (such as unescaping).

greg0ire commented 6 months ago

TIL about --formatted… I guess it could indeed be nicer with NOWDOC. Now I'm not sure we should support too many scenarios. Maybe we should just switch to NOWDOC and not expose any other option. I'm curious what other maintainers think.

ghost commented 6 months ago

@greg0ire you’re right. Nowdoc is a better choice in this case 👍

greg0ire commented 6 months ago

@derrabus @SenseException what do you think about this change? @michael-barchy-bf would you be willing to implement it?

greg0ire commented 6 months ago

Would you mind opening a pull request? Since it's a new feature, please target 3.8.x

ghost commented 6 months ago

Sorry, i'm not familiar with github, someone else should do it

michael-barchy-bf commented 4 months ago

This issue might be a duplicate of this one : https://github.com/doctrine/migrations/issues/939

greg0ire commented 4 months ago

Duplicate of #939