Thursday, May 26, 2011

Re: Compiling Vim with X on Mac

# HG changeset patch
# Parent e9538cfd0d9ca8e5024bbfc205a2789199bef42d

diff --git a/src/globals.h b/src/globals.h
--- a/src/globals.h
+++ b/src/globals.h
@@ -850,7 +850,7 @@

#endif /* FEAT_MBYTE */

-#ifdef FEAT_XIM
+#if defined(FEAT_XIM) && !defined(NO_X11_INCLUDES)
# ifdef FEAT_GUI_GTK
EXTERN GtkIMContext *xic INIT(= NULL);
/*
@@ -1271,16 +1271,16 @@
EXTERN linenr_T printer_page_num;
#endif

-#ifdef FEAT_XCLIPBOARD
+#if defined(FEAT_XCLIPBOARD) && !defined(NO_X11_INCLUDES)
EXTERN char *xterm_display INIT(= NULL); /* xterm display name; points
into argv[] */
EXTERN Display *xterm_dpy INIT(= NULL); /* xterm display pointer */
#endif
-#if defined(FEAT_XCLIPBOARD) || defined(FEAT_GUI_X11)
+#if (defined(FEAT_XCLIPBOARD) || defined(FEAT_GUI_X11)) && !defined(NO_X11_INCLUDES)
EXTERN XtAppContext app_context INIT(= (XtAppContext)NULL);
#endif

-#ifdef FEAT_GUI_GTK
+#if defined(FEAT_GUI_GTK) && !defined(NO_X11_INCLUDES)
EXTERN guint32 gtk_socket_id INIT(= 0);
EXTERN int echo_wid_arg INIT(= FALSE); /* --echo-wid argument */
#endif
@@ -1300,15 +1300,16 @@

#ifdef FEAT_CLIENTSERVER
EXTERN char_u *serverName INIT(= NULL); /* name of the server */
-# ifdef FEAT_X11
+# if defined(FEAT_X11) && !defined(NO_X11_INCLUDES)
EXTERN Window commWindow INIT(= None);
EXTERN Window clientWindow INIT(= None);
EXTERN Atom commProperty INIT(= None);
EXTERN char_u *serverDelayedStartName INIT(= NULL);
-# else
-# ifdef PROTO
+# endif
+# ifdef WIN32
+# ifdef PROTO
typedef int HWND;
-# endif
+# endif
EXTERN HWND clientWindow INIT(= 0);
# endif
#endif
diff --git a/src/gui.h b/src/gui.h
--- a/src/gui.h
+++ b/src/gui.h
@@ -7,6 +7,12 @@
* Do ":help credits" in Vim to see a list of people who contributed.
*/

+/* NO_X11_INCLUDES implies that no GUI stuff will be available for X11 GUIs. */
+#if !defined(NO_X11_INCLUDES) || \
+ (!defined(FEAT_GUI_ATHENA) && \
+ !defined(FEAT_GUI_GTK) && \
+ !defined(FEAT_GUI_MOTIF))
+
#ifdef FEAT_GUI_MOTIF
# include <Xm/Xm.h>
#endif
@@ -535,3 +541,5 @@
# define CONVERT_FROM_UTF8(String) (String)
# define CONVERT_FROM_UTF8_FREE(String) ((String) = (char_u *)NULL)
#endif /* FEAT_GUI_GTK */
+
+#endif /* !NO_X11_INCLUDES */
diff --git a/src/proto.h b/src/proto.h
--- a/src/proto.h
+++ b/src/proto.h
@@ -20,15 +20,15 @@
* Machine-dependent routines.
*/
/* avoid errors in function prototypes */
-# if !defined(FEAT_X11) && !defined(FEAT_GUI_GTK)
+# if (!defined(FEAT_X11) && !defined(FEAT_GUI_GTK)) || defined(NO_X11_INCLUDES)
# define Display int
# define Widget int
# endif
-# ifndef FEAT_GUI_GTK
+# if !defined(FEAT_GUI_GTK) || defined(NO_X11_INCLUDES)
# define GdkEvent int
# define GdkEventKey int
# endif
-# ifndef FEAT_X11
+# if !defined(FEAT_X11) || defined(NO_X11_INCLUDES)
# define XImage int
# endif

@@ -197,7 +197,7 @@

/* Ugly solution for "BalloonEval" not being defined while it's used in some
* .pro files. */
-# ifndef FEAT_BEVAL
+# if !defined(FEAT_BEVAL) || defined(NO_X11_INCLUDES)
# define BalloonEval int
# endif

@@ -206,7 +206,12 @@
# endif

# ifdef FEAT_GUI
-# include "gui.pro"
+# if !defined(NO_X11_INCLUDES) || \
+ (!defined(FEAT_GUI_ATHENA) && \
+ !defined(FEAT_GUI_GTK) && \
+ !defined(FEAT_GUI_MOTIF))
+# include "gui.pro"
+# endif
# if defined(UNIX) || defined(MACOS)
# include "pty.pro"
# endif
@@ -222,15 +227,15 @@
# ifdef FEAT_GUI_W32
# include "gui_w32.pro"
# endif
-# ifdef FEAT_GUI_GTK
+# if defined(FEAT_GUI_GTK) && !defined(NO_X11_INCLUDES)
# include "gui_gtk.pro"
# include "gui_gtk_x11.pro"
# endif
-# ifdef FEAT_GUI_MOTIF
+# if defined(FEAT_GUI_MOTIF) && !defined(NO_X11_INCLUDES)
# include "gui_motif.pro"
# include "gui_xmdlg.pro"
# endif
-# ifdef FEAT_GUI_ATHENA
+# if defined(FEAT_GUI_ATHENA) && !defined(NO_X11_INCLUDES)
# include "gui_athena.pro"
# ifdef FEAT_BROWSE
extern char *vim_SelFile __ARGS((Widget toplevel, char *prompt, char *init_path, int (*show_entry)(), int x, int y, guicolor_T fg, guicolor_T bg, guicolor_T scroll_fg, guicolor_T scroll_bg));
@@ -239,7 +244,7 @@
# ifdef FEAT_GUI_MAC
# include "gui_mac.pro"
# endif
-# ifdef FEAT_GUI_X11
+# if defined(FEAT_GUI_X11) && !defined(NO_X11_INCLUDES)
# include "gui_x11.pro"
# endif
# ifdef FEAT_GUI_PHOTON
@@ -253,7 +258,7 @@
# ifdef FEAT_OLE
# include "if_ole.pro"
# endif
-# if defined(FEAT_CLIENTSERVER) && defined(FEAT_X11)
+# if defined(FEAT_CLIENTSERVER) && defined(FEAT_X11) && !defined(NO_X11_INCLUDES)
# include "if_xcmdsrv.pro"
# endif

diff --git a/src/structs.h b/src/structs.h
--- a/src/structs.h
+++ b/src/structs.h
@@ -77,10 +77,15 @@
* This is here because gui.h needs the pos_T and win_T, and win_T needs gui.h
* for scrollbar_T.
*/
-#ifdef FEAT_GUI
+/* NO_X11_INCLUDES implies that no GUI stuff will be available for X11 GUIs. */
+#if defined(FEAT_GUI) && \
+ (!defined(NO_X11_INCLUDES) || \
+ (!defined(FEAT_GUI_ATHENA) && \
+ !defined(FEAT_GUI_GTK) && \
+ !defined(FEAT_GUI_MOTIF)))
# include "gui.h"
#else
-# ifdef FEAT_XCLIPBOARD
+# if defined(FEAT_XCLIPBOARD) && !defined(NO_X11_INCLUDES)
# include <X11/Intrinsic.h>
# endif
# define guicolor_T int /* avoid error in prototypes */
@@ -863,6 +868,8 @@
};
#endif /* FEAT_SYN_HL */

+#ifndef NO_X11_INCLUDES
+/* Structure includes GUI-related types which might not be available. */
/*
* Structure shared between syntax.c, screen.c and gui_x11.c.
*/
@@ -896,6 +903,10 @@
# endif
} ae_u;
} attrentry_T;
+#else
+/* */
+# define attrentry_T int /* avoid error in prototypes */
+#endif

#ifdef USE_ICONV
# ifdef HAVE_ICONV_H
@@ -2112,9 +2123,6 @@
int w_fraction;
int w_prev_fraction_row;

-#ifdef FEAT_GUI
- scrollbar_T w_scrollbars[2]; /* vert. Scrollbars for this window */
-#endif
#ifdef FEAT_LINEBREAK
linenr_T w_nrwidth_line_count; /* line count when ml_nrwidth_width
* was computed. */
@@ -2154,6 +2162,13 @@
#ifdef FEAT_RUBY
void *w_ruby_ref;
#endif
+
+#ifndef NO_X11_INCLUDES
+/* We can't rely on anything GUI-related without X11 includes. */
+# ifdef FEAT_GUI
+ scrollbar_T w_scrollbars[2]; /* vert. Scrollbars for this window */
+# endif
+#endif
};

/*
@@ -2266,7 +2281,7 @@
} cursorentry_T;
#endif /* CURSOR_SHAPE */

-#ifdef FEAT_MENU
+#if defined(FEAT_MENU) && !defined(NO_X11_INCLUDES)

/* Indices into vimmenu_T->strings[] and vimmenu_T->noremap[] for each mode */
#define MENU_INDEX_INVALID -1
@@ -2342,7 +2357,7 @@
#ifdef FEAT_GUI_ATHENA
Pixmap image; /* Toolbar image */
#endif
-#ifdef FEAT_BEVAL_TIP
+#if defined(FEAT_BEVAL_TIP) && !defined(NO_X11_INCLUDES)
BalloonEval *tip; /* tooltip for this menu item */
#endif
#ifdef FEAT_GUI_W16
diff --git a/src/vim.h b/src/vim.h
--- a/src/vim.h
+++ b/src/vim.h
@@ -121,7 +121,7 @@
|| defined(FEAT_GUI_W32) \
|| defined(FEAT_GUI_W16) \
|| defined(FEAT_GUI_PHOTON)
-# if !defined(FEAT_GUI) && !defined(NO_X11_INCLUDES)
+# if !defined(FEAT_GUI)
# define FEAT_GUI
# endif
#endif
@@ -193,38 +193,6 @@
# define FEAT_X11
#endif

-#ifdef NO_X11_INCLUDES
- /* In os_mac_conv.c and os_macosx.m NO_X11_INCLUDES is defined to avoid
- * X11 headers. Disable all X11 related things to avoid conflicts. */
-# ifdef FEAT_X11
-# undef FEAT_X11
-# endif
-# ifdef FEAT_GUI_X11
-# undef FEAT_GUI_X11
-# endif
-# ifdef FEAT_XCLIPBOARD
-# undef FEAT_XCLIPBOARD
-# endif
-# ifdef FEAT_GUI_MOTIF
-# undef FEAT_GUI_MOTIF
-# endif
-# ifdef FEAT_GUI_ATHENA
-# undef FEAT_GUI_ATHENA
-# endif
-# ifdef FEAT_GUI_GTK
-# undef FEAT_GUI_GTK
-# endif
-# ifdef FEAT_BEVAL_TIP
-# undef FEAT_BEVAL_TIP
-# endif
-# ifdef FEAT_XIM
-# undef FEAT_XIM
-# endif
-# ifdef FEAT_CLIENTSERVER
-# undef FEAT_CLIENTSERVER
-# endif
-#endif
-
/* The Mac conversion stuff doesn't work under X11. */
#if defined(FEAT_MBYTE) && defined(MACOS_X)
# define MACOS_CONVERT
@@ -1906,12 +1874,14 @@
short_u state; /* Current selection state */
short_u mode; /* Select by char, word, or line. */

-# if defined(FEAT_GUI_X11) || defined(FEAT_XCLIPBOARD)
+# ifndef NO_X11_INCLUDES
+# if defined(FEAT_GUI_X11) || defined(FEAT_XCLIPBOARD)
Atom sel_atom; /* PRIMARY/CLIPBOARD selection ID */
-# endif
+# endif

-# ifdef FEAT_GUI_GTK
+# ifdef FEAT_GUI_GTK
GdkAtom gtk_sel_atom; /* PRIMARY/CLIPBOARD selection ID */
+# endif
# endif

# ifdef MSWIN
>> I build with a standard toolset using ./configure and make.
>> NO_X11_INCLUDES is only defined in and by the two Mac-specific source
>> files named above. As one of the code comments says, this is to avoid a
>> clash between definitions of Boolean in the X11 and Mac header files.
>> All the actual GTK components compile with the X11 headers included (and
>> not any Mac headers).
>
> OK, I got confused. I think in vim.h FEAT_GUI should be defined even
> when NO_X11_INCLUDES is defined. If there is some place where FEAT_GUI
> causes trouble then add a check for NO_X11_INCLUDES there. That should
> be a lot cleaner an avoids unexpected side effects. NO_X11_INCLUDES
> suggests only the includes are skipped, nothing else.

I agree. It is a much bigger and more complicated patch, though, as not
including those files has a lot of side-effects in terms of which
structures can be defined. I've done what I think is most sensible in
the attached patch. What do you think?

I've checked it compiles with the GTK and MacVim GUIs (a couple of
clashes with MacVim's changes, but easy enough to resolve).

I can't check with the Carbon GUI as that doesn't compile on my system
anyway. I'm not sure why that is. Maybe it's using APIs that are too
old. That raises an interestiong question: does anybody use it, and if
not, is the NO_X11_INCLUDES thing even necessary any more? Maybe it
isn't, and we could deprecate and remove the Carbon GUI entirely, and
get rid of these Mac-specific hacks. MacVim essentially replaces the old
Carbon GUI anyway.

There may be a few Athena or Motif things that slipped through where a
NO_X11_INCLUDES check is needed but which I didn't add.

>>> Is this already in src/INSTALLmac.txt? If not, can you add it?
>>
>> I don't think INSTALLmac.txt needs any modification. It does use
>> --disable-darwin to compile an X11 GUI, which I don't do (I seem to
>> remember that it disabled something I wanted--maybe native Mac clipboard
>> support--I can't remember). At any rate, it is nicer not to need to
>> supply it, and perhaps future Mac-specific features will rely on it that
>> we don't want to disable just because the GUI is GTK. So perhaps it
>> would be appropriate to remove --disable-darwin from INSTALLmac.txt, but
>> I'm unsure of exactly what the consequences are, and it's certainly not
>> necessary to change it.
>
> I would rather only make changes if we know what we are doing :-).

Absolutely! If and when we have more Darwin-specific features that
others want/need with an X11 GUI, we can remove it. With MacVim on the
scene now, and getting better and better, the need for the X11 GUIs on
OS X is decreasing anyway.

Ben.

No comments: