more cli hackery

almost there now, parsing strings to standard types

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 !=
+  why don't we get an error for out not set?
-  NULL' failed, we are not building the option group correctly
 
 
 
-- add something to vips.c to let us call operations via VipsOperation
+- make something for Python as well
-
-  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 <action> [flags] [arguments]
-  
-
-  see vips_call_argv() in operation.c
-
-  how do we make argv strings into VipsImage? unclear
-
-  make something for Python as well
 
   wrap new API for C++
 

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, 

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
+int
-vips_object_set_arg( VipsObject *object, const char *name, const char *value )
+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 };
 
+	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 )) )

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
+	if( vips_object_set_argument_from_string( VIPS_OBJECT( operation ), 
-	 * use vips_object_from_string() to create it. Otherwise, just set as
+		pspec->name, value ) )
-	 * a property.
+		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_set_argument_from_string( VIPS_OBJECT( operation ), 
-
+		option_name, value ) )
-	/*
-	if( !vips_object_has_arg( VIPS_OBJECT( operation ), 
-		option_name, value ) ) {
-		g_set_error( error, "vips", -1, "bad argument" );
 		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, 
+	group = g_option_group_new( object_class->nickname, 
-		VIPS_OBJECT( operation )->description, 
+		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] ) ) {
+		if( vips_call_argv_set_required( operation, argv[i] ) ) 
-			g_object_unref( operation );
 			return( -1 );
-		}
 
-	if( vips_object_build( VIPS_OBJECT( operation ) ) ) {
+	if( vips_object_build( VIPS_OBJECT( operation ) ) ) 
-		g_object_unref( operation );
 		return( -1 );
-	}
-
-	g_object_unref( operation );
 
 	return( 0 );
 }

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();