Change the arguments of syncDiff to the same as Sequelize constructor

[wkkwok]

Using url affects compatibility as older versions of postgresql does not support.
Instead, passing config object should be more compatibility to other nodejs db clients, thus maybe helpful when dbdiff plan to support other database later on.

P.S. Sometimes is troublesome when database user password contain special characters, e.g. '@', '/'...

[gimenete]

Yeah, good idea. But there are two things I would like to discuss:

With the tools that I'm used to use I use a lot URLs so I would like to keep compatibility also with URLs. Can you make Sequelize.prototype.syncDiff accept both (url, options) and (database, username, password, options)?

Also you removed the self.sync() to the master database. Any particular reason to do that? I think that the default sync() without force: true is safe.

Thanks for your PR!

[wkkwok]

I checked Sequelize constructor source code. I found that it can handle the conversion between URLs and connection config object.
Therefore, just get config object from sequelize instance instead of passing-in argument should do the job.

About removing self.sync(), my intention is to allow the library to production all table creation sql query instead of queries responsible for unsynchronized part only. We can manually sync other master database first if we want to get the latter result.

P.S. I am currently working on a project where we need a auto migration system (generating sequelize migration compatible js file). We want to avoid the use of sequelize.sync() because It magically done "ONLY PARTS" of the things. It is hard for my team to keep track on the changes.

[gimenete]

The test doesn't pass.

When I said I wanted to keep support for URLs I meant this:

// old method
Sequelize.prototype.syncDiff = function(url, options) {
// new method
Sequelize.prototype.syncDiff = function(database, username, password, options) {

Both ways should be supported. Probably checking the number of arguments passed to the method.

if (arguments.length === 2) // use the url
else // use database, username and password

[wkkwok]

sequelize is currently doing this:
`var Sequelize = function(database, username, password, options) {
var urlParts;
options = options || {};

if (arguments.length === 1 || (arguments.length === 2 && typeof username === 'object')) {
options = username || {};
urlParts = url.parse(arguments[0]);

username = null;
password = null;

if (urlParts.pathname) {
  database = urlParts.pathname.replace(/^\//, '');
}

options.dialect = urlParts.protocol.replace(/:$/, '');
options.host = urlParts.hostname;

if (urlParts.port) {
  options.port = urlParts.port;
}

if (urlParts.auth) {
  username = urlParts.auth.split(':')[0];
  password = urlParts.auth.split(':')[1];
}

}`

So I think we can pass either url or config object to sequelize constructor first and get config object for db client from sequelize.config. I haven't tested it with URL though.

[gimenete]

Yeah, in order to accept your changes syncDiff(url, options) should work. There is a test for that and right now is not passing.