Re: gtksourceview requires libgnome

[Gustavo Giráldez <gustavo giraldez gmx net>]

Hi Michael, 

On Sat, 2003-05-10 at 04:15, Michael Terry wrote:
> Try the attached code for size.
> 
> It removes the libgnome dependency, adding one header file for some 
> stolen code from it.  It also makes building the test programs optional 
> (controlled by an --enable-build-tests flag) since test-widget depends 
> on gnome-vfs.  This way, a default build will not build the tests and 
> does not need anything from gnome.  But, you can build the tests if ya like.

I see the default behavior is to not build the test programs.  I'd like
it to be the other way around, and call the option
--disable-build-tests.  If you want you can still disable the build
automatically by testing for the presence of gnome-vfs, if configure
failing with no special options is your concern.

Also, instead of adding a new header file, I would prefer to not use
boilerplate macros at all, and define the class initialization functions
in the source files, just as it's done for most of the other classes.

> It seemed to work for me, and passed a 'make distcheck.'
> 
> One question I have is, should you still put stuff in ~/.gnome2?

Even if GtkSourceView has no gnome dependency, *it's still* a Gnome
library to be bundled in the Gnome Desktop release.  Removing the
libgnome dependency is appropriate because it makes no sense to depend
and link to a whole library for one or two function simple calls.  So I
would say yes to your question, for that reason and because there is no
better place (I don't want to pollute the user's home directory with yet
another "dot" entry).  You can always override the directory yourself
when initializing the LanguagesManager object.

More comments inserted in the patch (some hunks snipped).

Regards,
Gustavo


> 
> -mt
> 
> ______________________________________________________________________
> Index: configure.in
> ===================================================================
> RCS file: /cvs/gnome/gtksourceview/configure.in,v
> retrieving revision 1.7
> diff -u -r1.7 configure.in
> --- configure.in	5 May 2003 03:17:13 -0000	1.7
> +++ configure.in	10 May 2003 07:08:16 -0000
> @@ -39,14 +39,26 @@
>  
>  PKG_CHECK_MODULES(DEP,
>  	gtk+-2.0 >= $GTK_REQUIRED_VERSION
> -	libgnome-2.0 >= $GNOME_REQUIRED
>  	libxml-2.0 >= $LIBXML_REQUIRED_VERSION)
>  AC_SUBST(DEP_CFLAGS)
>  AC_SUBST(DEP_LIBS)
>  
> -# Environment
> -GNOME_COMMON_INIT
> -GNOME_COMPILE_WARNINGS(error)

I don't like these to be removed, especially the second.  After all,
this only affects people building from CVS.  The compile warnings are
very useful in early detecting bugs, and I personally like to have my
modules always compiling *without* warnings (hence the -Werror setting
here)..

> +# Do we build optional tests? (they require gnome-vfs)
> +AC_ARG_ENABLE(build_tests,AC_HELP_STRING([--enable-build-tests],
> +					     [enable building of the test programs]),
> +[case "${enableval}" in
> +  yes) build_tests=true ;;
> +  no)  build_tests=false ;;
> +  *) AC_MSG_ERROR(bad value ${enableval} for --enable-build-tests) ;;
> +esac],[build_tests=false])
> +AM_CONDITIONAL(BUILD_TESTS, test x$build_tests = xtrue)
> +
> +if test x$build_tests = xtrue; then
> +	PKG_CHECK_MODULES(TESTS,
> +		gnome-vfs-2.0 >= $GNOME_VFS_REQUIRED_VERSION)
> +	AC_SUBST(TESTS_CFLAGS)
> +	AC_SUBST(TESTS_LIBS)
> +fi
>  
>  # I18N stuff
>  ALL_LINGUAS="es"
> @@ -55,7 +67,6 @@
>  AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE, "$GETTEXT_PACKAGE", [Package name for gettext])
>  
>  AM_GLIB_GNU_GETTEXT
> -AM_GLIB_DEFINE_LOCALEDIR(GNOMELOCALEDIR)

This is an unnecessary change.  You can rename the variable if you want,
but I prefer this be defined in config.h so for example a change in the
installation prefix forces a recompile, which is needed after all.

>  
>  # Output files
>  AC_CONFIG_FILES([
> Index: gtksourceview/Makefile.am
> ===================================================================
> RCS file: /cvs/gnome/gtksourceview/gtksourceview/Makefile.am,v
> retrieving revision 1.10
> diff -u -r1.10 Makefile.am
> --- gtksourceview/Makefile.am	4 May 2003 22:43:57 -0000	1.10
> +++ gtksourceview/Makefile.am	10 May 2003 07:08:16 -0000
> @@ -1,13 +1,15 @@
>  SUBDIRS = language-specs
>  
> +localedir = $(datadir)/locale
> +
>  INCLUDES = 				\
>  	-DDATADIR=\""$(datadir)"\" 	\
> +	-DLOCALEDIR=\""$(localedir)"\"  \

Same as above.

>  	-DG_LOG_DOMAIN=\"GtkSourceView\"\
>  	-DG_DISABLE_DEPRECATED 		\
>  	-DGTK_DISABLE_DEPRECATED 	\
>  	-DGDK_DISABLE_DEPRECATED 	\
>  	-DGDK_PIXBUF_DISABLE_DEPRECATED	\
> -	-DGNOME_DISABLE_DEPRECATED	\
>  	-I$(top_srcdir) -I$(srcdir) 	\
>  	$(WARN_CFLAGS) 			\
>  	$(DEP_CFLAGS)
> @@ -47,6 +49,7 @@
>  	gtksourcestylescheme.c		\
>  	gtksourcemarker.h		\
>  	gtksourcemarker.c		\
> +	gtksourceview-macros.h    \
>  	$(BUILT_SOURCES)
>  
>  libgtksourceview_1_0_la_LIBADD = $(DEP_LIBS)
> Index: gtksourceview/gtksourcelanguagesmanager.c
> ===================================================================
> RCS file: /cvs/gnome/gtksourceview/gtksourceview/gtksourcelanguagesmanager.c,v
> retrieving revision 1.2
> diff -u -r1.2 gtksourcelanguagesmanager.c
> --- gtksourceview/gtksourcelanguagesmanager.c	2 May 2003 17:27:51 -0000	1.2
> +++ gtksourceview/gtksourcelanguagesmanager.c	10 May 2003 07:08:17 -0000
> @@ -24,8 +24,6 @@
>  #include <string.h>
>  
>  #include <libxml/xmlreader.h>
> -#include <libgnome/gnome-util.h>
> -#include <libgnome/gnome-macros.h>
>  
>  #include "gtksourcelanguagesmanager.h"
>  
> @@ -35,6 +33,8 @@
>  #include "gtksourcelanguage.h"
>  #include "gtksourcetag.h"
>  
> +#include "gtksourceview-macros.h"
> +
>  #define DEFAULT_GCONF_BASE_DIR		"/apps/gtksourceview"
>  
>  #define DEFAULT_LANGUAGE_DIR		DATADIR "/gtksourceview-1.0/language-specs"
> @@ -52,8 +52,7 @@
>  	GSList		*language_specs_directories;
>  };
>  
> -
> -GNOME_CLASS_BOILERPLATE (GtkSourceLanguagesManager, 
> +GTK_SOURCE_VIEW_CLASS_BOILERPLATE (GtkSourceLanguagesManager, 
>  			 gtk_source_languages_manager,
>  			 GObject, 
>  			 G_TYPE_OBJECT)
> @@ -196,7 +195,9 @@
>  					g_strdup (DEFAULT_LANGUAGE_DIR));
>  		lm->priv->language_specs_directories = 
>  			g_slist_prepend (lm->priv->language_specs_directories,
> -					gnome_util_home_file (USER_LANGUAGE_DIR));
> +					g_strconcat (g_get_home_dir(), 
> +						"/", ".gnome2", (USER_LANGUAGE_DIR), 
> +						NULL));

The ".gnome2" string should at least be a macro, and probably calculated
at configure time (i.e. to be ready for Gnome 3 ;-)  Also, please use
g_build_filename instead of g_strconcat, since it's more portable.

>  
>  		return;
>  	}
> @@ -336,9 +337,11 @@
>  	while (file_name != NULL)
>  	{
>  		gchar *full_path = g_build_filename (directory, file_name, NULL);
> -
> +		gchar *last_dot = strrchr (full_path, '.');
> +		
>  		if (!g_file_test (full_path, G_FILE_TEST_IS_DIR) && 
> -		    (strcmp (g_extension_pointer (full_path), "lang") == 0))
> +		    last_dot && 
> +		    (strcmp (last_dot + 1, "lang") == 0))
>  			filenames = g_slist_prepend (filenames, full_path);
>  		else
>  			g_free (full_path);
> Index: tests/Makefile.am
> ===================================================================
> RCS file: /cvs/gnome/gtksourceview/tests/Makefile.am,v
> retrieving revision 1.3
> diff -u -r1.3 Makefile.am
> --- tests/Makefile.am	4 May 2003 22:44:01 -0000	1.3
> +++ tests/Makefile.am	10 May 2003 07:08:17 -0000
> @@ -8,7 +8,8 @@
>  	-DGNOME_DISABLE_DEPRECATED	\
>  	-I$(top_srcdir) -I$(srcdir) 	\
>  	$(WARN_CFLAGS) 			\
> -	$(DEP_CFLAGS)
> +	$(DEP_CFLAGS)                  \
> +	$(TESTS_CFLAGS)
>  
>  # testing executables
>  noinst_PROGRAMS = test-widget testregion
> @@ -17,7 +18,8 @@
>  
>  test_widget_LDADD = 			\
>  	$(top_builddir)/gtksourceview/libgtksourceview-1.0.la \
> -	$(DEP_LIBS)
> +	$(DEP_LIBS)                     \
> +	$(TESTS_LIBS)
>  
>  testregion_SOURCES = testregion.c
>  

________________________________________________________________________