From ee2b1f71ce7c3829a4eba37a3e2417d9277a4aad Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 31 Dec 2012 14:10:54 +0000 Subject: [PATCH] better option parsing for "vips" The vips driver program was parsing options in a single pass. This failed if an option came in two parts, for example: vips --plugin x.plg list the argument to --plug would be picked up as the action for "vips", since actions were selected before option parsing Now we parse in two passes: the first pass picks up options for vips itself and for the libvips library, then we select the action, then we parse again, including any options created by the action --- ChangeLog | 3 + configure.in | 6 +- libvips/deprecated/package.c | 14 ++++- tools/vips.c | 112 ++++++++++++++++++++++++----------- 4 files changed, 95 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9be0bcae..b83ec94a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +31/12/12 started 7.30.7 +- better option parsing for "vips", thanks Haida + 14/11/12 started 7.30.6 - capture tiff warnings earlier diff --git a/configure.in b/configure.in index dade4754..3ee86630 100644 --- a/configure.in +++ b/configure.in @@ -2,7 +2,7 @@ # also update the version number in the m4 macros below -AC_INIT([vips], [7.30.6], [vipsip@jiscmail.ac.uk]) +AC_INIT([vips], [7.30.7], [vipsip@jiscmail.ac.uk]) # required for gobject-introspection AC_PREREQ(2.62) @@ -17,7 +17,7 @@ AC_CONFIG_MACRO_DIR([m4]) # user-visible library versioning m4_define([vips_major_version], [7]) m4_define([vips_minor_version], [30]) -m4_define([vips_micro_version], [6]) +m4_define([vips_micro_version], [7]) m4_define([vips_version], [vips_major_version.vips_minor_version.vips_micro_version]) @@ -37,7 +37,7 @@ VIPS_VERSION_STRING=$VIPS_VERSION-`date` # binary interface changes not backwards compatible?: reset age to 0 LIBRARY_CURRENT=33 -LIBRARY_REVISION=8 +LIBRARY_REVISION=9 LIBRARY_AGE=2 # patched into include/vips/version.h diff --git a/libvips/deprecated/package.c b/libvips/deprecated/package.c index 4ef3e048..38fbd150 100644 --- a/libvips/deprecated/package.c +++ b/libvips/deprecated/package.c @@ -38,6 +38,10 @@ */ +/* +#define DEBUG + */ + #ifdef HAVE_CONFIG_H #include #endif /*HAVE_CONFIG_H*/ @@ -631,6 +635,10 @@ im_load_plugin( const char *name ) { Plugin *plug; +#ifdef DEBUG + printf( "im_load_plugin: \"%s\"\n", name ); +#endif /*DEBUG*/ + if( !g_module_supported() ) { vips_error( "plugin", "%s", _( "plugins not supported on this platform" ) ); @@ -683,7 +691,7 @@ im_load_plugin( const char *name ) } #ifdef DEBUG - printf( "added package \"%s\" ...\n", plug->pack->name ); + printf( "added package \"%s\"\n", plug->pack->name ); #endif /*DEBUG*/ return( plug->pack ); @@ -710,6 +718,10 @@ im_load_plugins( const char *fmt, ... ) (void) im_vsnprintf( dir_name, PATH_MAX - 1, fmt, ap ); va_end( ap ); +#ifdef DEBUG + printf( "im_load_plugins: searching \"%s\"\n", dir_name ); +#endif /*DEBUG*/ + if( !(dir = g_dir_open( dir_name, 0, NULL )) ) /* Silent success for dir not there. */ diff --git a/tools/vips.c b/tools/vips.c index 0a25b847..02b6058a 100644 --- a/tools/vips.c +++ b/tools/vips.c @@ -38,6 +38,8 @@ * 6/2/12 * - long arg names in decls to help SWIG * - don't wrap im_remainderconst_vec() + * 31/12/12 + * - parse options in two passes (thanks Haida) */ /* @@ -953,16 +955,6 @@ parse_options( GOptionContext *context, int *argc, char **argv ) error_exit( NULL ); } - /* We support --plugin and --version for all cases. - */ - if( main_option_plugin ) { - if( !im_load_plugin( main_option_plugin ) ) - error_exit( NULL ); - } - - if( main_option_version ) - printf( "vips-%s\n", im_version_string() ); - /* Remove any "--" argument. If one of our arguments is a negative * number, the user will need to have added the "--" flag to stop * GOption parsing. But "--" is still passed down to us and we need to @@ -978,17 +970,16 @@ parse_options( GOptionContext *context, int *argc, char **argv ) } static GOptionGroup * -add_main_group( GOptionContext *context, VipsOperation *user_data ) +add_operation_group( GOptionContext *context, VipsOperation *user_data ) { - GOptionGroup *main_group; + GOptionGroup *group; - main_group = g_option_group_new( NULL, NULL, NULL, user_data, NULL ); - g_option_group_add_entries( main_group, main_option ); - g_option_group_set_translation_domain( main_group, GETTEXT_PACKAGE ); - g_option_context_set_main_group( context, main_group ); - g_option_context_add_group( context, im_get_option_group() ); + group = g_option_group_new( "operation", + _( "Operation" ), _( "Operation help" ), user_data, NULL ); + g_option_group_set_translation_domain( group, GETTEXT_PACKAGE ); + g_option_context_add_group( context, group ); - return( main_group ); + return( group ); } /* VIPS universal main program. @@ -999,11 +990,14 @@ main( int argc, char **argv ) char *action; GOptionContext *context; GOptionGroup *main_group; + GOptionGroup *group; VipsOperation *operation; im_function *fn; int i, j; gboolean handled; + GError *error = NULL; + if( im_init_world( argv[0] ) ) error_exit( NULL ); textdomain( GETTEXT_PACKAGE ); @@ -1021,8 +1015,59 @@ main( int argc, char **argv ) fprintf( stderr, "** DEBUG_FATAL\n" ); #endif /*!DEBUG_FATAL*/ + context = g_option_context_new( _( "[ACTION] [OPTIONS] [PARAMETERS] - " + "VIPS driver program" ) ); + + /* Add and parse the outermost options: the ones this program uses. + * For example, we need + * to be able to spot that in the case of "--plugin ./poop.plg" we + * must remove two args. + */ + main_group = g_option_group_new( NULL, NULL, NULL, NULL, NULL ); + g_option_group_add_entries( main_group, main_option ); + g_option_group_set_translation_domain( main_group, GETTEXT_PACKAGE ); + g_option_context_set_main_group( context, main_group ); + + /* Add the libvips options too. + */ + g_option_context_add_group( context, im_get_option_group() ); + + /* We add more options later, for example as options to vips8 + * operations. Ignore any unknown options in this first parse. + */ + g_option_context_set_ignore_unknown_options( context, TRUE ); + + /* Also disable help output: we want to be able to display full help + * in a second pass after all options have been created. + */ + g_option_context_set_help_enabled( context, FALSE ); + + if( !g_option_context_parse( context, &argc, &argv, &error ) ) { + if( error ) { + fprintf( stderr, "%s\n", error->message ); + g_error_free( error ); + } + + error_exit( NULL ); + } + + if( main_option_plugin ) { + if( !im_load_plugin( main_option_plugin ) ) + error_exit( NULL ); + } + + if( main_option_version ) + printf( "vips-%s\n", im_version_string() ); + + /* Reenable help and unknown option detection ready for the second + * option parse. + */ + g_option_context_set_ignore_unknown_options( context, FALSE ); + g_option_context_set_help_enabled( context, TRUE ); + /* Try to find our action. */ + handled = FALSE; action = NULL; /* Should we try to run the thing we are named as? @@ -1032,7 +1077,8 @@ main( int argc, char **argv ) if( !action ) { /* Look for the first non-option argument, if any, and make - * that our action. + * that our action. The parse above will have removed most of + * them, but --help (for example) could still remain. */ for( i = 1; i < argc; i++ ) if( argv[i][0] != '-' ) { @@ -1048,17 +1094,13 @@ main( int argc, char **argv ) } } - context = g_option_context_new( _( "[ACTION] [OPTIONS] [PARAMETERS] - " - "VIPS driver program" ) ); - handled = FALSE; - /* Could be one of our built-in actions. */ if( action ) for( i = 0; i < VIPS_NUMBER( actions ); i++ ) if( strcmp( action, actions[i].name ) == 0 ) { - main_group = add_main_group( context, NULL ); - g_option_group_add_entries( main_group, + group = add_operation_group( context, NULL ); + g_option_group_add_entries( group, actions[i].group ); parse_options( context, &argc, argv ); @@ -1076,9 +1118,6 @@ main( int argc, char **argv ) if( action && !handled && (fn = im_find_function( action )) ) { - (void) add_main_group( context, NULL ); - parse_options( context, &argc, argv ); - if( im_run_command( action, argc - 1, argv + 1 ) ) { if( argc == 1 ) usage( fn ); @@ -1100,8 +1139,8 @@ main( int argc, char **argv ) if( action && !handled && (operation = vips_operation_new( action )) ) { - main_group = add_main_group( context, operation ); - vips_call_options( main_group, operation ); + group = add_operation_group( context, operation ); + vips_call_options( group, operation ); parse_options( context, &argc, argv ); if( vips_call_argv( operation, argc - 1, argv + 1 ) ) { @@ -1121,12 +1160,18 @@ main( int argc, char **argv ) handled = TRUE; } - /* vips_operation_new() set an error msg for unknown operation. + /* vips_operation_new() sets an error msg for unknown operation. */ if( action && !handled ) im_error_clear(); + /* Still not handled? We may not have called parse_options(), so + * --help args may not have been processed. + */ + if( !handled ) + parse_options( context, &argc, argv ); + if( action && !handled ) { printf( "%s", _( "possible actions:\n" ) ); @@ -1139,11 +1184,6 @@ main( int argc, char **argv ) error_exit( _( "unknown action \"%s\"" ), action ); } - if( !handled ) { - (void) add_main_group( context, NULL ); - parse_options( context, &argc, argv ); - } - g_option_context_free( context ); vips_shutdown();