qrush / sub

a delicious way to organize programs
http://37signals.com/svn/posts/3264-automating-with-convention-introducing-sub
MIT License
1.74k stars 148 forks source link

Reliable way to display/format an error during autocomplete #19

Closed bradical closed 11 years ago

bradical commented 11 years ago

One of my scripts requires that the user have a valid properties file in their share directory. I've written a function to check fo the existence of the file and the property but the error is being chopped up during auto-complete. Is there any way to tell the auto-completer to keep my error on one line?

PROPS_FILE=$_SUB_ROOT/share/sub/mysql.propertie
check_for_props_and_load() {
if [ ! -f $PROPS_FILE ];
then
  echo "MySQL properties file not found. Please create a file in share/sub directory with at least a PASSWORD property. You can copy the mysql.properties.default file"
  exit
fi
source $PROPS_FILE
if [ -z "$PASSWORD" ];
then
  echo "Properties file exists but does not contain PASSWORD. Please add a MySQL password to your properties file."
  exit
fi
}

check_for_props_and_load

if [ "$1" = "--complete" ]; then
  echo "show databases;" | mysql -u root --password=$PASSWORD
  exit
fi
mislav commented 11 years ago

You should omit outputting errors if --complete was given. Errors should only be used to display to stderr some info useful to the user when they invoke the command in normal mode

bradical commented 11 years ago

So in my example above, how would you propose handling auto-complete in the case where the MySQL password is not found in the properties file or the properties file itself is missing? Perhaps I'm not thinking about auto-complete the right way.

mislav commented 11 years ago

A command in autocomplete mode should only output words for completion, nothing else. In your case, if you don't have db configuration available you should not output anything. In the main command, however (the one you are invoking directly) you should output the warning about configuration not being available.

But you might want to get a different opinion from someone else on this. I'm not entirely sure what's the best practice here

bradical commented 11 years ago

Makes sense. I can separate out the part that checks for the existence of the property needed to do autocomplete from the part that generates an error message.

On Oct 31, 2012, at 6:16 PM, Mislav Marohnić notifications@github.com wrote:

A command in autocomplete mode should only output words for completion, nothing else. In your case, if you don't have db configuration available you should not output anything. In the main command, however (the one you are invoking directly) you should output the warning about configuration not being available.

But you might want to get a different opinion from someone else on this. I'm not entirely sure what's the best practice here — Reply to this email directly or view it on GitHub.

bradical commented 11 years ago

I came up with another solution that works to at least preserve the message as a whole: write the error message to stderr instead of stdout:

PROPS_FILE=$_SUB_ROOT/share/sub/mysql.properties
check_for_props_and_load() {
if [ ! -f $PROPS_FILE ];
then
  echo "MySQL properties file not found. Please create a file in share/sub directory with at least a PASSWORD property. You can copy the mysql.properties.default file" >&2
  exit
fi
source $PROPS_FILE
if [ -z "$PASSWORD" ];
then
  echo "Properties file exists but does not contain PASSWORD. Please add a MySQL password to your properties file." >&2
  exit
fi
}

check_for_props_and_load

if [ "$1" = "--complete" ]; then
  echo "show databases;" | mysql -u root --password=$PASSWORD
  exit
fi

... rest of script ....
mislav commented 11 years ago

BTW if there's not a password property, you might consider trying to use an empty password first. It's common practice to set up a database on the development machine with an empty password (because you don't need security locally).

Convention over configuration