Fixing passing --database-url as global cli arg

[weiznich]

--database-url is passed as global cli argument, this means it may be
not part of the current ArgMatches. To solve this we need to look recursivly in all current subcommands for the database-url

[sgrif]

Does setting the global config accomplish the same thing? If not I think we should fix this in clap.

[weiznich]

Does setting the global config accomplish the same thing? If not I think we should fix this in clap.

The documentation of global states explicitly the current behavior, so this seems to be desired or at least not fixable without changing larger parts of clap.

NOTE: Global arguments, when matched, only exist in the command's matches that they were matched to. For example, if you defined a --flag global argument in the top most parent command, but the user supplied the arguments top cmd1 cmd2 --flag only cmd2's ArgMatches would return true if tested for ArgMatches::is_present("flag").

[weiznich]

There is also this issue on the clap repo, but setting this flag does not solve the problem…

Edit:
Setting this flag allows to simplify the fix, as we need now only to perform the recursive lookup.

[sgrif]

Can you add a few test cases?

[weiznich]

Something like the last commit?

[sgrif]

Every test case needs to have a unique project name.

[sgrif]

The last commit is the sort of thing I had in mind, yes. However, those tests pass without your code changes

[sgrif]

Based on some cursory testing, it looks like we should only need the .setting(AppSettings::PropagateGlobalValuesDown). However, that setting causes diesel --database-url=... database drop to work, but not diesel database --database-url=... drop. The addition of the database_url_from_cli function doesn't seem to affect it.

[weiznich]

Based on some cursory testing, it looks like we should only need the .setting(AppSettings::PropagateGlobalValuesDown). However, that setting causes diesel --database-url=... database drop to work, but not diesel database --database-url=... drop. The addition of the database_url_from_cli function doesn't seem to affect it.

Executing target/debug/diesel migration run --migration-dir some/migrations/dir --database-url /tmp/test.db fails for me without the database_url_from_cli function.

Also this function may fix the issue with diesel --database-url=...

[weiznich]

However, those tests pass without your code changes.

I've modified those tests to run without the DATABASE_URL environment variable. Now migration_run_runs_pending_migrations_custom_database_url_1 fails without the using database_url_from_cli

[sgrif]

Can you add a note that we can remove that function once clap-rs/clap#978 is fixed?

[sgrif]

You need to run rustfmt

[weiznich]

Fixed by #1304