dbconsole arguments order mismatch #749

Closed
lighthouse-import opened this Issue May 16, 2011 · 13 comments

Projects

None yet

3 participants

@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/5930
Created by Andrei Kulakov - 2011-03-09 22:23:14 UTC

rails dbconsole --help

yields

Usage: dbconsole [options] [environment]
-p, --include-password           Automatically provide the password from database.yml
--mode [MODE]                Automatically put the sqlite3 database in the specified mode (html, list, line, column).
-h, --header

but neather

rails dbconsole -p production

or

rails dbconsole -p test

sets correct rails environment.

rails dbconsole production -p

and

rails dbconsole production -p

set correct environment.


I supply patch and hope this is correct way to solve the issue.

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-11-08 01:51:51 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-11-08 01:51:57 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Vijay Dev - 2011-02-05 16:51:14 UTC

Won't it be easier to just change the Usage to specify environment first and then the options? :)

@Andrei: Applying the patch fails. Please check it against master.

Imported from Lighthouse.
Comment by Vijay Dev - 2011-02-05 16:58:37 UTC

If someone decides, we can change the usage message, attaching the simple patch.

Imported from Lighthouse.
Comment by Andrei Kulakov - 2011-02-05 17:33:02 UTC

Renewed patch.

Imported from Lighthouse.
Comment by Xavier Noria - 2011-03-09 19:55:12 UTC

I think it is more common to have options first, arguments later, generally speaking. I'd vote for revising the implementation.

Imported from Lighthouse.
Comment by Andrei Kulakov - 2011-03-09 20:02:59 UTC

Renewed patch. Maybe there is a better implementation.

Imported from Lighthouse.
Comment by Xavier Noria - 2011-03-09 20:17:43 UTC

I have not debugged the issue, but this should easily work correctly with optparse. I mean, no manual option handling if possible, better to use a standard library that solves that.

Could you give it a whirl please?

Imported from Lighthouse.
Comment by Andrei Kulakov - 2011-03-09 20:56:04 UTC

I couldn't make it load correct database, when I set ENV['RAILS_ENV'] within block of opt.on.
opt.on("-e", "--environment=name", String,
"Specifies the environment to load database for (test/development/production).",
"Default: development") do |env|
ENV['RAILS_ENV'] = %w(production development test).detect { |e| e =~ /^#{env}/} || env
end

I think manual option handling has something to do with warning in this comment:
# Has to set the RAILS_ENV before config/application is required
if ARGV.first && !ARGV.first.index("-") && env = ARGV.first
ENV['RAILS_ENV'] = %w(production development test).find { |e| e.index(env) } || env
end

Could you give it a whirl please?

Apologies for my knowledge of English, could you rephrase that please.

Imported from Lighthouse.
Comment by Andrei Kulakov - 2011-03-09 21:52:36 UTC

Regular rails console has usage message:
Usage: console [environment] [options]
and similar option handling implementation to dbconsole.

But rails server specifies environment through option:
-e, --environment=name Specifies the environment to run this server under (test/development/production).
Default: development

Imported from Lighthouse.
Comment by Xavier Noria - 2011-03-09 22:23:14 UTC

Yes there are counterexamples, for example find(1) needs the directory first, and then options. But I think the most common interface is options, then arguments.

A pity Rails commands are not consistent there. Maybe something worth taking into account for the next major revision.

Attachments saved to Gist: http://gist.github.com/971723

Member

Fixed here: 4e873ff

@josevalim: Can be closed.

@josevalim josevalim closed this May 16, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment