From 825280b365fa1469a4d89b8c7ee5113c177d2788 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 13 May 2011 10:08:53 +0100 Subject: [PATCH] more cli hackery almost there now, parsing strings to standard types --- TODO | 55 +++++++++-------------------- libvips/include/vips/object.h | 2 ++ libvips/iofuncs/object.c | 62 ++++++++++++++++++++++++++++---- libvips/iofuncs/operation.c | 66 ++++++++++++----------------------- tools/vips.c | 25 +++++++++++-- 5 files changed, 119 insertions(+), 91 deletions(-) diff --git a/TODO b/TODO index f1188f96..f78d107e 100644 --- a/TODO +++ b/TODO @@ -1,48 +1,27 @@ +- we seem to have code shared between object and operation: + vips_object_set_arg() and vips_object_set_required_test(), common this up + + currently adding stuff to operation.c to set args from strings, with extra + logic for things like create image from filename ... useful for object.c + as well? - try: - vips add + $ vips add + VipsAdd (add), add two images + VipsOperation.add (left, right, out) + where: + left :: VipsImage + right :: VipsImage + out :: VipsImage + check_required: required construct param left to VipsAdd not set + check_required: required construct param right to VipsAdd not set - get GLib-CRITICAL **: g_option_context_add_group: assertion `group->name != - NULL' failed, we are not building the option group correctly + why don't we get an error for out not set? -- add something to vips.c to let us call operations via VipsOperation - - make optional args to operations into command-line switches - - vips add --saturated --expand=12 in1.v in2.v out.v - - means - - vips_call("add", in1, in2, out, "saturated", TRUE, "expand", 12, NULL); - - but how do we integrate this with GOption parsing? - - perhaps the vips program should change, so - - vips --list all - - becomes - - vips list all - - ie. the various flags that performed actions should become "verbs" - - this lets us delay option parsing until we have seen the "verb" part of the - command (which is always argv[1]) and we can build a custom GOptionEntry - - new form: - - vips [flags] [arguments] - - - see vips_call_argv() in operation.c - - how do we make argv strings into VipsImage? unclear - - make something for Python as well +- make something for Python as well wrap new API for C++ diff --git a/libvips/include/vips/object.h b/libvips/include/vips/object.h index 9b2dcfd0..e701fcdf 100644 --- a/libvips/include/vips/object.h +++ b/libvips/include/vips/object.h @@ -268,6 +268,8 @@ GType vips_object_get_type( void ); void vips_object_class_install_argument( VipsObjectClass *, GParamSpec *pspec, VipsArgumentFlags flags, guint offset ); +int vips_object_set_argument_from_string( VipsObject *object, + const char *name, const char *value ); typedef void *(*VipsObjectSetArguments)( VipsObject *, void *, void * ); VipsObject *vips_object_new( GType type, diff --git a/libvips/iofuncs/object.c b/libvips/iofuncs/object.c index c92a2b21..b1ee3b25 100644 --- a/libvips/iofuncs/object.c +++ b/libvips/iofuncs/object.c @@ -970,13 +970,62 @@ vips_object_class_install_argument( VipsObjectClass *object_class, /* Set a named arg from a string. */ -static int -vips_object_set_arg( VipsObject *object, const char *name, const char *value ) +int +vips_object_set_argument_from_string( VipsObject *object, + const char *name, const char *value ) { + VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object ); + + GParamSpec *pspec; + VipsArgumentClass *argument_class; + VipsArgumentInstance *argument_instance; GValue gvalue = { 0 }; - g_value_init( &gvalue, G_TYPE_STRING ); - g_value_set_string( &gvalue, value ); + printf( "vips_object_set_argument_from_string: %s = %s\n", + name, value ); + + pspec = g_object_class_find_property( G_OBJECT_CLASS( class ), name ); + if( !pspec ) { + vips_error( "vips_object_set_argument_from_string", + _( "object %s has no argument %s" ), + G_OBJECT_TYPE_NAME( object ), name ); + return( -1 ); + } + + argument_class = (VipsArgumentClass *) + vips__argument_table_lookup( class->argument_table, pspec ); + argument_instance = + vips__argument_get_instance( argument_class, object ); + + if( G_IS_PARAM_SPEC_OBJECT( pspec ) && + G_PARAM_SPEC_VALUE_TYPE( pspec ) == VIPS_TYPE_IMAGE ) { + VipsImage *image; + char *mode; + + mode = (argument_class->flags & VIPS_ARGUMENT_OUTPUT) ? + "w" : "r"; + if( !(image = vips_image_new_from_file( value, mode )) ) + return( -1 ); + g_value_init( &gvalue, G_TYPE_OBJECT ); + g_value_set_object( &gvalue, image ); + } + else if( G_IS_PARAM_SPEC_BOOLEAN( pspec ) ) { + gboolean b; + + g_value_init( &gvalue, G_TYPE_BOOLEAN ); + b = TRUE; + if( value && + (strcasecmp( value, "false" ) == 0 || + strcasecmp( value, "no" ) == 0 || + strcmp( value, "0" ) == 0) ) + b = FALSE; + g_value_set_boolean( &gvalue, b ); + } + else { + g_value_init( &gvalue, G_TYPE_STRING ); + g_value_set_string( &gvalue, value ); + } + g_object_set_property( G_OBJECT( object ), name, &gvalue ); g_value_unset( &gvalue ); @@ -1012,7 +1061,7 @@ vips_object_set_required( VipsObject *object, const char *value ) return( -1 ); } - if( vips_object_set_arg( object, pspec->name, value ) ) + if( vips_object_set_argument_from_string( object, pspec->name, value ) ) return( -1 ); return( 0 ); @@ -1042,7 +1091,8 @@ vips_object_set_args( VipsObject *object, const char *p ) if( !(p = vips__token_need( p, VIPS_TOKEN_STRING, string2, PATH_MAX )) ) return( -1 ); - if( vips_object_set_arg( object, string, string2 ) ) + if( vips_object_set_argument_from_string( object, + string, string2 ) ) return( -1 ); if( !(p = vips__token_must( p, &token, string2, PATH_MAX )) ) diff --git a/libvips/iofuncs/operation.c b/libvips/iofuncs/operation.c index c5d79a63..0f28ebb3 100644 --- a/libvips/iofuncs/operation.c +++ b/libvips/iofuncs/operation.c @@ -58,6 +58,7 @@ G_DEFINE_ABSTRACT_TYPE( VipsOperation, vips_operation, VIPS_TYPE_OBJECT ); /* What to show about the argument. */ typedef struct { + char *message; /* header message on first print */ gboolean required; /* show required args or optional */ gboolean oftype; /* "is of type" message */ int n; /* Arg number */ @@ -75,6 +76,9 @@ vips_operation_print_arg( VipsObject *object, GParamSpec *pspec, if( print->required == ((argument_class->flags & VIPS_ARGUMENT_REQUIRED) != 0) && !argument_instance->assigned ) { + if( print->message && print->n == 0 ) + vips_buf_appendf( buf, "%s\n", print->message ); + if( print->oftype ) vips_buf_appendf( buf, " %s :: %s\n", pspec->name, @@ -130,7 +134,8 @@ vips_operation_print( VipsObject *object, VipsBuf *buf ) /* First pass through args: show the required names. */ - vips_buf_appendf( buf, "VipsOperation.%s (", object_class->nickname ); + vips_buf_appendf( buf, "%s (", object_class->nickname ); + print.message = NULL; print.required = TRUE; print.oftype = FALSE; print.n = 0; @@ -140,7 +145,7 @@ vips_operation_print( VipsObject *object, VipsBuf *buf ) /* Show required types. */ - vips_buf_appends( buf, "where:\n" ); + print.message = "where:"; print.required = TRUE; print.oftype = TRUE; print.n = 0; @@ -149,7 +154,7 @@ vips_operation_print( VipsObject *object, VipsBuf *buf ) /* Show optional args. */ - vips_buf_appends( buf, "optional arguments:\n" ); + print.message = "optional arguments:"; print.required = FALSE; print.oftype = TRUE; print.n = 0; @@ -351,21 +356,6 @@ vips_call_split( const char *operation_name, va_list optional, ... ) return( result ); } -/* Set a named arg from a string. - */ -static int -vips_object_set_arg( VipsObject *object, const char *name, const char *value ) -{ - GValue gvalue = { 0 }; - - g_value_init( &gvalue, G_TYPE_STRING ); - g_value_set_string( &gvalue, value ); - g_object_set_property( G_OBJECT( object ), name, &gvalue ); - g_value_unset( &gvalue ); - - return( 0 ); -} - static void * vips_object_set_required_test( VipsObject *object, GParamSpec *pspec, @@ -395,11 +385,9 @@ vips_call_argv_set_required( VipsOperation *operation, const char *value ) return( -1 ); } - /* If this arg needs a value that's a subtype of VipsObject, we can - * use vips_object_from_string() to create it. Otherwise, just set as - * a property. - */ - + if( vips_object_set_argument_from_string( VIPS_OBJECT( operation ), + pspec->name, value ) ) + return( -1 ); return( 0 ); } @@ -410,17 +398,9 @@ vips_call_options_set( const gchar *option_name, const gchar *value, { VipsOperation *operation = (VipsOperation *) data; - printf( "setting optional arg %s = %s\n", option_name, value ); - - /* - if( !vips_object_has_arg( VIPS_OBJECT( operation ), - option_name, value ) ) { - g_set_error( error, "vips", -1, "bad argument" ); + if( vips_object_set_argument_from_string( VIPS_OBJECT( operation ), + option_name, value ) ) return( FALSE ); - } - */ - - vips_object_set_arg( VIPS_OBJECT( operation ), option_name, value ); return( TRUE ); } @@ -458,10 +438,11 @@ vips_call_options_add( VipsObject *object, GOptionGroup * vips_call_options( VipsOperation *operation ) { + VipsObjectClass *object_class = VIPS_OBJECT_GET_CLASS( operation ); GOptionGroup *group; - group = g_option_group_new( VIPS_OBJECT( operation )->nickname, - VIPS_OBJECT( operation )->description, + group = g_option_group_new( object_class->nickname, + object_class->description, _( "Show operation options" ), operation, NULL ); @@ -474,28 +455,25 @@ vips_call_options( VipsOperation *operation ) /* Our main command-line entry point. Optional args should have been set by * the GOption parser already, see above. + * + * We don't create the operation, so we must not unref it. The caller must + * unref on error too. */ int vips_call_argv( VipsOperation *operation, int argc, char **argv ) { int i; - g_assert( argc > 0 ); + g_assert( argc >= 0 ); /* Now set required args from the rest of the command-line. */ for( i = 1; i < argc; i++ ) - if( vips_call_argv_set_required( operation, argv[i] ) ) { - g_object_unref( operation ); + if( vips_call_argv_set_required( operation, argv[i] ) ) return( -1 ); - } - if( vips_object_build( VIPS_OBJECT( operation ) ) ) { - g_object_unref( operation ); + if( vips_object_build( VIPS_OBJECT( operation ) ) ) return( -1 ); - } - - g_object_unref( operation ); return( 0 ); } diff --git a/tools/vips.c b/tools/vips.c index b46ef866..affaf93c 100644 --- a/tools/vips.c +++ b/tools/vips.c @@ -931,7 +931,7 @@ parse_options( GOptionContext *context, int *argc, char **argv ) error_exit( "%s", g_get_prgname() ); } - /* We support --plugin for all cases. + /* We support --plugin and --version for all cases. */ if( main_option_plugin ) { if( !im_load_plugin( main_option_plugin ) ) @@ -1059,6 +1059,7 @@ main( int argc, char **argv ) handled = TRUE; } + im_error_clear(); /* Could be a vips8 VipsOperation. */ @@ -1070,15 +1071,33 @@ main( int argc, char **argv ) g_option_context_add_group( context, group ); parse_options( context, &argc, argv ); - if( vips_call_argv( operation, argc, argv ) ) + if( vips_call_argv( operation, argc, argv ) ) { + if( argc == 0 ) { + char *help; + + help = g_option_context_get_help( context, + FALSE, group ); + printf( "%s", help ); + vips_object_print( VIPS_OBJECT( operation ) ); + error_exit( NULL ); + } + error_exit( NULL ); + } + + g_object_unref( operation ); handled = TRUE; } + im_error_clear(); - if( !handled ) + if( !handled ) { parse_options( context, &argc, argv ); + if( argc > 1 ) + error_exit( "unknown argument %s\n", argv[1] ); + } + g_option_context_free( context ); im_close_plugins();