unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
       [not found] ` <E1YHiQk-0004jf-UH@vcs.savannah.gnu.org>
@ 2015-02-01  6:11   ` Dmitry Gutov
  2015-02-01  8:50     ` joakim
  2015-02-01 15:36     ` Eli Zaretskii
  0 siblings, 2 replies; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01  6:11 UTC (permalink / raw)
  To: emacs-devel, Joakim Verona

On 02/01/2015 02:30 AM, Joakim Verona wrote:

>      Better changelog for xwidgets

> diff --git a/ChangeLog b/ChangeLog
> index d7fd76c..7c2e53e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-02-01  Joakim Verona  <joakim@verona.se>
> +	Support for the new Xwidget feature.
> +	* configure.ac:

This ChangeLog entry, as well as ones after it, looks awfully incomplete.

And shouldn't there be more of them?

Also, the current standard is that the commit message should contain the 
full ChangeLog entry.

Given the above, maybe you should revert the merge. Then squash all 
commits in xwidget into one patch, amend it with proper ChangeLog 
entries, and then do the merge.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01  6:11   ` [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets Dmitry Gutov
@ 2015-02-01  8:50     ` joakim
  2015-02-01 10:53       ` Paul Eggert
  2015-02-01 15:36     ` Eli Zaretskii
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-01  8:50 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 02/01/2015 02:30 AM, Joakim Verona wrote:
>
>>      Better changelog for xwidgets
>
>> diff --git a/ChangeLog b/ChangeLog
>> index d7fd76c..7c2e53e 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2015-02-01  Joakim Verona  <joakim@verona.se>
>> +	Support for the new Xwidget feature.
>> +	* configure.ac:
>
> This ChangeLog entry, as well as ones after it, looks awfully incomplete.
>
> And shouldn't there be more of them?
>
> Also, the current standard is that the commit message should contain
> the full ChangeLog entry.
>
> Given the above, maybe you should revert the merge. Then squash all
> commits in xwidget into one patch, amend it with proper ChangeLog
> entries, and then do the merge.
>

Perhaps that would be best.

On the other hand since I demonstratably don't know what a proper Changelog entry
should look like, I would apreciate some help in that regard.


-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01  8:50     ` joakim
@ 2015-02-01 10:53       ` Paul Eggert
  2015-02-01 15:46         ` joakim
  2015-02-03 22:38         ` joakim
  0 siblings, 2 replies; 50+ messages in thread
From: Paul Eggert @ 2015-02-01 10:53 UTC (permalink / raw)
  To: joakim, Dmitry Gutov; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]

joakim@verona.se wrote:
>> Given the above, maybe you should revert the merge. Then squash all
>> >commits in xwidget into one patch, amend it with proper ChangeLog
>> >entries, and then do the merge.
>> >
> Perhaps that would be best.

Yes, please do that.  Here are some other things to do before committing the merge.

* Reindent as per GNU standards.  Start with "indent -gnu" but it won't do a 
perfect job.  E.g., say "char *p" not "char* p".

* Fit it into 80 columns.

* Use GNU style for comments.  These should typically use complete, imperative 
sentences.

* Configure with "./configure --enable-gcc-warnings --with-xwidgets 
--with-x-toolkit=gtk3" and fix all the warnings.

* It's OK to assume C99 now.

* Don't make functions extern unless they need to be extern.  Compilers do a 
better job with static functions, typically.

* Some of those function names are too long; please shorten them.

* A lot of the printf statements look like they shouldn't be there.

* There's some commented-out code that should be removed.

* Omit pointer casts that aren't needed (when casting to and from void *).

I started to look into all that and came up with the attached patch, relative to 
commit 9fe732a02afbe0b3d4a85d2bcae687900ab881f7; please have a look.  But the 
result still doesn't compile due to warnings and I'm sure I missed a lot of 
things.  I hope you can finish the job.  (Also, the ChangeLog entries need to be 
written -- I started on that but it's a big job and it's something the author of 
the patch really should do.)

[-- Attachment #2: 0001-First-cut-at-xwidget-fixes.patch --]
[-- Type: text/x-patch, Size: 136172 bytes --]

From df74bc7f211e165e231087e3d968b1cd661e2d70 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 1 Feb 2015 02:48:57 -0800
Subject: [PATCH] First cut at xwidget fixes.

---
 ChangeLog           |    6 +-
 configure.ac        |   28 +-
 etc/NEWS            |    8 +-
 src/ChangeLog       |    1 +
 src/buffer.c        |    7 +-
 src/coding.c        |    2 +-
 src/dispextern.h    |   14 +-
 src/dispnew.c       |   10 +-
 src/emacs.c         |    5 +-
 src/emacsgtkfixed.c |   88 +--
 src/keyboard.c      |    9 +-
 src/lisp.h          |    1 +
 src/print.c         |    2 +-
 src/termhooks.h     |    1 +
 src/window.c        |    4 +-
 src/xdisp.c         |   83 +--
 src/xterm.c         |   11 +-
 src/xwidget.c       | 1999 ++++++++++++++++++++++++++-------------------------
 src/xwidget.h       |  115 +--
 19 files changed, 1189 insertions(+), 1205 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7c2e53e..8d820ec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,10 @@
 2015-02-01  Joakim Verona  <joakim@verona.se>
+
 	Support for the new Xwidget feature.
-	* configure.ac:
+	* configure.ac (xwidgets): New option.
+	(HAVE_XWIDGETS, HAVE_WEBKIT, HAVE_GIR, WEBKIT_REQUIRED)
+	(WEBKIT_MODULES, GIR_REQUIRED, GIR_MODULES, HAVE_GIR): New vars.
+	(TOOLKIT_LIBW): Append -lXcomposite if --with-x-toolkit=no.
 
 2015-01-28  Paul Eggert  <eggert@cs.ucla.edu>
 
diff --git a/configure.ac b/configure.ac
index f4fcf52..9087035 100644
--- a/configure.ac
+++ b/configure.ac
@@ -374,7 +374,8 @@ otherwise for the first of `gfile' or `inotify' that is usable.])
  ],
  [with_file_notification=$with_features])
 
-OPTION_DEFAULT_OFF([xwidgets],[enable use of some gtk widgets it Emacs buffers])
+OPTION_DEFAULT_OFF([xwidgets],
+  [enable use of some gtk widgets in Emacs buffers])
 
 ## For the times when you want to build Emacs but don't have
 ## a suitable makeinfo, and can live without the manuals.
@@ -2534,23 +2535,24 @@ HAVE_XWIDGETS=no
 HAVE_WEBKIT=no
 HAVE_GIR=no
 
-if test "${with_xwidgets}" != "no" && test "${USE_GTK_TOOLKIT}" = "GTK3"  && test "$window_system" != "none" ; then
-   echo "xwidgets enabled, checking webkit, and others"
-   HAVE_XWIDGETS=yes
-   AC_DEFINE(HAVE_XWIDGETS, 1, [Define to 1 if you have xwidgets support.])
-dnl xwidgets
-dnl - enable only if GTK3 is enabled, and we have a window system
-dnl - check for webkit and gobject introspection
+if test "$with_xwidgets" != "no" && test "$USE_GTK_TOOLKIT" = "GTK3" &&
+   test "$window_system" != "none"
+then
+  HAVE_XWIDGETS=yes
+  AC_DEFINE([HAVE_XWIDGETS], 1, [Define to 1 if you have xwidgets support.])
 
-
-#webkit version for gtk3.
+  dnl xwidgets
+  dnl - enable only if GTK3 is enabled, and we have a window system
+  dnl - check for webkit and gobject introspection
+  dnl webkit version for gtk3.
   WEBKIT_REQUIRED=1.4.0
   WEBKIT_MODULES="webkitgtk-3.0 >= $WEBKIT_REQUIRED"
 
   if test "${with_gtk3}" = "yes"; then
     PKG_CHECK_MODULES(WEBKIT, $WEBKIT_MODULES, HAVE_WEBKIT=yes, HAVE_WEBKIT=no)
     if test $HAVE_WEBKIT = yes; then
-        AC_DEFINE(HAVE_WEBKIT_OSR, 1, [Define to 1 if you have webkit_osr support.])
+        AC_DEFINE([HAVE_WEBKIT_OSR], 1,
+	  [Define to 1 if you have webkit_osr support.])
     fi
   fi
 
@@ -2558,10 +2560,8 @@ dnl - check for webkit and gobject introspection
   GIR_MODULES="gobject-introspection-1.0 >= $GIR_REQUIRED"
   PKG_CHECK_MODULES(GIR, $GIR_MODULES, HAVE_GIR=yes, HAVE_GIR=no)
   if test $HAVE_GIR = yes; then
-     AC_DEFINE(HAVE_GIR, 1, [Define to 1 if you have GIR support.])
+     AC_DEFINE([HAVE_GIR], 1, [Define to 1 if you have GIR support.])
   fi
-
-
 fi
 
 CFLAGS=$OLD_CFLAGS
diff --git a/etc/NEWS b/etc/NEWS
index 4371a01..dd4b7c5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -65,9 +65,11 @@ so if you want to use it, you can always take a copy from an older Emacs.
 
 \f
 * Changes in Emacs 25.1
-** Xwidgets : A new feature for embedding native widgets
-inside Emacs buffers. If you have gtk3 and webkit-devel installed,
-you can try the embedded webkit browser with m-x xwidget-webkit-browse-url.
+
+** Xwidgets: A new feature for embedding native widgets
+inside Emacs buffers.  If you have gtk3 and webkit-devel installed,
+you can try the embedded webkit browser with M-x xwidget-webkit-browse-url.
+
 ** `package-install-from-buffer' and `package-install-file' work on directories.
 This follows the same rules as installing from a .tar file, except the
 -pkg file is optional.
diff --git a/src/ChangeLog b/src/ChangeLog
index 5635e1b..8da6c38 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,4 +1,5 @@
 2015-02-01  Joakim Verona  <joakim@verona.se>
+
 	Support for the new Xwidget feature.
 	* window.c, Makefile.in, buffer.c, dispextern.h, dispnew.c, emacs.c:
 	* emacsgtkfixed.c, emacsgtkfixed.h, keyboard.c, lisp.h, print.c:
diff --git a/src/buffer.c b/src/buffer.c
index 223683d..d185685 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -43,8 +43,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "frame.h"
 
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
-#endif  /* HAVE_XWIDGETS */
+# include "xwidget.h"
+#endif
 #ifdef WINDOWSNT
 #include "w32heap.h"		/* for mmap_* */
 #endif
@@ -1762,7 +1762,8 @@ cleaning up all windows currently displaying the buffer to be killed. */)
   GCPRO1 (buffer);
   kill_buffer_xwidgets (buffer);
   UNGCPRO;
-#endif  /* HAVE_XWIDGETS */
+#endif
+
   /* Killing buffer processes may run sentinels which may have killed
      our buffer.  */
   if (!BUFFER_LIVE_P (b))
diff --git a/src/coding.c b/src/coding.c
index 1a0e1279..8f291d6 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -5985,7 +5985,7 @@ bool
 raw_text_coding_system_p (struct coding_system *coding)
 {
   return (coding->decoder == decode_coding_raw_text
-	  && coding->encoder == encode_coding_raw_text) ? true : false;
+	  && coding->encoder == encode_coding_raw_text);
 }
 
 
diff --git a/src/dispextern.h b/src/dispextern.h
index fbf0c74..76c5bc5 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -347,10 +347,11 @@ enum glyph_type
   IMAGE_GLYPH,
 
   /* Glyph is a space of fractional width and/or height.  */
-  STRETCH_GLYPH
+  STRETCH_GLYPH,
+
 #ifdef HAVE_XWIDGETS
   /* Glyph is an external widget drawn by the GUI toolkit.   */
-  ,XWIDGET_GLYPH
+  XWIDGET_GLYPH,
 #endif
 };
 
@@ -504,7 +505,7 @@ struct glyph
     int img_id;
 
 #ifdef HAVE_XWIDGETS
-    struct xwidget* xwidget;
+    struct xwidget *xwidget;
 #endif
     /* Sub-structure for type == STRETCH_GLYPH.  */
     struct
@@ -1361,7 +1362,7 @@ struct glyph_string
   struct image *img;
 
 #ifdef HAVE_XWIDGETS
-  struct xwidget* xwidget;
+  struct xwidget *xwidget;
 #endif
   /* Slice */
   struct glyph_slice slice;
@@ -2529,9 +2530,10 @@ struct it
   ptrdiff_t image_id;
 
 #ifdef HAVE_XWIDGETS
-  /* If what == IT_XWIDGET*/
-  struct xwidget* xwidget;
+  /* If what == IT_XWIDGET.  */
+  struct xwidget *xwidget;
 #endif
+
   /* Values from `slice' property.  */
   struct it_slice slice;
 
diff --git a/src/dispnew.c b/src/dispnew.c
index e614cee..2f27e17 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -49,7 +49,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #endif /* HAVE_WINDOW_SYSTEM */
 
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
 
 #include <errno.h>
@@ -3547,8 +3547,9 @@ update_window (struct window *w, bool force_p)
 #endif
 
 #ifdef HAVE_XWIDGETS
-  xwidget_end_redisplay(w, w->current_matrix);
+  xwidget_end_redisplay (w, w->current_matrix);
 #endif
+
   clear_glyph_matrix (desired_matrix);
 
   return paused_p;
@@ -4133,8 +4134,9 @@ scrolling_window (struct window *w, bool header_line_p)
     }
 
 #ifdef HAVE_XWIDGETS
- //currently this is needed to detect xwidget movement reliably. or probably not.
-    return 0;
+  /* Currently this is needed to detect xwidget movement reliably. or
+     probably not.  */
+  return 0;
 #endif
 
   /* Give up if some rows in the desired matrix are not enabled.  */
diff --git a/src/emacs.c b/src/emacs.c
index 87b1f11..eca5d6a 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -66,8 +66,9 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "window.h"
 
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
+
 #include "systty.h"
 #include "atimer.h"
 #include "blockinput.h"
@@ -1438,7 +1439,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       syms_of_xmenu ();
       syms_of_fontset ();
 #ifdef HAVE_XWIDGETS
-      syms_of_xwidget();
+      syms_of_xwidget ();
 #endif
       syms_of_xsettings ();
 #ifdef HAVE_X_SM
diff --git a/src/emacsgtkfixed.c b/src/emacsgtkfixed.c
index d52a139..608d91a 100644
--- a/src/emacsgtkfixed.c
+++ b/src/emacsgtkfixed.c
@@ -34,27 +34,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 # pragma GCC diagnostic ignored "-Wunused-local-typedefs"
 #endif
 
-//#define EMACS_TYPE_FIXED emacs_fixed_get_type ()
-/* #define EMACS_FIXED(obj) \ */
-/*   G_TYPE_CHECK_INSTANCE_CAST (obj, EMACS_TYPE_FIXED, EmacsFixed) */
-
 typedef struct _EmacsFixed EmacsFixed;
 typedef struct _EmacsFixedPrivate EmacsFixedPrivate;
 typedef struct _EmacsFixedClass EmacsFixedClass;
 
-/* struct _EmacsFixed */
-/* { */
-/*   GtkFixed container; */
-
-/*   /\*< private >*\/ */
-/*   EmacsFixedPrivate *priv; */
-/* }; */
-
-/* struct _EmacsFixedClass */
-/* { */
-/*   GtkFixedClass parent_class; */
-/* }; */
-
 struct _EmacsFixedPrivate
 {
   struct frame *f;
@@ -77,27 +60,21 @@ struct GtkFixedPrivateL
 };
 
 static void emacs_fixed_gtk_widget_size_allocate (GtkWidget *widget,
-                                           GtkAllocation *allocation){
+						  GtkAllocation *allocation)
+{
   //for xwidgets
 
   //TODO 1st call base class method
-  EmacsFixedClass *klass;
-  GtkWidgetClass *parent_class;
-  struct GtkFixedPrivateL* priv;
-  GtkFixedChild *child;
-  GtkAllocation child_allocation;
-  GtkRequisition child_requisition;
-  GList *children;
-  struct xwidget_view* xv;
-  
-  klass = EMACS_FIXED_GET_CLASS (widget);
-  parent_class = g_type_class_peek_parent (klass);
+  EmacsFixedClass *klass = EMACS_FIXED_GET_CLASS (widget);
+  GtkWidgetClass *parent_class = g_type_class_peek_parent (klass);
+
   parent_class->size_allocate (widget, allocation);
 
-  priv = G_TYPE_INSTANCE_GET_PRIVATE (widget,
-                               GTK_TYPE_FIXED,
-                               struct GtkFixedPrivateL);
-  
+  struct GtkFixedPrivateL *priv
+    = G_TYPE_INSTANCE_GET_PRIVATE (widget,
+				   GTK_TYPE_FIXED,
+				   struct GtkFixedPrivateL);
+
   gtk_widget_set_allocation (widget, allocation);
 
   if (gtk_widget_get_has_window (widget))
@@ -110,16 +87,17 @@ static void emacs_fixed_gtk_widget_size_allocate (GtkWidget *widget,
                                 allocation->height);
     }
 
-  for (children = priv->children;
-       children;
-       children = children->next)
+  for (GList *children = priv->children; children; children = children->next)
     {
-      child = children->data;
+      GtkFixedChild *child = children->data;
 
       if (!gtk_widget_get_visible (child->widget))
         continue;
 
+      GtkRequisition child_requisition;
       gtk_widget_get_preferred_size (child->widget, &child_requisition, NULL);
+
+      GtkAllocation child_allocation;
       child_allocation.x = child->x;
       child_allocation.y = child->y;
 
@@ -132,17 +110,16 @@ static void emacs_fixed_gtk_widget_size_allocate (GtkWidget *widget,
       child_allocation.width = child_requisition.width;
       child_allocation.height = child_requisition.height;
 
+      struct xwidget_view *xv
+	= g_object_get_data (G_OBJECT (child->widget), XG_XWIDGET_VIEW);
+      if (xv)
+	{
+	  child_allocation.width = xv->clip_right;
+	  child_allocation.height = xv->clip_bottom - xv->clip_top;
+	}
 
-
-      xv = (struct xwidget_view*) g_object_get_data (G_OBJECT (child->widget), XG_XWIDGET_VIEW);
-      if(xv){
-        child_allocation.width = xv->clip_right;
-        child_allocation.height = xv->clip_bottom - xv->clip_top;
-      }
       gtk_widget_size_allocate (child->widget, &child_allocation);
-
     }
-
 }
 
 #endif  /* HAVE_XWIDGETS */
@@ -150,26 +127,16 @@ static void emacs_fixed_gtk_widget_size_allocate (GtkWidget *widget,
 static void
 emacs_fixed_class_init (EmacsFixedClass *klass)
 {
-  GtkWidgetClass *widget_class;
-  GtkFixedClass *fixed_class;
-
-  widget_class = (GtkWidgetClass*) klass;
-  fixed_class = (GtkFixedClass*) klass;
+  GtkWidgetClass *widget_class = (GtkWidgetClass *) klass;
 
   widget_class->get_preferred_width = emacs_fixed_get_preferred_width;
   widget_class->get_preferred_height = emacs_fixed_get_preferred_height;
 #ifdef HAVE_XWIDGETS
-  widget_class->size_allocate =  emacs_fixed_gtk_widget_size_allocate;
+  widget_class->size_allocate = emacs_fixed_gtk_widget_size_allocate;
 #endif
   g_type_class_add_private (klass, sizeof (EmacsFixedPrivate));
 }
 
-static GType
-emacs_fixed_child_type (GtkFixed *container)
-{
-  return GTK_TYPE_WIDGET;
-}
-
 static void
 emacs_fixed_init (EmacsFixed *fixed)
 {
@@ -185,7 +152,7 @@ emacs_fixed_init (EmacsFixed *fixed)
  *
  * Returns: a new #EmacsFixed.
  */
-GtkWidget*
+GtkWidget *
 emacs_fixed_new (struct frame *f)
 {
   EmacsFixed *fixed = g_object_new (EMACS_TYPE_FIXED, NULL);
@@ -224,10 +191,7 @@ emacs_fixed_get_preferred_height (GtkWidget *widget,
    (Bug#8919), and so users can resize our frames as they wish.  */
 
 void
-XSetWMSizeHints (Display* d,
-                 Window w,
-                 XSizeHints* hints,
-                 Atom prop)
+XSetWMSizeHints (Display *d, Window w, XSizeHints* hints, Atom prop)
 {
   struct x_display_info *dpyinfo = x_display_info_for_display (d);
   struct frame *f = x_top_window_to_frame (dpyinfo, w);
diff --git a/src/keyboard.c b/src/keyboard.c
index a9ff77d..6ad98c1 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6087,9 +6087,9 @@ make_lispy_event (struct input_event *event)
 #ifdef HAVE_XWIDGETS
     case XWIDGET_EVENT:
       {
-        return  Fcons (Qxwidget_event,event->arg);
+        return Fcons (Qxwidget_event,event->arg);
       }
-#endif /* HAVE_XWIDGETS */
+#endif
 
 
 #if defined HAVE_GFILENOTIFY || defined HAVE_INOTIFY
@@ -11111,10 +11111,11 @@ syms_of_keyboard (void)
 
 #ifdef HAVE_XWIDGETS
   DEFSYM (Qxwidget_event,"xwidget-event");
-#endif /* HAVE_XWIDGETS */
+#endif
+
 #ifdef USE_FILE_NOTIFY
   DEFSYM (Qfile_notify, "file-notify");
-#endif /* USE_FILE_NOTIFY */
+#endif
 
   /* Menu and tool bar item parts.  */
   DEFSYM (QCenable, ":enable");
diff --git a/src/lisp.h b/src/lisp.h
index 87bc3ef..2da142b 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -781,6 +781,7 @@ enum pvec_type
   PVEC_WINDOW_CONFIGURATION,
   PVEC_SUBR,
   PVEC_OTHER,
+
 #ifdef HAVE_XWIDGETS
   PVEC_XWIDGET,
   PVEC_XWIDGET_VIEW,
diff --git a/src/print.c b/src/print.c
index 75288bc..cca73ec 100644
--- a/src/print.c
+++ b/src/print.c
@@ -38,7 +38,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "font.h"
 
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
 
 #include <float.h>
diff --git a/src/termhooks.h b/src/termhooks.h
index 58ae239..d65d5ba 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -240,6 +240,7 @@ enum event_kind
   /* events generated by xwidgets*/
    , XWIDGET_EVENT
 #endif
+
 #ifdef USE_FILE_NOTIFY
   /* File or directory was changed.  */
   , FILE_NOTIFY_EVENT
diff --git a/src/window.c b/src/window.c
index b423010..2b1f7fb 100644
--- a/src/window.c
+++ b/src/window.c
@@ -45,7 +45,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "msdos.h"
 #endif
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
 
 static int displayed_window_lines (struct window *);
@@ -4563,7 +4563,7 @@ Signal an error when WINDOW is the only window on its frame.  */)
       /* Block input.  */
       block_input ();
 #ifdef HAVE_XWIDGETS
-      xwidget_view_delete_all_in_window(w);
+      xwidget_view_delete_all_in_window (w);
 #endif
       window_resize_apply (p, horflag);
       /* If this window is referred to by the dpyinfo's mouse
diff --git a/src/xdisp.c b/src/xdisp.c
index 01d598f..6cbfbf1 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -319,8 +319,9 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #endif /* HAVE_WINDOW_SYSTEM */
 
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
+
 #ifndef FRAME_X_OUTPUT
 #define FRAME_X_OUTPUT(f) ((f)->output_data.x)
 #endif
@@ -846,7 +847,7 @@ static int next_element_from_buffer (struct it *);
 static int next_element_from_composition (struct it *);
 static int next_element_from_image (struct it *);
 #ifdef HAVE_XWIDGETS
-static int next_element_from_xwidget(struct it *);
+static int next_element_from_xwidget (struct it *);
 #endif
 static int next_element_from_stretch (struct it *);
 static void load_overlay_strings (struct it *, ptrdiff_t);
@@ -5136,7 +5137,7 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
              || (CONSP (value) && EQ (XCAR (value), Qspace))
 #ifdef HAVE_XWIDGETS
              || ((it ? FRAME_WINDOW_P (it->f) : frame_window_p)
-		 && valid_xwidget_spec_p(value))
+		 && valid_xwidget_spec_p (value))
 #endif
              );
 
@@ -5221,7 +5222,6 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
           it->position = start_pos;
 	  it->object = NILP (object) ? it->w->contents : object;
 	  *position = start_pos;
-
           it->xwidget = lookup_xwidget(value);
 	}
 #endif
@@ -6725,9 +6725,9 @@ static int (* get_next_element[NUM_IT_METHODS]) (struct it *it) =
   next_element_from_string,
   next_element_from_c_string,
   next_element_from_image,
-  next_element_from_stretch
+  next_element_from_stretch,
 #ifdef HAVE_XWIDGETS
-  ,next_element_from_xwidget
+  next_element_from_xwidget,
 #endif
 };
 
@@ -8041,7 +8041,7 @@ next_element_from_image (struct it *it)
 }
 
 #ifdef HAVE_XWIDGETS
-/* im not sure about this FIXME JAVE*/
+/* I'm not sure about this.  FIXME JAVE */
 static int
 next_element_from_xwidget (struct it *it)
 {
@@ -17056,13 +17056,6 @@ try_window_reusing_current_matrix (struct window *w)
     return 0;
 #endif
 
-#ifdef HAVE_XWIDGETS_xxx
- //currently this is needed to detect xwidget movement reliably. or probably not.
-  printf("try_window_reusing_current_matrix\n");
-    return 0;
-#endif
-
-
   if (/* This function doesn't handle terminal frames.  */
       !FRAME_WINDOW_P (f)
       /* Don't try to reuse the display if windows have been split
@@ -24096,13 +24089,13 @@ calc_pixel_width_or_height (double *res, struct it *it, Lisp_Object prop,
 
 	      return OK_PIXELS (width_p ? img->width : img->height);
 	    }
-#ifdef HAVE_XWIDGETS
+# ifdef HAVE_XWIDGETS
 	  if (FRAME_WINDOW_P (it->f) && valid_xwidget_spec_p (prop))
 	    {
-              //TODO dont return dummy size
-              return OK_PIXELS (width_p ? 100 : 100);
+              // TODO: Don't return dummy size.
+              return OK_PIXELS (100);
             }
-#endif
+# endif
 #endif
 	  if (EQ (car, Qplus) || EQ (car, Qminus))
 	    {
@@ -24957,15 +24950,15 @@ compute_overhangs_and_x (struct glyph_string *s, int x, int backward_p)
 #ifdef HAVE_XWIDGETS
 #define BUILD_XWIDGET_GLYPH_STRING(START, END, HEAD, TAIL, HL, X, LAST_X) \
      do									\
-       { \
-	 s = (struct glyph_string *) alloca (sizeof *s);		\
+       {								\
+	 s = alloca (sizeof *s);					\
 	 INIT_GLYPH_STRING (s, NULL, w, row, area, START, HL);		\
 	 fill_xwidget_glyph_string (s);					\
-	 append_glyph_string (&HEAD, &TAIL, s);				\
-	 ++START;							\
+	 append_glyph_string (&(HEAD), &(TAIL), s);			\
+	 ++(START);							\
          s->x = (X);							\
        }								\
-     while (0)
+     while (false)
 #endif
 
 
@@ -25122,11 +25115,13 @@ compute_overhangs_and_x (struct glyph_string *s, int x, int backward_p)
 					HL, X, LAST_X);			\
 	      break;
 
-#define BUILD_GLYPH_STRINGS_XW(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+#ifdef HAVE_XWIDGETS
+# define BUILD_GLYPH_STRINGS_XW(START, END, HEAD, TAIL, HL, X, LAST_X)	\
             case XWIDGET_GLYPH:                                         \
               BUILD_XWIDGET_GLYPH_STRING (START, END, HEAD, TAIL,       \
                                           HL, X, LAST_X);               \
               break;
+#endif
 
 #define BUILD_GLYPH_STRINGS_2(START, END, HEAD, TAIL, HL, X, LAST_X)	\
 	    case GLYPHLESS_GLYPH:					\
@@ -25148,14 +25143,14 @@ compute_overhangs_and_x (struct glyph_string *s, int x, int backward_p)
 
 
 #ifdef HAVE_XWIDGETS
-#define BUILD_GLYPH_STRINGS(START, END, HEAD, TAIL, HL, X, LAST_X)	\
-BUILD_GLYPH_STRINGS_1(START, END, HEAD, TAIL, HL, X, LAST_X)	\
-BUILD_GLYPH_STRINGS_XW(START, END, HEAD, TAIL, HL, X, LAST_X)	\
-BUILD_GLYPH_STRINGS_2(START, END, HEAD, TAIL, HL, X, LAST_X)
+# define BUILD_GLYPH_STRINGS(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+    BUILD_GLYPH_STRINGS_1(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+    BUILD_GLYPH_STRINGS_XW(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+    BUILD_GLYPH_STRINGS_2(START, END, HEAD, TAIL, HL, X, LAST_X)
 #else
-#define BUILD_GLYPH_STRINGS(START, END, HEAD, TAIL, HL, X, LAST_X)	\
-BUILD_GLYPH_STRINGS_1(START, END, HEAD, TAIL, HL, X, LAST_X)	\
-BUILD_GLYPH_STRINGS_2(START, END, HEAD, TAIL, HL, X, LAST_X)
+# define BUILD_GLYPH_STRINGS(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+    BUILD_GLYPH_STRINGS_1(START, END, HEAD, TAIL, HL, X, LAST_X)	\
+    BUILD_GLYPH_STRINGS_2(START, END, HEAD, TAIL, HL, X, LAST_X)
 #endif
 
 
@@ -25801,12 +25796,11 @@ produce_image_glyph (struct it *it)
 static void
 produce_xwidget_glyph (struct it *it)
 {
-  struct xwidget* xw;
-  struct face *face;
+  struct xwidget *xw;
   int glyph_ascent, crop;
   eassert (it->what == IT_XWIDGET);
 
-  face = FACE_FROM_ID (it->f, it->face_id);
+  struct face *face = FACE_FROM_ID (it->f, it->face_id);
   eassert (face);
   /* Make sure X resources of the face is loaded.  */
   prepare_face_for_display (it->f, face);
@@ -25827,8 +25821,8 @@ produce_xwidget_glyph (struct it *it)
     {
       if (face->box_line_width > 0)
 	{
-	    it->ascent += face->box_line_width;
-	    it->descent += face->box_line_width;
+	  it->ascent += face->box_line_width;
+	  it->descent += face->box_line_width;
 	}
 
       if (it->start_of_box_run_p)
@@ -25840,18 +25834,16 @@ produce_xwidget_glyph (struct it *it)
 
   /* Automatically crop wide image glyphs at right edge so we can
      draw the cursor on same display row.  */
-  if ((crop = it->pixel_width - (it->last_visible_x - it->current_x), crop > 0)
-      && (it->hpos == 0 || it->pixel_width > it->last_visible_x / 4))
-    {
-      it->pixel_width -= crop;
-    }
+  crop = it->pixel_width - (it->last_visible_x - it->current_x);
+  if (crop > 0 && (it->hpos == 0 || it->pixel_width > it->last_visible_x / 4))
+    it->pixel_width -= crop;
 
   if (it->glyph_row)
     {
-      struct glyph *glyph;
       enum glyph_row_area area = it->area;
+      struct glyph *glyph
+	= it->glyph_row->glyphs[area] + it->glyph_row->used[area];
 
-      glyph = it->glyph_row->glyphs[area] + it->glyph_row->used[area];
       if (it->glyph_row->reversed_p)
 	{
 	  struct glyph *g;
@@ -25889,7 +25881,6 @@ produce_xwidget_glyph (struct it *it)
 	  glyph->glyph_not_available_p = 0;
 	  glyph->face_id = it->face_id;
           glyph->u.xwidget = it->xwidget;
-          //assert_valid_xwidget_id(glyph->u.xwidget_id,"produce_xwidget_glyph");
 	  glyph->font_type = FONT_TYPE_UNKNOWN;
 	  if (it->bidi_p)
 	    {
@@ -27617,11 +27608,9 @@ get_window_cursor_type (struct window *w, struct glyph *glyph, int *width,
   /* Use normal cursor if not blinked off.  */
   if (!w->cursor_off_p)
     {
-
 #ifdef HAVE_XWIDGETS
-      if (glyph != NULL && glyph->type == XWIDGET_GLYPH){
+      if (glyph != NULL && glyph->type == XWIDGET_GLYPH)
         return NO_CURSOR;
-      }
 #endif
       if (glyph != NULL && glyph->type == IMAGE_GLYPH)
 	{
diff --git a/src/xterm.c b/src/xterm.c
index abceefb..a4ce8c9 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -64,7 +64,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "frame.h"
 #include "dispextern.h"
 #ifdef HAVE_XWIDGETS
-#include "xwidget.h"
+# include "xwidget.h"
 #endif
 #include "fontset.h"
 #include "termhooks.h"
@@ -2681,11 +2681,10 @@ x_draw_glyph_string (struct glyph_string *s)
 
 #ifdef HAVE_XWIDGETS
     case XWIDGET_GLYPH:
-      //erase xwidget background
-      //x_draw_glyph_string_background (s, 0);
       x_draw_xwidget_glyph_string (s);
       break;
 #endif
+
     case STRETCH_GLYPH:
       x_draw_stretch_glyph_string (s);
       break;
@@ -8027,10 +8026,10 @@ x_draw_bar_cursor (struct window *w, struct glyph_row *row, int width, enum text
     return;
 
 #ifdef HAVE_XWIDGETS
-  if (cursor_glyph->type == XWIDGET_GLYPH){
-    return; //experimental avoidance of cursor on xwidget
-  }
+  if (cursor_glyph->type == XWIDGET_GLYPH)
+    return; // Experimental avoidance of cursor on xwidget.
 #endif
+
   /* If on an image, draw like a normal cursor.  That's usually better
      visible than drawing a bar, esp. if the image is large so that
      the bar might not be in the window.  */
diff --git a/src/xwidget.c b/src/xwidget.c
index 747e803..7485206 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -1,11 +1,30 @@
+/* Embed GTK widgets inside Emacs buffers.
+
+Copyright 2015 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
 #include <config.h>
+
 #ifdef HAVE_XWIDGETS
 
 #include <signal.h>
 
 #include <stdio.h>
 #include <setjmp.h>
-#ifdef HAVE_X_WINDOWS
 
 #include "lisp.h"
 #include "blockinput.h"
@@ -15,17 +34,17 @@
 #include <X11/cursorfont.h>
 
 #ifndef makedev
-#include <sys/types.h>
-#endif /* makedev */
+# include <sys/types.h>
+#endif
 
 #ifdef BSD_SYSTEM
-#include <sys/ioctl.h>
-#endif /* ! defined (BSD_SYSTEM) */
+# include <sys/ioctl.h>
+#endif
 
 #include "systime.h"
 
 #ifndef INCLUDED_FCNTL
-#include <fcntl.h>
+# include <fcntl.h>
 #endif
 #include <ctype.h>
 #include <errno.h>
@@ -52,109 +71,85 @@
 #include "atimer.h"
 #include "keymap.h"
 
-
 #ifdef USE_X_TOOLKIT
-#include <X11/Shell.h>
+# include <X11/Shell.h>
 #endif
 #include <X11/extensions/Xcomposite.h>
 #include <X11/extensions/Xrender.h>
 #include <cairo.h>
-#ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
-#endif
-#ifdef HAVE_UNISTD_H
 #include <unistd.h>
-#endif
 
 #include "gtkutil.h"
 #include "font.h"
-#endif  /* HAVE_X_WINDOWS */
 
 #include <gtk/gtk.h>
 #include <gdk/gdk.h>
 
 #ifdef HAVE_GTK3
-//for gtk3; sockets and plugs
-#include <gtk/gtkx.h>
-#include <gtk/gtkscrolledwindow.h>
-#include "emacsgtkfixed.h"
+// For gtk3; sockets and plugs.
+# include <gtk/gtkx.h>
+# include <gtk/gtkscrolledwindow.h>
+# include "emacsgtkfixed.h"
 #endif
 
 #include <wchar.h>
 
 #ifdef HAVE_WEBKIT_OSR
-#include <webkit/webkitwebview.h>
-#include <webkit/webkitwebplugindatabase.h>
-#include <webkit/webkitwebplugin.h>
-#include <webkit/webkitglobals.h>
-#include <webkit/webkitwebnavigationaction.h>
-#include <webkit/webkitdownload.h>
-#include <webkit/webkitwebpolicydecision.h>
+# include <webkit/webkitwebview.h>
+# include <webkit/webkitwebplugindatabase.h>
+# include <webkit/webkitwebplugin.h>
+# include <webkit/webkitglobals.h>
+# include <webkit/webkitwebnavigationaction.h>
+# include <webkit/webkitdownload.h>
+# include <webkit/webkitwebpolicydecision.h>
 #endif
 
-//for GIR
+// For GIR.
 #include <girepository.h>
 
 #include "xwidget.h"
 
-//TODO embryo of lisp allocators for xwidgets
-//TODO xwidget* should be Lisp_xwidget*
-struct xwidget*
+// TODO embryo of lisp allocators for xwidgets
+// TODO xwidget * should be Lisp_xwidget *
+static struct xwidget *
 allocate_xwidget (void)
 {
   return ALLOCATE_PSEUDOVECTOR (struct xwidget, height, PVEC_XWIDGET);
 }
 
-//TODO xwidget_view* should be Lisp_xwidget_view*
-struct xwidget_view*
+// TODO xwidget_view * should be Lisp_xwidget_view *
+static struct xwidget_view *
 allocate_xwidget_view (void)
 {
-  return ALLOCATE_PSEUDOVECTOR (struct xwidget_view, redisplayed, PVEC_XWIDGET_VIEW);
+  return ALLOCATE_PSEUDOVECTOR (struct xwidget_view, redisplayed,
+				PVEC_XWIDGET_VIEW);
 }
+
 #define XSETXWIDGET(a, b) (XSETPSEUDOVECTOR (a, b, PVEC_XWIDGET))
 #define XSETXWIDGET_VIEW(a, b) (XSETPSEUDOVECTOR (a, b, PVEC_XWIDGET_VIEW))
 
-struct xwidget_view* xwidget_view_lookup(struct xwidget* xw,     struct window *w);
-Lisp_Object xwidget_spec_value ( Lisp_Object spec, Lisp_Object  key,  int *found);
-gboolean offscreen_damage_event (GtkWidget *widget, GdkEvent *event, gpointer data);
-void     webkit_osr_document_load_finished_callback (WebKitWebView  *webkitwebview,
-                                                     WebKitWebFrame *arg1,
-                                                     gpointer        user_data);
-gboolean     webkit_osr_download_callback (WebKitWebView  *webkitwebview,
-                                       WebKitDownload *arg1,
-                                       gpointer        data);
-
-gboolean  webkit_osr_mime_type_policy_typedecision_requested_callback(WebKitWebView           *webView,
-                                                                      WebKitWebFrame          *frame,
-                                                                      WebKitNetworkRequest    *request,
-                                                                      gchar                   *mimetype,
-                                                                      WebKitWebPolicyDecision *policy_decision,
-                                                                      gpointer                 user_data);
+static struct xwidget_view *xwidget_view_lookup (struct xwidget *,
+						 struct window *);
+static void webkit_osr_document_load_finished_callback (WebKitWebView *,
+							WebKitWebFrame *,
+							gpointer);
+static gboolean webkit_osr_download_callback (WebKitWebView *,
+					      WebKitDownload *, gpointer);
+static gboolean webkit_osr_mime_type_policy_typedecision_requested_callback
+  (WebKitWebView *, WebKitWebFrame *, WebKitNetworkRequest *, gchar *,
+   WebKitWebPolicyDecision *, gpointer);
+static gboolean webkit_osr_new_window_policy_decision_requested_callback
+  (WebKitWebView *, WebKitWebFrame *, WebKitNetworkRequest *,
+   WebKitWebNavigationAction *, WebKitWebPolicyDecision *, gpointer);
+static gboolean webkit_osr_navigation_policy_decision_requested_callback
+  (WebKitWebView *, WebKitWebFrame *, WebKitNetworkRequest *,
+   WebKitWebNavigationAction *, WebKitWebPolicyDecision *, gpointer);
+static GtkWidget *xwgir_create (char *, char *);
+static void send_xembed_ready_event (struct xwidget *, int);
 
-gboolean webkit_osr_new_window_policy_decision_requested_callback(WebKitWebView             *webView,
-                                                                  WebKitWebFrame            *frame,
-                                                                  WebKitNetworkRequest      *request,
-                                                                  WebKitWebNavigationAction *navigation_action,
-                                                                  WebKitWebPolicyDecision   *policy_decision,
-                                                                  gpointer                   user_data);
-
-
-gboolean webkit_osr_navigation_policy_decision_requested_callback(WebKitWebView             *webView,
-                                                        WebKitWebFrame            *frame,
-                                                        WebKitNetworkRequest      *request,
-                                                        WebKitWebNavigationAction *navigation_action,
-                                                        WebKitWebPolicyDecision   *policy_decision,
-                                                                  gpointer                   user_data);
-
-GtkWidget* xwgir_create(char* class, char* namespace);
-
-
-
-static void
-send_xembed_ready_event (struct xwidget* xw, int xembedid);
 DEFUN ("make-xwidget", Fmake_xwidget, Smake_xwidget, 7, 8, 0,
-         doc: /* Make an xwidget from BEG to END of TYPE.
-
+       doc: /* Make an xwidget from BEG to END of TYPE.
 If BUFFER is nil it uses the current buffer. If BUFFER is a string and
 no such buffer exists, it is created.
 
@@ -165,529 +160,518 @@ TYPE is a symbol which can take one of the following values:
 - socket
 - socket-osr
 - cairo
-*/
-         )
-  (Lisp_Object beg, Lisp_Object end,
-   Lisp_Object type,
-   Lisp_Object title,
-   Lisp_Object width, Lisp_Object height,
-   Lisp_Object data,
-   Lisp_Object buffer)
-{
-  //should work a bit like "make-button"(make-button BEG END &rest PROPERTIES)
+*/)
+  (Lisp_Object beg, Lisp_Object end, Lisp_Object type, Lisp_Object title,
+   Lisp_Object width, Lisp_Object height, Lisp_Object data, Lisp_Object buffer)
+{
+  // Should work a bit like "make-button"(make-button BEG END &rest PROPERTIES)
   // arg "type" and fwd should be keyword args eventually
-  //(make-xwidget 3 3 'button "oei" 31 31 nil)
-  //(xwidget-info (car xwidget-list))
-  struct xwidget* xw = allocate_xwidget();
+  // (make-xwidget 3 3 'button "oei" 31 31 nil)
+  // (xwidget-info (car xwidget-list))
+  struct xwidget *xw = allocate_xwidget ();
   Lisp_Object val;
   xw->type = type;
   xw->title = title;
+
+  // No need to gcpro because Fcurrent_buffer doesn't call Feval/eval_sub.
   if (NILP (buffer))
-      buffer = Fcurrent_buffer(); // no need to gcpro because Fcurrent_buffer doesn't call Feval/eval_sub.
+    buffer = Fcurrent_buffer ();
   else
-      buffer = Fget_buffer_create (buffer);
+    buffer = Fget_buffer_create (buffer);
   xw->buffer = buffer;
 
-  xw->height = XFASTINT(height);
-  xw->width = XFASTINT(width);
+  xw->height = XFASTINT (height);
+  xw->width = XFASTINT (width);
   xw->kill_without_query = 0;
-  XSETXWIDGET (val, xw); // set the vectorlike_header of VAL with the correct value
+
+  // Set the vectorlike_header of VAL with the correct value.
+  XSETXWIDGET (val, xw);
+
   Vxwidget_list = Fcons (val, Vxwidget_list);
   xw->widgetwindow_osr = NULL;
   xw->widget_osr = NULL;
   xw->plist = Qnil;
 
-
-
-
 #ifdef HAVE_WEBKIT_OSR
-  /* DIY mvc. widget is rendered offscreen,
-     later bitmap copied to the views.
-   */
-  if (EQ(xw->type, Qwebkit_osr)||
-      EQ(xw->type, Qsocket_osr)||
-      (!NILP (Fget(xw->type, QCxwgir_class)))) {
-      block_input();
+  // DIY mvc. widget is rendered offscreen, later bitmap copied to the views.
+  if (EQ (xw->type, Qwebkit_osr) || EQ (xw->type, Qsocket_osr)
+      || !NILP (Fget (xw->type, QCxwgir_class)))
+    {
+      block_input ();
       xw->widgetwindow_osr = gtk_offscreen_window_new ();
-      gtk_window_resize(GTK_WINDOW(xw->widgetwindow_osr), xw->width, xw->height);
-      xw->widgetscrolledwindow_osr = NULL; //webkit osr is the only scrolled component atm
-
-      if (EQ(xw->type, Qwebkit_osr)){
-        xw->widgetscrolledwindow_osr =   gtk_scrolled_window_new(NULL, NULL);
-        gtk_scrolled_window_set_min_content_height(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr ),xw->height);
-        gtk_scrolled_window_set_min_content_width(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr ),xw->width);
-        gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr ), GTK_POLICY_ALWAYS, GTK_POLICY_ALWAYS);
-
-        xw->widget_osr=webkit_web_view_new();
-        gtk_container_add(GTK_CONTAINER(xw->widgetscrolledwindow_osr ), GTK_WIDGET( WEBKIT_WEB_VIEW(xw->widget_osr)));
-      }
-      if(EQ(xw->type, Qsocket_osr))
-          xw->widget_osr = gtk_socket_new();
-      if(!NILP (Fget(xw->type, QCxwgir_class)))
-          xw->widget_osr = xwgir_create(SDATA(Fcar(Fcdr(Fget(xw->type, QCxwgir_class)))),
-                                        SDATA(Fcar(Fget(xw->type, QCxwgir_class))));
-
-      gtk_widget_set_size_request (GTK_WIDGET (xw->widget_osr), xw->width, xw->height);
-
-      if (EQ(xw->type, Qwebkit_osr)){
-        gtk_container_add (GTK_CONTAINER (xw->widgetwindow_osr), xw->widgetscrolledwindow_osr);
-      }else{
-        gtk_container_add (GTK_CONTAINER (xw->widgetwindow_osr), xw->widget_osr);
-      }
+      gtk_window_resize (GTK_WINDOW (xw->widgetwindow_osr),
+			 xw->width, xw->height);
+
+      // Webkit osr is the only scrolled component atm.
+      xw->widgetscrolledwindow_osr = NULL;
+
+      if (EQ (xw->type, Qwebkit_osr))
+	{
+	  xw->widgetscrolledwindow_osr = gtk_scrolled_window_new (NULL, NULL);
+	  gtk_scrolled_window_set_min_content_height
+	    (GTK_SCROLLED_WINDOW (xw->widgetscrolledwindow_osr), xw->height);
+	  gtk_scrolled_window_set_min_content_width
+	    (GTK_SCROLLED_WINDOW (xw->widgetscrolledwindow_osr), xw->width);
+	  gtk_scrolled_window_set_policy
+	    (GTK_SCROLLED_WINDOW (xw->widgetscrolledwindow_osr),
+	     GTK_POLICY_ALWAYS, GTK_POLICY_ALWAYS);
+
+	  xw->widget_osr = webkit_web_view_new ();
+	  gtk_container_add (GTK_CONTAINER (xw->widgetscrolledwindow_osr),
+			     GTK_WIDGET (WEBKIT_WEB_VIEW (xw->widget_osr)));
+	}
+
+      if (EQ (xw->type, Qsocket_osr))
+	xw->widget_osr = gtk_socket_new ();
+      if (!NILP (Fget (xw->type, QCxwgir_class)))
+	xw->widget_osr
+	  = xwgir_create (SSDATA (Fcar (Fcdr (Fget (xw->type, QCxwgir_class)))),
+			  SSDATA (Fcar (Fget (xw->type, QCxwgir_class))));
+
+      gtk_widget_set_size_request (GTK_WIDGET (xw->widget_osr),
+				   xw->width, xw->height);
+
+      gtk_container_add (GTK_CONTAINER (xw->widgetwindow_osr),
+			 (EQ (xw->type, Qwebkit_osr)
+			  ? xw->widgetscrolledwindow_osr : xw->widget_osr));
 
       gtk_widget_show (xw->widget_osr);
       gtk_widget_show (xw->widgetwindow_osr);
       gtk_widget_show (xw->widgetscrolledwindow_osr);
 
-      /* store some xwidget data in the gtk widgets for convenient retrieval in the event handlers. */
-      g_object_set_data (G_OBJECT (xw->widget_osr), XG_XWIDGET, (gpointer) (xw));
-      g_object_set_data (G_OBJECT (xw->widgetwindow_osr), XG_XWIDGET, (gpointer) (xw));
-
-      /* signals */
-      if (EQ(xw->type, Qwebkit_osr)) {
-          g_signal_connect (G_OBJECT (xw->widget_osr),
-                            "document-load-finished",
-                            G_CALLBACK (webkit_osr_document_load_finished_callback),
-                            xw);
-
-          g_signal_connect (G_OBJECT (xw->widget_osr),
-                            "download-requested",
-                            G_CALLBACK (webkit_osr_download_callback),
-                            xw);
-
-          g_signal_connect (G_OBJECT (xw->widget_osr),
-                            "mime-type-policy-decision-requested",
-                            G_CALLBACK (webkit_osr_mime_type_policy_typedecision_requested_callback),
-                            xw);
-
-          g_signal_connect (G_OBJECT (xw->widget_osr),
-                            "new-window-policy-decision-requested",
-                            G_CALLBACK (webkit_osr_new_window_policy_decision_requested_callback),
-                            xw);
-
-          g_signal_connect (G_OBJECT (xw->widget_osr),
-                            "navigation-policy-decision-requested",
-                            G_CALLBACK (webkit_osr_navigation_policy_decision_requested_callback),
-                            xw);
-      }
-
-      if (EQ(xw->type, Qsocket_osr)) {
-          send_xembed_ready_event (xw, gtk_socket_get_id (GTK_SOCKET (xw->widget_osr)));
-          //gtk_widget_realize(xw->widget);
-      }
-
-
-      unblock_input();
-
-  }
-#endif  /* HAVE_WEBKIT_OSR */
+      /* Store some xwidget data in the gtk widgets for convenient
+         retrieval in the event handlers.  */
+      g_object_set_data (G_OBJECT (xw->widget_osr), XG_XWIDGET, xw);
+      g_object_set_data (G_OBJECT (xw->widgetwindow_osr), XG_XWIDGET, xw);
+
+      /* Signals.  */
+      if (EQ (xw->type, Qwebkit_osr))
+	{
+	  g_signal_connect
+	    (G_OBJECT (xw->widget_osr),
+	     "document-load-finished",
+	     G_CALLBACK (webkit_osr_document_load_finished_callback), xw);
+
+	  g_signal_connect (G_OBJECT (xw->widget_osr),
+			    "download-requested",
+			    G_CALLBACK (webkit_osr_download_callback), xw);
+
+	  g_signal_connect
+	    (G_OBJECT (xw->widget_osr),
+	     "mime-type-policy-decision-requested",
+	     (G_CALLBACK
+	      (webkit_osr_mime_type_policy_typedecision_requested_callback)),
+	     xw);
+
+	  g_signal_connect
+	    (G_OBJECT (xw->widget_osr),
+	     "new-window-policy-decision-requested",
+	     (G_CALLBACK
+	      (webkit_osr_new_window_policy_decision_requested_callback)),
+	     xw);
+
+	  g_signal_connect
+	    (G_OBJECT (xw->widget_osr),
+	     "navigation-policy-decision-requested",
+	     (G_CALLBACK
+	      (webkit_osr_navigation_policy_decision_requested_callback)),
+	     xw);
+	}
+
+      if (EQ (xw->type, Qsocket_osr))
+	send_xembed_ready_event (xw, (gtk_socket_get_id
+				      (GTK_SOCKET (xw->widget_osr))));
+
+      unblock_input ();
+    }
+#endif /* HAVE_WEBKIT_OSR */
 
   return val;
 }
 
-DEFUN ("get-buffer-xwidgets", Fget_buffer_xwidgets, Sget_buffer_xwidgets, 1, 1, 0,
+DEFUN ("get-buffer-xwidgets", Fget_buffer_xwidgets, Sget_buffer_xwidgets,
+       1, 1, 0,
        doc: /* Return a list of xwidgets associated with BUFFER.
-BUFFER may be a buffer or the name of one.
-       */
-       )
-     (Lisp_Object buffer)
+BUFFER may be a buffer or the name of one.  */)
+  (Lisp_Object buffer)
 {
-    Lisp_Object xw, tail, xw_list;
-
-    if (NILP (buffer)) return Qnil;
-    buffer = Fget_buffer (buffer);
-    if (NILP (buffer)) return Qnil;
+  if (NILP (buffer))
+    return Qnil;
+  buffer = Fget_buffer (buffer);
+  if (NILP (buffer))
+    return Qnil;
 
-    xw_list = Qnil;
+  Lisp_Object xw_list = Qnil;
 
-    for (tail = Vxwidget_list; CONSP (tail); tail = XCDR (tail))
-        {
-            xw = XCAR (tail);
-            if (XWIDGETP (xw) && EQ (Fxwidget_buffer (xw), buffer))
-                xw_list = Fcons (xw, xw_list);
-        }
-    return xw_list;
+  for (Lisp_Object tail = Vxwidget_list; CONSP (tail); tail = XCDR (tail))
+    {
+      Lisp_Object xw = XCAR (tail);
+      if (XWIDGETP (xw) && EQ (Fxwidget_buffer (xw), buffer))
+	xw_list = Fcons (xw, xw_list);
+    }
+  return xw_list;
 }
 
-int
-xwidget_hidden(struct xwidget_view *xv)
+static int
+xwidget_hidden (struct xwidget_view *xv)
 {
-  return  xv->hidden;
+  return xv->hidden;
 }
 
-
 static void
-buttonclick_handler (GtkWidget * widget, gpointer data)
+buttonclick_handler (GtkWidget *widget, gpointer data)
 {
-  Lisp_Object xwidget_view, xwidget;
-  XSETXWIDGET_VIEW (xwidget_view, (struct xwidget_view *) data);
-  xwidget = Fxwidget_view_model (xwidget_view);
+  Lisp_Object xwidget_view;
+  XSETXWIDGET_VIEW (xwidget_view, data);
+  Lisp_Object xwidget = Fxwidget_view_model (xwidget_view);
 
-  struct input_event event;
   Lisp_Object frame = Fwindow_frame (Fxwidget_view_window (xwidget_view));
-  struct frame *f = XFRAME (frame);
-  printf ("button clicked xw:%d '%s'\n", XXWIDGET (xwidget), XXWIDGET (xwidget)->title);
+  printf ("button clicked xw:%p '%s'\n", XXWIDGET (xwidget),
+	  SSDATA (XXWIDGET (xwidget)->title));
 
+  struct input_event event;
   EVENT_INIT (event);
   event.kind = XWIDGET_EVENT;
-
   event.frame_or_window = frame;
-
-  event.arg = Qnil;
-  event.arg = Fcons (xwidget, event.arg);
-  event.arg = Fcons (intern ("buttonclick"), event.arg);
+  event.arg = list2 (intern ("buttonclick"), xwidget);
 
   kbd_buffer_store_event (&event);
 }
 
-
 static void
-send_xembed_ready_event (struct xwidget* xw, int xembedid)
+send_xembed_ready_event (struct xwidget *xw, int xembedid)
 {
   Lisp_Object xw_lo;
-  XSETXWIDGET(xw_lo, xw);
+  XSETXWIDGET (xw_lo, xw);
   struct input_event event;
   EVENT_INIT (event);
   event.kind = XWIDGET_EVENT;
-  event.frame_or_window = Qnil;	//frame; //how to get the frame here? //TODO i store it in the xwidget now
-
-  event.arg = Qnil;
-  event.arg = Fcons (make_number (xembedid), event.arg);
-  event.arg = Fcons (xw_lo, event.arg);
-  event.arg = Fcons (intern ("xembed-ready"), event.arg);
-
+  // frame;
+  // how to get the frame here?
+  // TODO i store it in the xwidget now
+  event.frame_or_window = Qnil;
+  event.arg = list3 (intern ("xembed-ready"), xw_lo, make_number (xembedid));
 
   kbd_buffer_store_event (&event);
-
 }
 
-void
+static void
 xwidget_show_view (struct xwidget_view *xv)
 {
   xv->hidden = 0;
-  gtk_widget_show(xv->widgetwindow);
-  gtk_fixed_move (GTK_FIXED (xv->emacswindow), xv->widgetwindow,  xv->x  + xv->clip_left, xv->y + xv->clip_top); //TODO refactor
+  gtk_widget_show (xv->widgetwindow);
+  gtk_fixed_move (GTK_FIXED (xv->emacswindow), xv->widgetwindow,
+		  xv->x + xv->clip_left, xv->y + xv->clip_top);
+  // TODO refactor
 }
 
-
-/* hide an xvidget view */
-void
+/* Hide an xvidget view.  */
+static void
 xwidget_hide_view (struct xwidget_view *xv)
 {
   xv->hidden = 1;
-  //gtk_widget_hide(xw->widgetwindow);
-  gtk_fixed_move (GTK_FIXED (xv->emacswindow), xv->widgetwindow,
-                  10000, 10000);
+  // gtk_widget_hide (xw->widgetwindow);
+  gtk_fixed_move (GTK_FIXED (xv->emacswindow), xv->widgetwindow, 10000, 10000);
 }
 
-
-void
-xwidget_plug_added(GtkSocket *socket,
-                   gpointer   user_data)
+static void
+xwidget_plug_added (GtkSocket *socket, gpointer user_data)
 {
-  //hmm this doesnt seem to get called for foreign windows
-  printf("xwidget_plug_added\n");
+  // Hmm, this doesn't seem to get called for foreign windows.
+  printf ("xwidget_plug_added\n");
 }
 
-gboolean
-xwidget_plug_removed(GtkSocket *socket,
-                     gpointer   user_data)
+static gboolean
+xwidget_plug_removed (GtkSocket *socket, gpointer user_data)
 {
-  printf("xwidget_plug_removed\n");
-  return TRUE; /* dont run the default handler because that kills the socket and we want to reuse it*/
+  printf ("xwidget_plug_removed\n");
+  return TRUE;
+  /* Don't run the default handler because that kills the socket and
+     we want to reuse it.  */
 }
 
-
-void
-xwidget_slider_changed (GtkRange *range,
-                        gpointer  user_data)
+static void
+xwidget_slider_changed (GtkRange *range, gpointer user_data)
 {
-  //slider value changed. change value of siblings
-  //correspondingly. but remember that changing value will again
-  //trigger signal
+  /* Slider value changed. change value of siblings
+     correspondingly. but remember that changing value will again
+     trigger signal.
+
+     TODO MVC view storage wont be an array futureish so the loop
+     needs to change eventually
 
-  //TODO MVC view storage wont be an array futureish so the loop needs to change eventually
-  //TODO MVC it would be nice if this code could be reusable but, alas, C is not a functional language
-  //issues are:
-  // - the type of the controllers value (double, boolean etc)
-  // - the getter and setter (but they can be func pointers)
-  // a behemoth macro is always an option.
-  double v=gtk_range_get_value(range);
-  struct xwidget_view* xvp = g_object_get_data (G_OBJECT (range), XG_XWIDGET_VIEW);
-  struct xwidget_view* xv;
+     TODO MVC it would be nice if this code could be reusable but,
+     alas, C is not a functional language
 
-  printf("slider changed val:%f\n", v);
+     issues are:
+     - the type of the controllers value (double, boolean etc)
+     - the getter and setter (but they can be func pointers)
+     A behemoth macro is always an option.  */
+  double v = gtk_range_get_value (range);
+  struct xwidget_view *xvp = g_object_get_data (G_OBJECT (range),
+						XG_XWIDGET_VIEW);
+  printf ("slider changed val:%f\n", v);
 
   for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))
-    {
-      if (XWIDGET_VIEW_P (XCAR (tail))) {
-        xv = XXWIDGET_VIEW (XCAR (tail));
-        if (EQ (xvp->model, xv->model)) {
-          //block sibling views signal handlers
-          g_signal_handler_block(xv->widget, xv->handler_id);
-
-          //set values of sibling views and unblock
-          gtk_range_set_value(GTK_RANGE(xv->widget), v);
-          g_signal_handler_unblock(xv->widget,xv->handler_id);
-        }
+    if (XWIDGET_VIEW_P (XCAR (tail)))
+      {
+	struct xwidget_view *xv = XXWIDGET_VIEW (XCAR (tail));
+	if (EQ (xvp->model, xv->model))
+	  {
+	    // Block sibling views signal handlers.
+	    g_signal_handler_block (xv->widget, xv->handler_id);
+
+	    // Set values of sibling views and unblock.
+	    gtk_range_set_value (GTK_RANGE (xv->widget), v);
+	    g_signal_handler_unblock (xv->widget, xv->handler_id);
+	  }
       }
-    }
 }
 
 
 /* when the off-screen webkit master view changes this signal is called.
    it copies the bitmap from the off-screen webkit instance */
-gboolean
+static gboolean
 offscreen_damage_event (GtkWidget *widget, GdkEvent *event, gpointer data)
 {
-  //TODO this is wrong! should just queu a redraw of onscreen widget
+  // TODO this is wrong! should just queue a redraw of onscreen widget.
   gtk_widget_queue_draw (GTK_WIDGET (data));
   return FALSE;
 }
 
-void
-store_xwidget_event_string(struct xwidget* xw, char* eventname, const char* eventstr)
+static void
+store_xwidget_event_string (struct xwidget *xw, char const *eventname,
+			    const char *eventstr)
 {
-  //refactor attempt
+  // Refactor attempt.
   struct input_event event;
   Lisp_Object xwl;
-  XSETXWIDGET(xwl,xw);
+  XSETXWIDGET (xwl, xw);
   EVENT_INIT (event);
   event.kind = XWIDGET_EVENT;
-  event.frame_or_window = Qnil;	//frame; //how to get the frame here? //TODO i store it in the xwidget now
+  //frame; //how to get the frame here? //TODO i store it in the xwidget now
+  event.frame_or_window = Qnil;
 
-  event.arg = Qnil;
-  event.arg = Fcons (build_string(eventstr), event.arg);  //string so dont intern
-  event.arg = Fcons (xwl, event.arg); //TODO
-  event.arg = Fcons (intern (eventname), event.arg);//interning should be ok
+  event.arg = list3 (intern (eventname), xwl, build_string (eventstr));
   kbd_buffer_store_event (&event);
 
 }
 
-//TODO deprecated, use load-status
-void
-webkit_osr_document_load_finished_callback (WebKitWebView  *webkitwebview,
-                                                     WebKitWebFrame *arg1,
-                                                     gpointer        data)
+// TODO deprecated, use load-status.
+static void
+webkit_osr_document_load_finished_callback (WebKitWebView *webkitwebview,
+					    WebKitWebFrame *arg1,
+					    gpointer data)
 {
   //TODO this event sending code should be refactored
-  //  struct xwidget *xw = (struct xwidget *) data;
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (webkitwebview), XG_XWIDGET);
-  printf("webkit finished loading\n");
-
-  store_xwidget_event_string(xw,
-                             "document-load-finished", "");
+  //  struct xwidget *xw = data;
+  struct xwidget *xw = g_object_get_data (G_OBJECT (webkitwebview),
+					  XG_XWIDGET);
+  printf ("webkit finished loading\n");
+  store_xwidget_event_string (xw, "document-load-finished", "");
 }
 
-gboolean
-webkit_osr_download_callback (WebKitWebView  *webkitwebview,
-                                       WebKitDownload *arg1,
-                                       gpointer        data)
+static gboolean
+webkit_osr_download_callback (WebKitWebView *webkitwebview,
+			      WebKitDownload *arg1, gpointer data)
 {
   //TODO this event sending code should be refactored
-  struct input_event event;
-  //  struct xwidget *xw = (struct xwidget *) data;
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (webkitwebview), XG_XWIDGET);
-  printf("download requested %s\n", webkit_download_get_uri (arg1));
+  //  struct xwidget *xw = data;
+  struct xwidget *xw = g_object_get_data (G_OBJECT (webkitwebview),
+					  XG_XWIDGET);
+  printf ("download requested %s\n", webkit_download_get_uri (arg1));
 
 
-  printf("webkit finished loading\n");
+  printf ("webkit finished loading\n");
 
-  store_xwidget_event_string(xw, "download-requested", webkit_download_get_uri (arg1));
+  store_xwidget_event_string (xw, "download-requested",
+			      webkit_download_get_uri (arg1));
 
   return FALSE;
 }
 
-gboolean
-webkit_osr_mime_type_policy_typedecision_requested_callback(WebKitWebView           *webView,
-                                                            WebKitWebFrame          *frame,
-                                                            WebKitNetworkRequest    *request,
-                                                            gchar                   *mimetype,
-                                                            WebKitWebPolicyDecision *policy_decision,
-                                                            gpointer                 user_data)
-{
-  printf("mime policy requested\n");
-  // this function makes webkit send a download signal for all unknown mime types
-  // TODO defer the decision to lisp, so that its possible to make Emacs handle text mime for instance
-  if(!webkit_web_view_can_show_mime_type(webView, mimetype)){
-    webkit_web_policy_decision_download (policy_decision);
-    return TRUE;
-  }else{
+static gboolean
+webkit_osr_mime_type_policy_typedecision_requested_callback
+  (WebKitWebView *webView, WebKitWebFrame *frame,
+   WebKitNetworkRequest *request, gchar *mimetype,
+   WebKitWebPolicyDecision *policy_decision, gpointer user_data)
+{
+  printf ("mime policy requested\n");
+
+  /* This function makes webkit send a download signal for all unknown
+     mime types.
+
+     TODO defer the decision to lisp, so that its possible to make
+     Emacs handle text mime for instance */
+  if (!webkit_web_view_can_show_mime_type (webView, mimetype))
+    {
+      webkit_web_policy_decision_download (policy_decision);
+      return TRUE;
+    }
+  else
     return FALSE;
-  }
 }
 
 
-gboolean
-webkit_osr_new_window_policy_decision_requested_callback(WebKitWebView             *webView,
-                                                         WebKitWebFrame            *frame,
-                                                         WebKitNetworkRequest      *request,
-                                                         WebKitWebNavigationAction *navigation_action,
-                                                         WebKitWebPolicyDecision   *policy_decision,
-                                                         gpointer                   user_data)
+static gboolean
+webkit_osr_new_window_policy_decision_requested_callback
+  (WebKitWebView *webView, WebKitWebFrame *frame,
+   WebKitNetworkRequest *request, WebKitWebNavigationAction *navigation_action,
+   WebKitWebPolicyDecision *policy_decision, gpointer user_data)
 {
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (webView), XG_XWIDGET);
-  printf("webkit_osr_new_window_policy_decision_requested_callback %s\n",
-         webkit_web_navigation_action_get_original_uri (navigation_action));
+  struct xwidget *xw = g_object_get_data (G_OBJECT (webView), XG_XWIDGET);
+  printf ("webkit_osr_new_window_policy_decision_requested_callback %s\n",
+	  webkit_web_navigation_action_get_original_uri (navigation_action));
 
-  store_xwidget_event_string(xw,  "new-window-policy-decision-requested", webkit_web_navigation_action_get_original_uri (navigation_action)
-                            );
+  store_xwidget_event_string (xw, "new-window-policy-decision-requested",
+			      webkit_web_navigation_action_get_original_uri
+			      (navigation_action));
   return FALSE;
 }
 
-gboolean
-webkit_osr_navigation_policy_decision_requested_callback(WebKitWebView             *webView,
-                                                         WebKitWebFrame            *frame,
-                                                         WebKitNetworkRequest      *request,
-                                                         WebKitWebNavigationAction *navigation_action,
-                                                         WebKitWebPolicyDecision   *policy_decision,
-                                                         gpointer                   user_data)
+static gboolean
+webkit_osr_navigation_policy_decision_requested_callback
+  (WebKitWebView *webView, WebKitWebFrame *frame,
+   WebKitNetworkRequest *request, WebKitWebNavigationAction *navigation_action,
+   WebKitWebPolicyDecision *policy_decision, gpointer user_data)
 {
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (webView), XG_XWIDGET);
-  printf("webkit_osr_navigation_policy_decision_requested_callback %s\n",
-         webkit_web_navigation_action_get_original_uri (navigation_action));
-  store_xwidget_event_string(xw,  "navigation-policy-decision-requested", webkit_web_navigation_action_get_original_uri (navigation_action)
-                            );
+  struct xwidget *xw = g_object_get_data (G_OBJECT (webView), XG_XWIDGET);
+  printf ("webkit_osr_navigation_policy_decision_requested_callback %s\n",
+	  webkit_web_navigation_action_get_original_uri (navigation_action));
+  store_xwidget_event_string (xw, "navigation-policy-decision-requested",
+			      webkit_web_navigation_action_get_original_uri
+			      (navigation_action));
   return FALSE;
 }
 
-//for gtk3 offscreen rendered widgets
-gboolean
+// For gtk3 offscreen rendered widgets.
+static gboolean
 xwidget_osr_draw_callback (GtkWidget *widget, cairo_t *cr, gpointer data)
 {
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (widget), XG_XWIDGET);
-  struct xwidget_view* xv = (struct xwidget_view*) g_object_get_data (G_OBJECT (widget), XG_XWIDGET_VIEW);
+  struct xwidget *xw = g_object_get_data (G_OBJECT (widget), XG_XWIDGET);
+  struct xwidget_view *xv = g_object_get_data (G_OBJECT (widget),
+					       XG_XWIDGET_VIEW);
 
-  cairo_rectangle(cr, 0,0, xv->clip_right, xv->clip_bottom);//xw->width, xw->height);
-  cairo_clip(cr);
+  //xw->width, xw->height);
+  cairo_rectangle (cr, 0, 0, xv->clip_right, xv->clip_bottom);
+  cairo_clip (cr);
 
-  //
-  if(xw->widgetscrolledwindow_osr !=  NULL)
+  if (xw->widgetscrolledwindow_osr != NULL)
     gtk_widget_draw (xw->widgetscrolledwindow_osr, cr);
   else
     gtk_widget_draw (xw->widget_osr, cr);
   return FALSE;
 }
 
-GtkWidget* xwgir_create_debug;
-
-
+GtkWidget *xwgir_create_debug;
 
-gboolean
+static gboolean
 xwidget_osr_event_forward (GtkWidget *widget,
-                           GdkEvent  *event,
-                           gpointer   user_data)
+			   GdkEvent *event, gpointer user_data)
 {
-  /* copy events that arrive at the outer widget to the offscreen widget */
-  struct xwidget* xw = (struct xwidget*) g_object_get_data (G_OBJECT (widget), XG_XWIDGET);
-  GdkEvent* eventcopy =  gdk_event_copy(event);
-  eventcopy->any.window = gtk_widget_get_window(xw->widget_osr);// works
+  /* Copy events that arrive at the outer widget to the offscreen widget.  */
+  struct xwidget *xw = g_object_get_data (G_OBJECT (widget), XG_XWIDGET);
+  GdkEvent *eventcopy = gdk_event_copy (event);
+  eventcopy->any.window = gtk_widget_get_window (xw->widget_osr); // works
 
-  /* printf("xwidget_osr_event_forward redirect event to window:%d\n",   ((GdkEventAny*)eventcopy)->window); */
-  /* printf("A type:%d x:%f y:%f \n",   event->type, event->button.x, event->button.y); */
-  /* printf("B type:%d x:%f y:%f \n",   eventcopy->type, eventcopy->button.x, eventcopy->button.y); */
-    //gtk_button_get_event_window(xwgir_create_debug);
-  gtk_main_do_event(eventcopy); //TODO this will leak events. they should be deallocated later, perhaps in xwgir_event_callback
-  return TRUE; //dont propagate this event furter
+  /* TODO this will leak events. they should be deallocated later,
+     perhaps in xwgir_event_callback.  */
+  gtk_main_do_event (eventcopy);
+  return TRUE;			// Don't propagate this event further.
 }
 
-GIRepository *girepository ;
+GIRepository *girepository;
 
-DEFUN ("xwgir-require-namespace", Fxwgir_require_namespace, Sxwgir_require_namespace, 2,2,0,
+DEFUN ("xwgir-require-namespace", Fxwgir_require_namespace,
+       Sxwgir_require_namespace, 2, 2, 0,
        doc: /* Require a GObject Introspection namespace.
-               This must be done for all namespaces we want to use, before using other xwgir functions.*/)
+This must be done for all namespaces we want to use, before using
+other xwgir functions.  */)
   (Lisp_Object lnamespace, Lisp_Object lnamespace_version)
 {
-  char* namespace = SDATA(lnamespace);
-  char* namespace_version = SDATA(lnamespace_version);
+  char *namespace = SSDATA (lnamespace);
+  char *namespace_version = SSDATA (lnamespace_version);
   GError *error = NULL;
 
-  girepository = g_irepository_get_default();
-  g_irepository_require(girepository, namespace, namespace_version, 0, &error);
-  if (error) {
-    g_error("ERROR: %s\n", error->message);
-    return Qnil;
-  }
+  girepository = g_irepository_get_default ();
+  g_irepository_require (girepository, namespace, namespace_version, 0, &error);
+  if (error)
+    {
+      g_error ("ERROR: %s\n", error->message);
+      return Qnil;
+    }
   return Qt;
 }
 
-GtkWidget* xwgir_create(char* class, char* namespace){
-  //TODO this is more or less the same as xwgir-call-method, so should be refactored
+static GtkWidget *
+xwgir_create (char *class, char *namespace)
+{
+  // TODO this is more or less the same as xwgir-call-method,
+  // so should be refactored.
   //create a gtk widget, given its name
   //find the constructor
   //call it
   //also figure out how to pass args
 
-  GError *error = NULL;
   GIArgument return_value;
 
-  GIObjectInfo* obj_info = g_irepository_find_by_name(girepository, namespace, class);
-  GIFunctionInfo* f_info = g_object_info_find_method (obj_info, "new");
-  g_function_info_invoke(f_info,
-                         NULL, 0,
-                         NULL, 0,
-                         &return_value,
-                         NULL);
+  GIObjectInfo *obj_info
+    = g_irepository_find_by_name (girepository, namespace, class);
+  GIFunctionInfo *f_info = g_object_info_find_method (obj_info, "new");
+  g_function_info_invoke (f_info, NULL, 0, NULL, 0, &return_value, NULL);
   xwgir_create_debug = return_value.v_pointer;
   return return_value.v_pointer;
-
 }
 
-int
-xwgir_convert_lisp_to_gir_arg(GIArgument* giarg,
-                              GIArgInfo* arginfo,
-                              Lisp_Object lisparg )
+static int
+xwgir_convert_lisp_to_gir_arg (GIArgument *giarg,
+			       GIArgInfo *arginfo, Lisp_Object lisparg)
 {
-
-  GITypeTag   tag;
-  gboolean    is_pointer;
-  gboolean    is_enum;
-  tag =  g_type_info_get_tag (g_arg_info_get_type (arginfo));
+  GITypeTag tag = g_type_info_get_tag (g_arg_info_get_type (arginfo));
 
   switch (tag)
     {
     case GI_TYPE_TAG_BOOLEAN:
-      giarg->v_boolean = XFASTINT(lisparg);
+      giarg->v_boolean = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_INT8:
-      giarg->v_int8 = XFASTINT(lisparg);
+      giarg->v_int8 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_UINT8:
-      giarg->v_uint8 = XFASTINT(lisparg);
+      giarg->v_uint8 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_INT16:
-      giarg->v_int16 = XFASTINT(lisparg);
+      giarg->v_int16 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_UINT16:
-      giarg->v_uint16 = XFASTINT(lisparg);
+      giarg->v_uint16 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_INT32:
-      giarg->v_int32 = XFASTINT(lisparg);
+      giarg->v_int32 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_UINT32:
-      giarg->v_uint32 = XFASTINT(lisparg);
+      giarg->v_uint32 = XFASTINT (lisparg);
       break;
 
     case GI_TYPE_TAG_INT64:
-      giarg->v_int64 = XFASTINT(lisparg);
+      giarg->v_int64 = XFASTINT (lisparg);
       break;
     case GI_TYPE_TAG_UINT64:
-      giarg->v_uint64 = XFASTINT(lisparg);
+      giarg->v_uint64 = XFASTINT (lisparg);
       break;
 
-
     case GI_TYPE_TAG_FLOAT:
-      giarg->v_float = XFLOAT_DATA(lisparg);
+      giarg->v_float = XFLOAT_DATA (lisparg);
       break;
 
     case GI_TYPE_TAG_DOUBLE:
-      giarg->v_double = XFLOAT_DATA(lisparg);
+      giarg->v_double = XFLOAT_DATA (lisparg);
       break;
 
     case GI_TYPE_TAG_UTF8:
     case GI_TYPE_TAG_FILENAME:
-      //giarg->v_string = SDATA(lisparg);
-      giarg->v_pointer = SDATA(lisparg);
+      //giarg->v_string = SDATA (lisparg);
+      giarg->v_pointer = SDATA (lisparg);
       break;
 
     case GI_TYPE_TAG_ARRAY:
@@ -699,56 +683,64 @@ xwgir_convert_lisp_to_gir_arg(GIArgument* giarg,
     case GI_TYPE_TAG_VOID:
     case GI_TYPE_TAG_UNICHAR:
     case GI_TYPE_TAG_GTYPE:
-      //?? i dont know how to handle these yet TODO
-      printf("failed in my lisp to gir arg conversion duties. sob!\n");
+      //?? i don't know how to handle these yet.  TODO
+      printf ("failed in my lisp to gir arg conversion duties. sob!\n");
       return -1;
       break;
     }
   return 0;
 }
 
-#if 0
+#if false
 void
-refactor_attempt(){
-  //this methhod should be called from xwgir-xwidget-call-method and from xwgir xwidget construction
-  char* class = SDATA(Fcar(Fcdr(Fget(xw->type, QCxwgir_class))));
-
-  GIObjectInfo* obj_info = g_irepository_find_by_name(girepository, namespace, class);
-  GIFunctionInfo* f_info = g_object_info_find_method (obj_info, SDATA(method));
-
-  //loop over args, convert from lisp to primitive type, given arg introspection data
-  //TODO g_callable_info_get_n_args(f_info) should match
-  int argscount = XFASTINT(Flength(arguments));
-  if(argscount !=  g_callable_info_get_n_args(f_info)){
-    printf("xwgir call method arg count doesn match! \n");
-    return Qnil;
-  }
+refactor_attempt (void)
+{
+  /* This method should be called from xwgir-xwidget-call-method and
+     from xwgir xwidget construction.  */
+  char *class = SDATA (Fcar (Fcdr (Fget (xw->type, QCxwgir_class))));
+
+  GIObjectInfo *obj_info =
+    g_irepository_find_by_name (girepository, namespace, class);
+  GIFunctionInfo *f_info = g_object_info_find_method (obj_info, SDATA (method));
+
+  /* Loop over args, convert from lisp to primitive type, given arg
+     introspection data.
+
+     TODO g_callable_info_get_n_args (f_info) should match.  */
+  int argscount = XFASTINT (Flength (arguments));
+  if (argscount != g_callable_info_get_n_args (f_info))
+    {
+      printf ("xwgir call method arg count doesn't match!\n");
+      return Qnil;
+    }
   int i;
   for (i = 1; i < argscount + 1; ++i)
     {
-      xwgir_convert_lisp_to_gir_arg(&in_args[i], g_callable_info_get_arg(f_info, i - 1), Fnth(i - 1, arguments));
+      xwgir_convert_lisp_to_gir_arg (&in_args[i],
+				     g_callable_info_get_arg (f_info, i - 1),
+				     Fnth (i - 1, arguments));
     }
 
   in_args[0].v_pointer = widget;
-  if(g_function_info_invoke(f_info,
-                            in_args, argscount + 1,
-                            NULL, 0,
-                            &return_value,
-                            &error)) {
-    //g_error("ERROR: %s\n", error->message);
-    printf("invokation error\n");
-     return Qnil;
-   }
+  if (g_function_info_invoke (f_info,
+			      in_args, argscount + 1,
+			      NULL, 0, &return_value, &error))
+    {
+      //g_error ("ERROR: %s\n", error->message);
+      printf ("invokation error\n");
+      return Qnil;
+    }
   return Qt;
 }
-#endif  /* 0 */
+#endif /* false */
 
-DEFUN ("xwgir-xwidget-call-method", Fxwgir_xwidget_call_method,  Sxwgir_xwidget_call_method,       3, 3, 0,
+DEFUN ("xwgir-xwidget-call-method", Fxwgir_xwidget_call_method,
+       Sxwgir_xwidget_call_method, 3, 3, 0,
        doc:	/* Call Xwidget object method using GObject Introspection.
-                   XWIDGET is the xwidget instance to act upon.
-                    METHOD is the Gobject intrsopsection method name.
-                    ARGUMENTS is a list of arguments for the call. They will be converted to GObject types from Lisp types.
-                 */)
+XWIDGET is the xwidget instance to act upon.
+METHOD is the Gobject intrsopsection method name.
+ARGUMENTS is a list of arguments for the call.
+They will be converted to GObject types from Lisp types.  */)
   (Lisp_Object xwidget, Lisp_Object method, Lisp_Object arguments)
 {
   CHECK_XWIDGET (xwidget);
@@ -756,200 +748,216 @@ DEFUN ("xwgir-xwidget-call-method", Fxwgir_xwidget_call_method,  Sxwgir_xwidget_
   GIArgument return_value;
   GIArgument in_args[20];
 
-
-  struct xwidget* xw;
-  if (NILP (xwidget)) { printf("ERROR xwidget nil\n"); return Qnil; };
-  xw = XXWIDGET(xwidget);
-  if(NULL == xw) printf("ERROR xw is 0\n");
-  char* namespace = SDATA(Fcar(Fget(xw->type, QCxwgir_class)));
-  //we need the concrete widget, which happens in 2 ways depending on OSR or not TODO
-  GtkWidget* widget = NULL;
-  if(NULL == xw->widget_osr) {
-    widget = xwidget_view_lookup (xw, XWINDOW(FRAME_SELECTED_WINDOW (SELECTED_FRAME ()))) -> widget;
-  } else {
-    widget = xw->widget_osr;
-  }
-
-  //char* class = SDATA(SYMBOL_NAME(xw->type)); //this works but is unflexible
-  //figure out the class from the widget instead
-  /* printf("type class: %s %s\n", G_OBJECT_TYPE_NAME(widget), G_OBJECT_CLASS_NAME(G_OBJECT_GET_CLASS(widget))); */
-  /* char* class = G_OBJECT_TYPE_NAME(widget); //gives "GtkButton"(I want "Button") */
-  /* class += strlen(namespace);  //TODO check for corresponding api method. but this seems to work. */
-
-  char* class = SDATA(Fcar(Fcdr(Fget(xw->type, QCxwgir_class))));
-
-  GIObjectInfo* obj_info = g_irepository_find_by_name(girepository, namespace, class);
-  GIFunctionInfo* f_info = g_object_info_find_method (obj_info, SDATA(method));
-
-  //loop over args, convert from lisp to primitive type, given arg introspection data
-  //TODO g_callable_info_get_n_args(f_info) should match
-  int argscount = XFASTINT(Flength(arguments));
-  if(argscount !=  g_callable_info_get_n_args(f_info)){
-    printf("xwgir call method arg count doesn match! \n");
-    return Qnil;
-  }
+  struct xwidget *xw;
+  if (NILP (xwidget))
+    {
+      printf ("ERROR xwidget nil\n");
+      return Qnil;
+    };
+  xw = XXWIDGET (xwidget);
+  if (NULL == xw)
+    printf ("ERROR xw is 0\n");
+  char *namespace = SSDATA (Fcar (Fget (xw->type, QCxwgir_class)));
+  /* We need the concrete widget, which happens in two ways depending
+     on OSR or not TODO.  */
+  GtkWidget *widget = NULL;
+  widget = (xw->widget_osr
+	    ? xw->widget_osr
+	    : xwidget_view_lookup (xw,
+				   XWINDOW (FRAME_SELECTED_WINDOW
+					    (SELECTED_FRAME ())))->widget);
+
+  char *class = SSDATA (Fcar (Fcdr (Fget (xw->type, QCxwgir_class))));
+
+  GIObjectInfo *obj_info
+    = g_irepository_find_by_name (girepository, namespace, class);
+  GIFunctionInfo *f_info = g_object_info_find_method (obj_info,
+						      SSDATA (method));
+
+  /* Loop over args, convert from lisp to primitive type, given arg
+     introspection data.
+
+     TODO g_callable_info_get_n_args (f_info) should match.  */
+  int argscount = XFASTINT (Flength (arguments));
+  if (argscount != g_callable_info_get_n_args (f_info))
+    {
+      printf ("xwgir call method arg count doesn match! \n");
+      return Qnil;
+    }
   int i;
   Lisp_Object n;
   for (i = 1; i < argscount + 1; ++i)
     {
-        XSETFASTINT (n, i - 1);
-        xwgir_convert_lisp_to_gir_arg(&in_args[i], g_callable_info_get_arg(f_info, i - 1), Fnth(n, arguments));
+      XSETFASTINT (n, i - 1);
+      xwgir_convert_lisp_to_gir_arg (&in_args[i],
+				     g_callable_info_get_arg (f_info, i - 1),
+				     Fnth (n, arguments));
     }
 
   in_args[0].v_pointer = widget;
-  if(g_function_info_invoke(f_info,
-                            in_args, argscount + 1,
-                            NULL, 0,
-                            &return_value,
-                            &error)) {
-    //g_error("ERROR: %s\n", error->message);
-    printf("invokation error\n");
-     return Qnil;
-   }
+  if (g_function_info_invoke (f_info,
+			      in_args, argscount + 1,
+			      NULL, 0, &return_value, &error))
+    {
+      //g_error ("ERROR: %s\n", error->message);
+      printf ("invokation error\n");
+      return Qnil;
+    }
   return Qt;
 }
 
- void
+static void
 to_child (GtkWidget *bin,
-          double         widget_x,
-          double         widget_y,
-          double        *x_out,
-          double        *y_out)
+	  double widget_x, double widget_y, double *x_out, double *y_out)
 {
   *x_out = widget_x;
   *y_out = widget_y;
 }
 
 
-GdkWindow *
+static GdkWindow *
 offscreen_pick_embedded_child (GdkWindow *window,
-                               double x,
-                               double y,
-                               gpointer *data)
+			       double x, double y, gpointer *data)
 {
   //in this simple case we assume the window contains a single widget. easy.
   //but then we get the problem that the widget cant be embedded in several windows
   return gtk_widget_get_window (GTK_WIDGET (data));
 }
 
-void
+static void
 offscreen_to_embedder (GdkWindow *window,
-                       gdouble offscreen_x,
-                       gdouble offscreen_y,
-                       gpointer embedder_x,
-                       gpointer embedder_y,
-                       gpointer data)
+		       gdouble offscreen_x,
+		       gdouble offscreen_y,
+		       gpointer embedder_x, gpointer embedder_y, gpointer data)
 {
-  * (gdouble *) embedder_x = offscreen_x;
-  * (gdouble *) embedder_y = offscreen_y;
+  gdouble *px = embedder_x;
+  gdouble *py = embedder_y;
+  *px = offscreen_x;
+  *py = offscreen_y;
 }
 
-void
+static void
 offscreen_from_embedder (GdkWindow *window,
-                         gdouble embedder_x,
-                         gdouble embedder_y,
-                         gpointer offscreen_x,
-                         gpointer offscreen_y,
-                         gpointer user_data)
+			 gdouble embedder_x,
+			 gdouble embedder_y,
+			 gpointer offscreen_x,
+			 gpointer offscreen_y, gpointer user_data)
 {
-  * (gdouble *) offscreen_x = embedder_x;
-  * (gdouble *) offscreen_y = embedder_y;
+  gdouble *px = offscreen_x;
+  gdouble *py = offscreen_y;
+  *px = embedder_x;
+  *py = embedder_y;
 }
 
-gboolean
+static gboolean
 xwidget_osr_event_set_embedder (GtkWidget *widget,
-                                GdkEvent *event,
-                                gpointer data)
+				GdkEvent *event, gpointer data)
 {
-  struct xwidget_view *xv = (struct xwidget_view *) data;
+  struct xwidget_view *xv = data;
   struct xwidget *xww = XXWIDGET (xv->model);
-  printf("gdk_offscreen_window_set_embedder %d %d\n",
-         GDK_IS_WINDOW(gtk_widget_get_window (xww->widget_osr)),
-         GDK_IS_WINDOW(gtk_widget_get_window (GTK_WIDGET (xv->widget))));
-  gdk_offscreen_window_set_embedder (gtk_widget_get_window (xww->widgetwindow_osr),
-                                     gtk_widget_get_window (xv->widget));
+  printf ("gdk_offscreen_window_set_embedder %d %d\n",
+	  GDK_IS_WINDOW (gtk_widget_get_window (xww->widget_osr)),
+	  GDK_IS_WINDOW (gtk_widget_get_window (GTK_WIDGET (xv->widget))));
+  gdk_offscreen_window_set_embedder (gtk_widget_get_window
+				     (xww->widgetwindow_osr),
+				     gtk_widget_get_window (xv->widget));
+  return FALSE;
 }
 
 
-/* initializes and does initial placement of an xwidget view on screen */
-struct xwidget_view*
-xwidget_init_view (struct xwidget *xww,
-                   struct glyph_string *s,
-                   int x, int y)
+/* Initialize and do initial placement of an xwidget view on screen.  */
+static struct xwidget_view *
+xwidget_init_view (struct xwidget *xww, struct glyph_string *s, int x, int y)
 {
-  struct xwidget_view *xv = allocate_xwidget_view();
+  struct xwidget_view *xv = allocate_xwidget_view ();
   Lisp_Object val;
-  GdkColor color;
 
-  XSETXWIDGET_VIEW (val, xv)  ;
+  XSETXWIDGET_VIEW (val, xv);
   Vxwidget_view_list = Fcons (val, Vxwidget_view_list);
 
-  XSETWINDOW(xv->w, s->w);
-  XSETXWIDGET(xv->model, xww);
+  XSETWINDOW (xv->w, s->w);
+  XSETXWIDGET (xv->model, xww);
 
   //widget creation
-  if(EQ(xww->type, Qbutton))
+  if (EQ (xww->type, Qbutton))
+    {
+      xv->widget = gtk_button_new_with_label (SSDATA (xww->title));
+      g_signal_connect (G_OBJECT (xv->widget), "clicked", G_CALLBACK (buttonclick_handler), xv);	// the view rather than the model
+    }
+  else if (EQ (xww->type, Qtoggle))
     {
-      xv->widget = gtk_button_new_with_label (XSTRING(xww->title)->data);
-      g_signal_connect (G_OBJECT (xv->widget), "clicked",
-                        G_CALLBACK (buttonclick_handler), xv); // the view rather than the model
-    } else if (EQ(xww->type, Qtoggle)) {
-    xv->widget = gtk_toggle_button_new_with_label (XSTRING(xww->title)->data);
-    //xv->widget = gtk_entry_new ();//temp hack to experiment with key propagation TODO entry widget is useful for testing
-  } else if (EQ(xww->type, Qsocket)) {
-    xv->widget = gtk_socket_new ();
-    g_signal_connect_after(xv->widget, "plug-added", G_CALLBACK(xwidget_plug_added), "plug added");
-    g_signal_connect_after(xv->widget, "plug-removed", G_CALLBACK(xwidget_plug_removed), "plug removed");
-    //TODO these doesnt help
-    gtk_widget_add_events(xv->widget, GDK_KEY_PRESS);
-    gtk_widget_add_events(xv->widget, GDK_KEY_RELEASE);
-  } else if (EQ(xww->type, Qslider)) {
-    xv->widget =
-      //gtk_hscale_new (GTK_ADJUSTMENT(gtk_adjustment_new (0.0, 0.0, 100.0, 1.0, 10.0, 10.0)));
-      gtk_hscale_new_with_range ( 0.0, 100.0, 10.0);
-    gtk_scale_set_draw_value (GTK_SCALE (xv->widget), FALSE);	//i think its emacs role to show text and stuff, so disable the widgets own text
-    xv->handler_id = g_signal_connect_after(xv->widget, "value-changed", G_CALLBACK(xwidget_slider_changed), "slider changed");
-  } else if (EQ(xww->type, Qcairo)) {
-    //Cairo view
-    //uhm cairo is differentish in gtk 3.
-    //gdk_cairo_create (gtk_widget_get_window (FRAME_GTK_WIDGET (s->f)));
-    xv->widget = gtk_drawing_area_new();
-    g_signal_connect (G_OBJECT (    xv->widget), "draw",
-                      G_CALLBACK (xwidget_osr_draw_callback), NULL);
-
-  } else if (EQ(xww->type, Qwebkit_osr)||
-             EQ(xww->type, Qsocket_osr)||
-             (!NILP (Fget(xww->type, QCxwgir_class))))//xwgir widgets are OSR
-  {
-    printf("osr init:%s\n",SDATA(SYMBOL_NAME(xww->type)));
-    xv->widget = gtk_drawing_area_new();
-    gtk_widget_set_app_paintable ( xv->widget, TRUE); //because expose event handling
-    gtk_widget_add_events(xv->widget, GDK_ALL_EVENTS_MASK);
-
-    /* Draw the view on damage-event */
-    g_signal_connect (G_OBJECT (xww->widgetwindow_osr), "damage-event",
-                      G_CALLBACK (offscreen_damage_event), xv->widget);
-
-    if (EQ(xww->type, Qwebkit_osr)){
-      /* ///xwgir debug */
-      /* //forward events. this isnt compatible with the set_embedded strategy */
-      g_signal_connect (G_OBJECT (    xv->widget), "button-press-event",
-                        G_CALLBACK (xwidget_osr_event_forward), NULL);
-      g_signal_connect (G_OBJECT (    xv->widget), "button-release-event",
-                        G_CALLBACK (xwidget_osr_event_forward), NULL);
-      g_signal_connect (G_OBJECT (    xv->widget), "motion-notify-event",
-                        G_CALLBACK (xwidget_osr_event_forward), NULL);
-    }else{
-      //xwgir debug , orthogonal to forwarding
-      g_signal_connect (G_OBJECT (xv->widget), "enter-notify-event",
-                        G_CALLBACK (xwidget_osr_event_set_embedder), xv);
+      xv->widget = gtk_toggle_button_new_with_label (SSDATA (xww->title));
+      //xv->widget = gtk_entry_new ();//temp hack to experiment with key propagation TODO entry widget is useful for testing
     }
+  else if (EQ (xww->type, Qsocket))
+    {
+      xv->widget = gtk_socket_new ();
+      g_signal_connect_after (xv->widget, "plug-added",
+			      G_CALLBACK (xwidget_plug_added),
+			      (char *) "plug added");
+      g_signal_connect_after (xv->widget, "plug-removed",
+			      G_CALLBACK (xwidget_plug_removed),
+			      (char *) "plug removed");
+      //TODO these doesnt help
+      gtk_widget_add_events (xv->widget, GDK_KEY_PRESS);
+      gtk_widget_add_events (xv->widget, GDK_KEY_RELEASE);
+    }
+  else if (EQ (xww->type, Qslider))
+    {
+      xv->widget = gtk_hscale_new_with_range (0.0, 100.0, 10.0);
+
+      /* I think it's emacs role to show text and stuff, so disable
+	 the widget's own text.  */
+      gtk_scale_set_draw_value (GTK_SCALE (xv->widget), FALSE);
+      xv->handler_id
+	= g_signal_connect_after (xv->widget, "value-changed",
+				  G_CALLBACK (xwidget_slider_changed),
+				  (char *) "slider changed");
+    }
+  else if (EQ (xww->type, Qcairo))
+    {
+      //Cairo view
+      //uhm cairo is differentish in gtk 3.
+      //gdk_cairo_create (gtk_widget_get_window (FRAME_GTK_WIDGET (s->f)));
+      xv->widget = gtk_drawing_area_new ();
+      g_signal_connect (G_OBJECT (xv->widget), "draw",
+			G_CALLBACK (xwidget_osr_draw_callback), NULL);
 
-    //draw
-    g_signal_connect (G_OBJECT (xv->widget), "draw",
-                      G_CALLBACK (xwidget_osr_draw_callback), NULL);
+    }
+  else if (EQ (xww->type, Qwebkit_osr) || EQ (xww->type, Qsocket_osr) || (!NILP (Fget (xww->type, QCxwgir_class))))	//xwgir widgets are OSR
+    {
+      printf ("osr init:%s\n", SDATA (SYMBOL_NAME (xww->type)));
+      xv->widget = gtk_drawing_area_new ();
+      gtk_widget_set_app_paintable (xv->widget, TRUE);	//because expose event handling
+      gtk_widget_add_events (xv->widget, GDK_ALL_EVENTS_MASK);
+
+      /* Draw the view on damage-event */
+      g_signal_connect (G_OBJECT (xww->widgetwindow_osr), "damage-event",
+			G_CALLBACK (offscreen_damage_event), xv->widget);
+
+      if (EQ (xww->type, Qwebkit_osr))
+	{
+	  /* ///xwgir debug */
+	  /* //forward events. this isnt compatible with the set_embedded strategy */
+	  g_signal_connect (G_OBJECT (xv->widget), "button-press-event",
+			    G_CALLBACK (xwidget_osr_event_forward), NULL);
+	  g_signal_connect (G_OBJECT (xv->widget), "button-release-event",
+			    G_CALLBACK (xwidget_osr_event_forward), NULL);
+	  g_signal_connect (G_OBJECT (xv->widget), "motion-notify-event",
+			    G_CALLBACK (xwidget_osr_event_forward), NULL);
+	}
+      else
+	{
+	  //xwgir debug , orthogonal to forwarding
+	  g_signal_connect (G_OBJECT (xv->widget), "enter-notify-event",
+			    G_CALLBACK (xwidget_osr_event_set_embedder), xv);
+	}
+
+      //draw
+      g_signal_connect (G_OBJECT (xv->widget), "draw",
+			G_CALLBACK (xwidget_osr_draw_callback), NULL);
 
-  }
+    }
   //else return NULL;
 
   //widget realization
@@ -959,55 +967,60 @@ xwidget_init_view (struct xwidget *xww,
   //other containers than gtk_fixed where explored, but gtk_fixed had the most predictable behaviour so far.
   xv->emacswindow = FRAME_GTK_WIDGET (s->f);
   xv->widgetwindow = gtk_fixed_new ();
-  gtk_widget_set_has_window(xv->widgetwindow, TRUE);
+  gtk_widget_set_has_window (xv->widgetwindow, TRUE);
   gtk_container_add (GTK_CONTAINER (xv->widgetwindow), xv->widget);
 
   //store some xwidget data in the gtk widgets
-  g_object_set_data (G_OBJECT (xv->widget), XG_FRAME_DATA, (gpointer) (s->f)); //the emacs frame
-  g_object_set_data (G_OBJECT (xv->widget), XG_XWIDGET, (gpointer) (xww)); //the xwidget
-  g_object_set_data (G_OBJECT (xv->widget), XG_XWIDGET_VIEW, (gpointer) (xv)); //the xwidget
-  g_object_set_data (G_OBJECT (xv->widgetwindow), XG_XWIDGET, (gpointer) (xww)); //the xwidget window
-  g_object_set_data (G_OBJECT (xv->widgetwindow), XG_XWIDGET_VIEW, (gpointer) (xv)); //the xwidget window
+  g_object_set_data (G_OBJECT (xv->widget), XG_FRAME_DATA, (gpointer) (s->f));	//the emacs frame
+  g_object_set_data (G_OBJECT (xv->widget), XG_XWIDGET, (gpointer) (xww));	//the xwidget
+  g_object_set_data (G_OBJECT (xv->widget), XG_XWIDGET_VIEW, (gpointer) (xv));	//the xwidget
+  g_object_set_data (G_OBJECT (xv->widgetwindow), XG_XWIDGET, (gpointer) (xww));	//the xwidget window
+  g_object_set_data (G_OBJECT (xv->widgetwindow), XG_XWIDGET_VIEW, (gpointer) (xv));	//the xwidget window
 
 
-  gtk_widget_set_size_request (GTK_WIDGET (xv->widget), xww->width, xww->height);
+  gtk_widget_set_size_request (GTK_WIDGET (xv->widget), xww->width,
+			       xww->height);
   gtk_widget_set_size_request (xv->widgetwindow, xww->width, xww->height);
   gtk_fixed_put (GTK_FIXED (FRAME_GTK_WIDGET (s->f)), xv->widgetwindow, x, y);
-  xv->x = x;  xv->y = y;
+  xv->x = x;
+  xv->y = y;
   gtk_widget_show_all (xv->widgetwindow);
 
 
 
   //widgettype specific initialization only possible after realization
-  if (EQ(xww->type, Qsocket)) {
-    printf ("xwid:%d socket id:%x %d\n",
-            xww,
-            gtk_socket_get_id (GTK_SOCKET (xv->widget)),
-            gtk_socket_get_id (GTK_SOCKET (xv->widget)));
-    send_xembed_ready_event (xww,
-                             gtk_socket_get_id (GTK_SOCKET (xv->widget)));
-    //gtk_widget_realize(xw->widget);
-  }
-
-  //////////////////////////////////////////////////////////////
+  if (EQ (xww->type, Qsocket))
+    {
+      printf ("xwid:%p socket id:%x %d\n",
+	      xww,
+	      (int) gtk_socket_get_id (GTK_SOCKET (xv->widget)),
+	      (int) gtk_socket_get_id (GTK_SOCKET (xv->widget)));
+      send_xembed_ready_event (xww,
+			       gtk_socket_get_id (GTK_SOCKET (xv->widget)));
+      // gtk_widget_realize (xw->widget);
+    }
+
   // xwgir debug
-  if (//EQ(xww->type, Qwebkit_osr)|| //TODO should be able to choose compile time which method to use with webkit
-      EQ(xww->type, Qsocket_osr)||
-      (!NILP (Fget(xww->type, QCxwgir_class))))//xwgir widgets are OSR
+  if (EQ (xww->type, Qsocket_osr) || !NILP (Fget (xww->type, QCxwgir_class)))
     {
-      printf("gdk_offscreen_window_set_embedder %d %d\n",
-             GDK_IS_WINDOW(gtk_widget_get_window (xww->widget_osr)),
-             GDK_IS_WINDOW(gtk_widget_get_window (GTK_WIDGET (xv->widget))));
+      printf ("gdk_offscreen_window_set_embedder %d %d\n",
+	      GDK_IS_WINDOW (gtk_widget_get_window (xww->widget_osr)),
+	      GDK_IS_WINDOW (gtk_widget_get_window (GTK_WIDGET (xv->widget))));
       // set_embedder needs to be called after xv->widget realization
-      gdk_offscreen_window_set_embedder (gtk_widget_get_window (xww->widgetwindow_osr),
-                                         gtk_widget_get_window (xv->widget));
-      g_signal_connect (gtk_widget_get_window (xv->widget), "pick-embedded-child",
-                        G_CALLBACK (offscreen_pick_embedded_child), xww->widgetwindow_osr);
-
-      g_signal_connect (gtk_widget_get_window (xww->widgetwindow_osr), "from-embedder",
-                        G_CALLBACK (offscreen_from_embedder), NULL);
-      g_signal_connect (gtk_widget_get_window (xww->widgetwindow_osr), "to-embedder",
-                        G_CALLBACK (offscreen_to_embedder), NULL);
+      gdk_offscreen_window_set_embedder (gtk_widget_get_window
+					 (xww->widgetwindow_osr),
+					 gtk_widget_get_window (xv->widget));
+      g_signal_connect (gtk_widget_get_window (xv->widget),
+			"pick-embedded-child",
+			G_CALLBACK (offscreen_pick_embedded_child),
+			xww->widgetwindow_osr);
+
+      g_signal_connect (gtk_widget_get_window (xww->widgetwindow_osr),
+			"from-embedder", G_CALLBACK (offscreen_from_embedder),
+			NULL);
+      g_signal_connect (gtk_widget_get_window (xww->widgetwindow_osr),
+			"to-embedder", G_CALLBACK (offscreen_to_embedder),
+			NULL);
     }
   ////////////////////////////////////////
 
@@ -1018,215 +1031,237 @@ xwidget_init_view (struct xwidget *xww,
 void
 x_draw_xwidget_glyph_string (struct glyph_string *s)
 {
-  /*
-    this method is called by the redisplay engine and places the xwidget on screen.
-    moving and clipping is done here. also view init.
-
-  */
-  int box_line_hwidth = eabs (s->face->box_line_width);
-  int box_line_vwidth = max (s->face->box_line_width, 0);
-  int height = s->height;
+  /* This method is called by the redisplay engine and places the
+     xwidget on screen.  Moving and clipping is done here.  Also view
+     init.  */
   struct xwidget *xww = s->xwidget;
-  struct xwidget_view *xv = xwidget_view_lookup(xww, s->w);
-  int clip_right; int clip_bottom; int clip_top; int clip_left;
+  struct xwidget_view *xv = xwidget_view_lookup (xww, s->w);
+  int clip_right;
+  int clip_bottom;
+  int clip_top;
+  int clip_left;
 
   int x = s->x;
   int y = s->y + (s->height / 2) - (xww->height / 2);
-  int moved=0;
+  int moved = 0;
 
   /* We do it here in the display loop because there is no other
      time to know things like window placement etc.
-  */
-  printf ("xv init for xw %d\n", xww);
+   */
+  printf ("xv init for xw %p\n", xww);
   xv = xwidget_init_view (xww, s, x, y);
 
   //calculate clipping, which is used for all manner of onscreen xwidget views
   //each widget border can get clipped by other emacs objects so there are four clipping variables
-  clip_right = min (xww->width, WINDOW_RIGHT_EDGE_X (s->w) - x - WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH(s->w) - WINDOW_RIGHT_FRINGE_WIDTH(s->w));
-  clip_left = max (0, WINDOW_LEFT_EDGE_X (s->w) - x + WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH(s->w) + WINDOW_LEFT_FRINGE_WIDTH(s->w));
-
-  clip_bottom = min (xww->height, WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
-  clip_top = max(0, WINDOW_TOP_EDGE_Y(s->w) -y );
+  clip_right =
+    min (xww->width,
+	 WINDOW_RIGHT_EDGE_X (s->w) - x -
+	 WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
+	 WINDOW_RIGHT_FRINGE_WIDTH (s->w));
+  clip_left =
+    max (0,
+	 WINDOW_LEFT_EDGE_X (s->w) - x +
+	 WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
+	 WINDOW_LEFT_FRINGE_WIDTH (s->w));
+
+  clip_bottom =
+    min (xww->height,
+	 WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
+  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
 
   //we are conserned with movement of the onscreen area. the area might sit still when the widget actually moves
   //this happens when an emacs window border moves across a widget window
   //so, if any corner of the outer widget clippng window moves, that counts as movement here, even
   //if it looks like no movement happens because the widget sits still inside the clipping area.
   //the widget can also move inside the clipping area, which happens later
-  moved = (xv->x  + xv->clip_left != x+clip_left)
-    || ((xv->y + xv->clip_top)!= (y+clip_top));
+  moved = (xv->x + xv->clip_left != x + clip_left)
+    || ((xv->y + xv->clip_top) != (y + clip_top));
   xv->x = x;
   xv->y = y;
-  if (moved)	//has it moved?
+  if (moved)			//has it moved?
     {
-      if (1)//!xwidget_hidden(xv))	//hidden equals not being seen during redisplay
-        {
-          //TODO should be possible to use xwidget_show_view here
-          gtk_fixed_move (GTK_FIXED (FRAME_GTK_WIDGET (s->f)),
-                          xv->widgetwindow,
-                          x + clip_left, y + clip_top);
-        }
+      //hidden equals not being seen during redisplay
+      if (1)			//!xwidget_hidden (xv))
+	{
+	  //TODO should be possible to use xwidget_show_view here
+	  gtk_fixed_move (GTK_FIXED (FRAME_GTK_WIDGET (s->f)),
+			  xv->widgetwindow, x + clip_left, y + clip_top);
+	}
     }
   //clip the widget window if some parts happen to be outside drawable area
   //an emacs window is not a gtk window, a gtk window covers the entire frame
   //cliping might have changed even if we havent actualy moved, we try figure out when we need to reclip for real
-  if((xv->clip_right != clip_right)
-     || (xv->clip_bottom != clip_bottom)
-     || (xv->clip_top != clip_top)
-     || (xv->clip_left != clip_left)){
-    gtk_widget_set_size_request (xv->widgetwindow,  clip_right + clip_left, clip_bottom + clip_top);
-    gtk_fixed_move(GTK_FIXED(xv->widgetwindow), xv->widget, -clip_left, -clip_top);
-
-    xv->clip_right = clip_right; xv->clip_bottom = clip_bottom; xv->clip_top = clip_top;xv->clip_left = clip_left;
-  }
+  if ((xv->clip_right != clip_right)
+      || (xv->clip_bottom != clip_bottom)
+      || (xv->clip_top != clip_top) || (xv->clip_left != clip_left))
+    {
+      gtk_widget_set_size_request (xv->widgetwindow, clip_right + clip_left,
+				   clip_bottom + clip_top);
+      gtk_fixed_move (GTK_FIXED (xv->widgetwindow), xv->widget, -clip_left,
+		      -clip_top);
+
+      xv->clip_right = clip_right;
+      xv->clip_bottom = clip_bottom;
+      xv->clip_top = clip_top;
+      xv->clip_left = clip_left;
+    }
   //if emacs wants to repaint the area where the widget lives, queue a redraw
   //TODO it seems its possible to get out of sync with emacs redraws so emacs bg sometimes shows up instead of xwidget
   //its just a visual glitch though
-  if (!xwidget_hidden(xv)){
-    gtk_widget_queue_draw (xv->widgetwindow);
-    gtk_widget_queue_draw (xv->widget);
-  }
+  if (!xwidget_hidden (xv))
+    {
+      gtk_widget_queue_draw (xv->widgetwindow);
+      gtk_widget_queue_draw (xv->widget);
+    }
 }
 
 
 #ifdef HAVE_WEBKIT_OSR
 
-//FUGLY macro that checks WEBKIT_IS_WEB_VIEW(xw->widget_osr) first
-#define WEBKIT_FN_INIT()                        \
-  struct xwidget* xw; \
-  CHECK_XWIDGET (xwidget); \
- if(NILP (xwidget)) {printf("ERROR xwidget nil\n"); return Qnil;};    \
-  xw = XXWIDGET(xwidget);                                                    \
-  if(NULL == xw) printf("ERROR xw is 0\n");                               \
-  if((NULL == xw->widget_osr) || !WEBKIT_IS_WEB_VIEW(xw->widget_osr)){  \
-    printf("ERROR xw->widget_osr does not hold a webkit instance\n");\
-    return Qnil;\
-  };
-
-
-DEFUN ("xwidget-webkit-goto-uri", Fxwidget_webkit_goto_uri,  Sxwidget_webkit_goto_uri,
-       2, 2, 0,
-       doc:	/* Make the webkit instance referenced by XWIDGET browse URI. */)
+// UGLY macro that checks WEBKIT_IS_WEB_VIEW (xw->widget_osr) first.
+#define WEBKIT_FN_INIT()						\
+  struct xwidget *xw;							\
+  CHECK_XWIDGET (xwidget);						\
+  if (NILP (xwidget)) { printf ("ERROR xwidget nil\n"); return Qnil; };	\
+  xw = XXWIDGET (xwidget);						\
+  if (NULL == xw) printf ("ERROR xw is 0\n");				\
+  if ((NULL == xw->widget_osr) || !WEBKIT_IS_WEB_VIEW (xw->widget_osr))	\
+    {									\
+      printf ("ERROR xw->widget_osr does not hold a webkit instance\n"); \
+      return Qnil;							\
+    };
+
+
+DEFUN ("xwidget-webkit-goto-uri", Fxwidget_webkit_goto_uri,
+       Sxwidget_webkit_goto_uri, 2, 2, 0,
+       doc: /* Make the webkit instance referenced by XWIDGET browse URI. */)
   (Lisp_Object xwidget, Lisp_Object uri)
 {
-  WEBKIT_FN_INIT();
-  CHECK_STRING(uri);
-  webkit_web_view_load_uri ( WEBKIT_WEB_VIEW(xw->widget_osr), SDATA(uri));
+  WEBKIT_FN_INIT ();
+  CHECK_STRING (uri);
+  webkit_web_view_load_uri (WEBKIT_WEB_VIEW (xw->widget_osr), SSDATA (uri));
   return Qnil;
 }
 
 
-DEFUN ("xwidget-webkit-execute-script", Fxwidget_webkit_execute_script,  Sxwidget_webkit_execute_script,
-       2, 2, 0,
-       doc:	/* webkit exec js.*/)
+DEFUN ("xwidget-webkit-execute-script", Fxwidget_webkit_execute_script,
+       Sxwidget_webkit_execute_script, 2, 2, 0,
+       doc: /* webkit exec js. */)
   (Lisp_Object xwidget, Lisp_Object script)
 {
-  WEBKIT_FN_INIT();
-  CHECK_STRING(script);
-  webkit_web_view_execute_script( WEBKIT_WEB_VIEW(xw->widget_osr), SDATA(script));
+  WEBKIT_FN_INIT ();
+  CHECK_STRING (script);
+  webkit_web_view_execute_script (WEBKIT_WEB_VIEW (xw->widget_osr),
+				  SSDATA (script));
   return Qnil;
 }
 
-DEFUN ("xwidget-webkit-get-title", Fxwidget_webkit_get_title,  Sxwidget_webkit_get_title,
-       1, 1, 0,
-       doc:	/* Get the title from the Webkit instance in XWIDGET.
-                   This can be used to work around the lack of a return value from the exec method.
-                */)
+DEFUN ("xwidget-webkit-get-title", Fxwidget_webkit_get_title,
+       Sxwidget_webkit_get_title, 1, 1, 0,
+       doc: /* Get the title from the Webkit instance in XWIDGET.
+This can be used to work around the lack of a return value from the
+exec method.  */)
   (Lisp_Object xwidget)
 {
   //TODO support multibyte strings
-  WEBKIT_FN_INIT();
-  const gchar* str=webkit_web_view_get_title( WEBKIT_WEB_VIEW(xw->widget_osr));
-  //return make_string_from_bytes(str, wcslen((const wchar_t *)str), strlen(str));
-  if(str == 0){
-    //TODO maybe return Qnil instead. I suppose webkit returns nullpointer when doc is not properly loaded or something
-    printf("xwidget-webkit-get-title null webkit title\n");
-    return build_string("");
-  }
-  return build_string(str);
+  WEBKIT_FN_INIT ();
+  const gchar *str
+    = webkit_web_view_get_title (WEBKIT_WEB_VIEW (xw->widget_osr));
+  if (str == 0)
+    {
+      /* TODO maybe return Qnil instead. I suppose webkit returns
+	 nullpointer when doc is not properly loaded or something.  */
+      printf ("xwidget-webkit-get-title null webkit title\n");
+      return build_string ("");
+    }
+  return build_string (str);
 }
 
 //TODO missnamed
-DEFUN ("xwidget-disable-plugin-for-mime", Fxwidget_disable_plugin_for_mime , Sxwidget_disable_plugin_for_mime,
-      1,1,0, doc: /* */)
+DEFUN ("xwidget-disable-plugin-for-mime", Fxwidget_disable_plugin_for_mime, Sxwidget_disable_plugin_for_mime, 1, 1, 0, doc:	/* */ )
   (Lisp_Object mime)
 {
   WebKitWebPlugin *wp = webkit_web_plugin_database_get_plugin_for_mimetype
-    (webkit_get_web_plugin_database(),  SDATA(mime));
-  if(wp == NULL) return Qnil;
-  if(webkit_web_plugin_get_enabled (wp)){
-    webkit_web_plugin_set_enabled  (wp, FALSE);
-    return Qt;
-  }
+    (webkit_get_web_plugin_database (), SSDATA (mime));
+  if (wp == NULL)
+    return Qnil;
+  if (webkit_web_plugin_get_enabled (wp))
+    {
+      webkit_web_plugin_set_enabled (wp, FALSE);
+      return Qt;
+    }
   return Qnil;
 }
 
 
-void
-xwidget_webkit_dom_dump(WebKitDOMNode* parent)
+static void
+xwidget_webkit_dom_dump (WebKitDOMNode *parent)
 {
-  WebKitDOMNodeList* list;
+  WebKitDOMNodeList *list;
   int i;
   int length;
-  WebKitDOMNode* attribute;
-  WebKitDOMNamedNodeMap* attrs;
-  WebKitDOMNode* child;
-  printf("node:%d type:%d name:%s content:%s\n",
-         parent,
-         webkit_dom_node_get_node_type(parent),//1 element 3 text 8 comment 2 attribute
-         webkit_dom_node_get_local_name(parent),
-         webkit_dom_node_get_text_content(parent));
-
-  if(webkit_dom_node_has_attributes(parent)){
-    attrs = webkit_dom_node_get_attributes(parent);
-
-    length = webkit_dom_named_node_map_get_length(attrs);
-    for (int i = 0; i < length; i++) {
-      attribute = webkit_dom_named_node_map_item(attrs,i);
-      printf(" attr node:%d type:%d name:%s content:%s\n",
-             attribute,
-             webkit_dom_node_get_node_type(attribute),//1 element 3 text 8 comment
-             webkit_dom_node_get_local_name(attribute),
-             webkit_dom_node_get_text_content(attribute));
+  WebKitDOMNode *attribute;
+  WebKitDOMNamedNodeMap *attrs;
+  WebKitDOMNode *child;
+  printf ("node:%p type:%d name:%s content:%s\n", parent,
+	  webkit_dom_node_get_node_type (parent),
+	  //1 element 3 text 8 comment 2 attribute
+	  webkit_dom_node_get_local_name (parent),
+	  webkit_dom_node_get_text_content (parent));
+
+  if (webkit_dom_node_has_attributes (parent))
+    {
+      attrs = webkit_dom_node_get_attributes (parent);
+
+      length = webkit_dom_named_node_map_get_length (attrs);
+      for (int i = 0; i < length; i++)
+	{
+	  attribute = webkit_dom_named_node_map_item (attrs, i);
+	  printf (" attr node:%d type:%d name:%s content:%s\n",
+		  attribute, webkit_dom_node_get_node_type (attribute),
+		  //1 element 3 text 8 comment
+		  webkit_dom_node_get_local_name (attribute),
+		  webkit_dom_node_get_text_content (attribute));
+	}
     }
-  }
-  list = webkit_dom_node_get_child_nodes(parent);
-  length = webkit_dom_node_list_get_length(list);
+  list = webkit_dom_node_get_child_nodes (parent);
+  length = webkit_dom_node_list_get_length (list);
 
-  for (int i = 0; i < length; i++) {
-    child = webkit_dom_node_list_item(list, i);
-    //if(webkit_dom_node_has_child_nodes(child))
-    xwidget_webkit_dom_dump(child);
-  }
+  for (int i = 0; i < length; i++)
+    {
+      child = webkit_dom_node_list_item (list, i);
+      xwidget_webkit_dom_dump (child);
+    }
 }
 
 
-DEFUN ("xwidget-webkit-dom-dump", Fxwidget_webkit_dom_dump,  Sxwidget_webkit_dom_dump,
-       1, 1, 0,
-       doc:	/*Dump the DOM contained in the webkit instance in XWIDGET.*/)
+DEFUN ("xwidget-webkit-dom-dump", Fxwidget_webkit_dom_dump, Sxwidget_webkit_dom_dump, 1, 1, 0, doc:	/*Dump the DOM contained in the webkit instance in XWIDGET. */
+       )
   (Lisp_Object xwidget)
 {
-  WEBKIT_FN_INIT();
-  xwidget_webkit_dom_dump(WEBKIT_DOM_NODE(webkit_web_view_get_dom_document( WEBKIT_WEB_VIEW(xw->widget_osr))));
+  WEBKIT_FN_INIT ();
+  xwidget_webkit_dom_dump (WEBKIT_DOM_NODE
+			   (webkit_web_view_get_dom_document
+			    (WEBKIT_WEB_VIEW (xw->widget_osr))));
   return Qnil;
 }
 
 
 
-#endif  /* HAVE_WEBKIT_OSR */
+#endif /* HAVE_WEBKIT_OSR */
 
 
 
 DEFUN ("xwidget-resize", Fxwidget_resize, Sxwidget_resize, 3, 3, 0, doc:
        /* Resize XWIDGET.
           NEW_WIDTH NEW_HEIGHT defines the new size.)
-        */)
+        */ )
   (Lisp_Object xwidget, Lisp_Object new_width, Lisp_Object new_height)
 {
   CHECK_XWIDGET (xwidget);
-  struct xwidget* xw = XXWIDGET(xwidget);
+  struct xwidget *xw = XXWIDGET (xwidget);
   struct xwidget_view *xv;
-  int  w, h;
+  int w, h;
 
   CHECK_NUMBER (new_width);
   CHECK_NUMBER (new_height);
@@ -1234,38 +1269,46 @@ DEFUN ("xwidget-resize", Fxwidget_resize, Sxwidget_resize, 3, 3, 0, doc:
   h = XFASTINT (new_height);
 
 
-  printf("resize xwidget %d (%d,%d)->(%d,%d)\n",xw, xw->width,xw->height,w,h);
-  xw->width=w;
-  xw->height=h;
+  printf ("resize xwidget %d (%d,%d)->(%d,%d)\n", xw, xw->width, xw->height, w,
+	  h);
+  xw->width = w;
+  xw->height = h;
   //if theres a osr resize it 1st
-  if(xw->widget_osr){
-    printf("resize xwidget_osr\n");
-    //gtk_container_set_resize_mode ( GTK_WINDOW(xw->widgetwindow_osr), GTK_RESIZE_QUEUE);
-    //gtk_container_set_resize_mode ( GTK_WINDOW(xw->widget_osr), GTK_RESIZE_QUEUE);
-
-
-    //gtk_layout_set_size (GTK_LAYOUT (xw->widgetwindow_osr), xw->width, xw->height);
-    gtk_widget_set_size_request (GTK_WIDGET (xw->widget_osr), xw->width, xw->height); //minimum size
-    //gtk_window_resize(    GTK_WINDOW(xw->widget_osr), xw->width, xw->height);
-    gtk_window_resize(    GTK_WINDOW(xw->widgetwindow_osr), xw->width, xw->height);
-    gtk_window_resize(    GTK_WINDOW(xw->widgetscrolledwindow_osr), xw->width, xw->height);
-    gtk_scrolled_window_set_min_content_height(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr ),xw->height);
-    gtk_scrolled_window_set_min_content_width(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr ),xw->width);
-
-    //gtk_container_resize_children ( GTK_WINDOW(xw->widgetwindow_osr));
-    gtk_container_resize_children (GTK_CONTAINER(xw->widgetwindow_osr));
-
-  }
+  if (xw->widget_osr)
+    {
+      printf ("resize xwidget_osr\n");
+
+      // Minimum size.
+      gtk_widget_set_size_request (GTK_WIDGET (xw->widget_osr),
+				   xw->width, xw->height);
+
+      gtk_window_resize (GTK_WINDOW (xw->widgetwindow_osr), xw->width,
+			 xw->height);
+      gtk_window_resize (GTK_WINDOW (xw->widgetscrolledwindow_osr), xw->width,
+			 xw->height);
+      gtk_scrolled_window_set_min_content_height (GTK_SCROLLED_WINDOW
+						  (xw->
+						   widgetscrolledwindow_osr),
+						  xw->height);
+      gtk_scrolled_window_set_min_content_width (GTK_SCROLLED_WINDOW
+						 (xw->widgetscrolledwindow_osr),
+						 xw->width);
+      gtk_container_resize_children (GTK_CONTAINER (xw->widgetwindow_osr));
+    }
 
-  for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail)) //TODO MVC refactor lazy linear search
+  for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))	//TODO MVC refactor lazy linear search
     {
-      if (XWIDGET_VIEW_P (XCAR (tail))) {
-        xv = XXWIDGET_VIEW (XCAR (tail));
-        if(XXWIDGET (xv->model) == xw) {
-          gtk_layout_set_size (GTK_LAYOUT (xv->widgetwindow), xw->width, xw->height);
-          gtk_widget_set_size_request (GTK_WIDGET (xv->widget), xw->width, xw->height);
-        }
-      }
+      if (XWIDGET_VIEW_P (XCAR (tail)))
+	{
+	  xv = XXWIDGET_VIEW (XCAR (tail));
+	  if (XXWIDGET (xv->model) == xw)
+	    {
+	      gtk_layout_set_size (GTK_LAYOUT (xv->widgetwindow), xw->width,
+				   xw->height);
+	      gtk_widget_set_size_request (GTK_WIDGET (xv->widget), xw->width,
+					   xw->height);
+	    }
+	}
     }
 
   return Qnil;
@@ -1274,28 +1317,39 @@ DEFUN ("xwidget-resize", Fxwidget_resize, Sxwidget_resize, 3, 3, 0, doc:
 
 
 DEFUN ("xwidget-set-adjustment", Fxwidget_set_adjustment, Sxwidget_set_adjustment, 4, 4, 0, doc:
-       /* set scrolling  */)
-  (Lisp_Object xwidget, Lisp_Object axis, Lisp_Object relative, Lisp_Object value)
+       /* set scrolling  */
+)
+  (Lisp_Object xwidget, Lisp_Object axis, Lisp_Object relative,
+   Lisp_Object value)
 {
   CHECK_XWIDGET (xwidget);
-  struct xwidget* xw = XXWIDGET(xwidget);
-  GtkAdjustment* adjustment;
-  float final_value=0.0;
+  struct xwidget *xw = XXWIDGET (xwidget);
+  GtkAdjustment *adjustment;
+  float final_value = 0.0;
 
-  if(EQ(Qvertical, axis)){
-    adjustment = gtk_scrolled_window_get_vadjustment(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr));
-  }
-  if(EQ(Qhorizontal, axis)){
-    adjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(xw->widgetscrolledwindow_osr));
-  }
+  if (EQ (Qvertical, axis))
+    {
+      adjustment =
+	gtk_scrolled_window_get_vadjustment (GTK_SCROLLED_WINDOW
+					     (xw->widgetscrolledwindow_osr));
+    }
+  if (EQ (Qhorizontal, axis))
+    {
+      adjustment =
+	gtk_scrolled_window_get_hadjustment (GTK_SCROLLED_WINDOW
+					     (xw->widgetscrolledwindow_osr));
+    }
 
-  if(EQ(Qt, relative)){
-    final_value=gtk_adjustment_get_value(adjustment)+XFASTINT(value);
-  }else{
-     final_value=0.0+XFASTINT(value);
-  }
+  if (EQ (Qt, relative))
+    {
+      final_value = gtk_adjustment_get_value (adjustment) + XFASTINT (value);
+    }
+  else
+    {
+      final_value = 0.0 + XFASTINT (value);
+    }
 
-  gtk_adjustment_set_value(adjustment, final_value);
+  gtk_adjustment_set_value (adjustment, final_value);
 
   return Qnil;
 }
@@ -1304,59 +1358,59 @@ DEFUN ("xwidget-set-adjustment", Fxwidget_set_adjustment, Sxwidget_set_adjustmen
 DEFUN ("xwidget-size-request", Fxwidget_size_request, Sxwidget_size_request, 1, 1, 0, doc:
        /* Desired size of the XWIDGET.
 
-  This can be used to read the xwidget desired size,  and resizes the Emacs allocated area accordingly.
+          This can be used to read the xwidget desired size,  and resizes the Emacs allocated area accordingly.
 
-(TODO crashes if arg not osr widget)*/)
+          (TODO crashes if arg not osr widget) */
+)
   (Lisp_Object xwidget)
 {
   CHECK_XWIDGET (xwidget);
   GtkRequisition requisition;
   Lisp_Object rv;
-  gtk_widget_size_request(XXWIDGET(xwidget)->widget_osr, &requisition);
+  gtk_widget_size_request (XXWIDGET (xwidget)->widget_osr, &requisition);
   rv = Qnil;
-  rv = Fcons (make_number(requisition.height), rv);
-  rv = Fcons (make_number(requisition.width), rv);
+  rv = Fcons (make_number (requisition.height), rv);
+  rv = Fcons (make_number (requisition.width), rv);
   return rv;
 
 }
 
-DEFUN ("xwidgetp", Fxwidgetp, Sxwidgetp, 1, 1, 0,
-       doc: /* Return t if OBJECT is a xwidget.  */)
+DEFUN ("xwidgetp", Fxwidgetp, Sxwidgetp, 1, 1, 0, doc:	/* Return t if OBJECT is a xwidget.  */
+       )
   (Lisp_Object object)
 {
   return XWIDGETP (object) ? Qt : Qnil;
 }
 
-DEFUN ("xwidget-view-p", Fxwidget_view_p, Sxwidget_view_p, 1, 1, 0,
-       doc: /* Return t if OBJECT is a xwidget-view.  */)
+DEFUN ("xwidget-view-p", Fxwidget_view_p, Sxwidget_view_p, 1, 1, 0, doc:/* Return t if OBJECT is a xwidget-view.  */
+       )
   (Lisp_Object object)
 {
   return XWIDGET_VIEW_P (object) ? Qt : Qnil;
 }
 
-DEFUN ("xwidget-info", Fxwidget_info , Sxwidget_info, 1,1,0,
-       doc: /* Get XWIDGET properties.
-             Currently type, title, width, height.*/)
+DEFUN ("xwidget-info", Fxwidget_info, Sxwidget_info, 1, 1, 0, doc:	/* Get XWIDGET properties.
+									   Currently type, title, width, height. */ )
   (Lisp_Object xwidget)
 {
   CHECK_XWIDGET (xwidget);
   Lisp_Object info, n;
-  struct xwidget* xw = XXWIDGET(xwidget);
+  struct xwidget *xw = XXWIDGET (xwidget);
 
   info = Fmake_vector (make_number (4), Qnil);
   ASET (info, 0, xw->type);
   ASET (info, 1, xw->title);
-  XSETFASTINT(n, xw->width);
+  XSETFASTINT (n, xw->width);
   ASET (info, 2, n);
-  XSETFASTINT(n, xw->height);
+  XSETFASTINT (n, xw->height);
   ASET (info, 3, n);
 
   return info;
 }
 
-DEFUN ("xwidget-view-info", Fxwidget_view_info , Sxwidget_view_info, 1, 1, 0, doc:
+DEFUN ("xwidget-view-info", Fxwidget_view_info, Sxwidget_view_info, 1, 1, 0, doc:
        /* Get XWIDGET-VIEW properties.
-        Currently x,y clip right, clip bottom, clip top, clip left*/)
+          Currently x,y clip right, clip bottom, clip top, clip left */ )
   (Lisp_Object xwidget_view)
 {
   CHECK_XWIDGET_VIEW (xwidget_view);
@@ -1364,71 +1418,64 @@ DEFUN ("xwidget-view-info", Fxwidget_view_info , Sxwidget_view_info, 1, 1, 0, do
   Lisp_Object info;
 
   info = Fmake_vector (make_number (6), Qnil);
-  ASET (info, 0,  make_number(xv->x));
-  ASET (info, 1,  make_number(xv->y));
-  ASET (info, 2,  make_number(xv->clip_right));
-  ASET (info, 3,  make_number(xv->clip_bottom));
-  ASET (info, 4,  make_number(xv->clip_top));
-  ASET (info, 5,  make_number(xv->clip_left));
+  ASET (info, 0, make_number (xv->x));
+  ASET (info, 1, make_number (xv->y));
+  ASET (info, 2, make_number (xv->clip_right));
+  ASET (info, 3, make_number (xv->clip_bottom));
+  ASET (info, 4, make_number (xv->clip_top));
+  ASET (info, 5, make_number (xv->clip_left));
 
   return info;
 }
 
-DEFUN ("xwidget-view-model", Fxwidget_view_model, Sxwidget_view_model,
-       1, 1, 0,
-       doc: /* Get XWIDGET-VIEW model. */)
+DEFUN ("xwidget-view-model", Fxwidget_view_model, Sxwidget_view_model, 1, 1, 0, doc:	/* Get XWIDGET-VIEW model. */
+       )
   (Lisp_Object xwidget_view)
 {
   CHECK_XWIDGET_VIEW (xwidget_view);
   return XXWIDGET_VIEW (xwidget_view)->model;
 }
 
-DEFUN ("xwidget-view-window", Fxwidget_view_window, Sxwidget_view_window,
-       1, 1, 0,
-       doc: /* Get XWIDGET-VIEW window. */)
+DEFUN ("xwidget-view-window", Fxwidget_view_window, Sxwidget_view_window, 1, 1, 0, doc:/* Get XWIDGET-VIEW window. */
+       )
   (Lisp_Object xwidget_view)
 {
   CHECK_XWIDGET_VIEW (xwidget_view);
   return XXWIDGET_VIEW (xwidget_view)->w;
 }
 
-DEFUN ("xwidget-send-keyboard-event", Fxwidget_send_keyboard_event, Sxwidget_send_keyboard_event, 2, 2, 0,
-       doc:/* Synthesize a kbd event for  XWIDGET. TODO crashes atm.. */
-       )
-  (Lisp_Object  xwidget, Lisp_Object keydescriptor)
+DEFUN ("xwidget-send-keyboard-event", Fxwidget_send_keyboard_event,
+       Sxwidget_send_keyboard_event, 2, 2, 0,
+       doc: /* Synthesize a kbd event for  XWIDGET. TODO crashes atm.. */)
+  (Lisp_Object xwidget, Lisp_Object keydescriptor)
 {
   //TODO this code crashes for offscreen widgets and ive tried many different strategies
   //int keyval = 0x058; //X
-  int keyval = XFASTINT(keydescriptor); //X
-  char *keystring = "";
-  GdkKeymapKey* keys;
+  int keyval = XFASTINT (keydescriptor);	//X
+  GdkKeymapKey *keys;
   gint n_keys;
-  GdkDeviceManager* manager;
-  struct xwidget *xw;
-  GtkWidget* widget;
-  GdkEventKey* ev;
-  Lisp_Object window;
   //popup_activated_flag = 1; //TODO just a hack
-  gdk_keymap_get_entries_for_keyval(gdk_keymap_get_default(), keyval, &keys, &n_keys);
+  gdk_keymap_get_entries_for_keyval (gdk_keymap_get_default (), keyval, &keys,
+				     &n_keys);
 
-  xw = XXWIDGET(xwidget);
+  struct xwidget *xw = XXWIDGET (xwidget);
 
-  ev = (GdkEventKey*)gdk_event_new(GDK_KEY_PRESS);
+  GdkEventKey *ev = gdk_event_new (GDK_KEY_PRESS);
 
 
   //todo what about windowless widgets?
 
-  window = FRAME_SELECTED_WINDOW (SELECTED_FRAME ());
+  Lisp_Object window = FRAME_SELECTED_WINDOW (SELECTED_FRAME ());
 
 
-  //TODO maybe we also need to special case sockets by picking up the plug rather than the socket
-  if(xw->widget_osr)
-    widget = xw->widget_osr;
-  else
-    widget = xwidget_view_lookup(xw, XWINDOW(window))->widget;
+  /* TODO maybe we also need to special case sockets by picking up the
+     plug rather than the socket.  */
+  GtkWidget *widget = (xw->widget_osr
+		       ? xw->widget_osr
+		       : xwidget_view_lookup (xw, XWINDOW (window))->widget);
 
-  ev->window = gtk_widget_get_window(widget);
-  gtk_widget_grab_focus(widget);
+  ev->window = gtk_widget_get_window (widget);
+  gtk_widget_grab_focus (widget);
   ev->send_event = FALSE;
 
   ev->hardware_keycode = keys[0].keycode;
@@ -1437,63 +1484,62 @@ DEFUN ("xwidget-send-keyboard-event", Fxwidget_send_keyboard_event, Sxwidget_sen
   ev->keyval = keyval;
   ev->time = GDK_CURRENT_TIME;
 
-  //ev->device = gdk_device_get_core_pointer();
-  manager = gdk_display_get_device_manager(gdk_window_get_display(ev->window));
-  gdk_event_set_device ((GdkEvent*)ev,   gdk_device_manager_get_client_pointer(manager));
-  gdk_event_put((GdkEvent*)ev);
-  //g_signal_emit_by_name(ev->window,"key-press-event", ev);
+  //ev->device = gdk_device_get_core_pointer ();
+  GdkDeviceManager *manager
+    = gdk_display_get_device_manager (gdk_window_get_display (ev->window));
+  gdk_event_set_device ((GdkEvent *) ev,
+			gdk_device_manager_get_client_pointer (manager));
+  gdk_event_put ((GdkEvent *) ev);
+  //g_signal_emit_by_name (ev->window,"key-press-event", ev);
 
   ev->type = GDK_KEY_RELEASE;
-  gdk_event_put((GdkEvent*)ev);
-  //g_signal_emit_by_name(ev->window,"key-release-event", ev);
-  //gtk_main_do_event(ev);
+  gdk_event_put ((GdkEvent *) ev);
+  //g_signal_emit_by_name (ev->window,"key-release-event", ev);
+  //gtk_main_do_event (ev);
 
   //TODO
   //if I delete the event the receiving component eventually crashes.
   //it ough TDTRT since event_put is supposed to copy the event
   //so probably this leaks events now
-  //gdk_event_free((GdkEvent*)ev);
+  //gdk_event_free ((GdkEvent *) ev);
 
   return Qnil;
 }
 
-DEFUN ("delete-xwidget-view", Fdelete_xwidget_view, Sdelete_xwidget_view,
-       1, 1, 0,
-       doc: /* Delete the XWIDGET-VIEW. */)
+DEFUN ("delete-xwidget-view", Fdelete_xwidget_view, Sdelete_xwidget_view, 1, 1, 0, doc:/* Delete the XWIDGET-VIEW. */
+       )
   (Lisp_Object xwidget_view)
 {
   CHECK_XWIDGET_VIEW (xwidget_view);
   struct xwidget_view *xv = XXWIDGET_VIEW (xwidget_view);
-  gtk_widget_destroy(xv->widgetwindow);
+  gtk_widget_destroy (xv->widgetwindow);
   Vxwidget_view_list = Fdelq (xwidget_view, Vxwidget_view_list);
 }
 
-DEFUN ("xwidget-view-lookup", Fxwidget_view_lookup, Sxwidget_view_lookup,
-       1, 2, 0,
-       doc: /* Return the xwidget-view associated to XWIDGET in
-WINDOW if specified, otherwise it uses the selected window. */)
+DEFUN ("xwidget-view-lookup", Fxwidget_view_lookup, Sxwidget_view_lookup, 1, 2, 0, doc:/* Return the xwidget-view associated to XWIDGET in
+											   WINDOW if specified, otherwise it uses the selected window. */
+)
   (Lisp_Object xwidget, Lisp_Object window)
 {
   CHECK_XWIDGET (xwidget);
 
   if (NILP (window))
-    window = Fselected_window();
+    window = Fselected_window ();
   CHECK_WINDOW (window);
 
   for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))
     {
       Lisp_Object xwidget_view = XCAR (tail);
       if (EQ (Fxwidget_view_model (xwidget_view), xwidget)
-          && EQ (Fxwidget_view_window (xwidget_view), window))
-        return xwidget_view;
+	  && EQ (Fxwidget_view_window (xwidget_view), window))
+	return xwidget_view;
     }
 
   return Qnil;
 }
 
-DEFUN ("set-frame-visible", Fset_frame_visible, Sset_frame_visible,
-       2, 2, 0,
-       doc: /* HACKY */)
+DEFUN ("set-frame-visible", Fset_frame_visible, Sset_frame_visible, 2, 2, 0, doc:	/* HACKY */
+       )
   (Lisp_Object frame, Lisp_Object flag)
 {
   CHECK_FRAME (frame);
@@ -1502,27 +1548,24 @@ DEFUN ("set-frame-visible", Fset_frame_visible, Sset_frame_visible,
   return flag;
 }
 
-DEFUN ("xwidget-plist", Fxwidget_plist, Sxwidget_plist,
-       1, 1, 0,
-       doc: /* Return the plist of XWIDGET.  */)
+DEFUN ("xwidget-plist", Fxwidget_plist, Sxwidget_plist, 1, 1, 0, doc:	/* Return the plist of XWIDGET.  */
+       )
   (register Lisp_Object xwidget)
 {
   CHECK_XWIDGET (xwidget);
   return XXWIDGET (xwidget)->plist;
 }
 
-DEFUN ("xwidget-buffer", Fxwidget_buffer, Sxwidget_buffer,
-       1, 1, 0,
-       doc: /* Return the buffer of XWIDGET.  */)
+DEFUN ("xwidget-buffer", Fxwidget_buffer, Sxwidget_buffer, 1, 1, 0, doc:/* Return the buffer of XWIDGET.  */
+       )
   (register Lisp_Object xwidget)
 {
   CHECK_XWIDGET (xwidget);
   return XXWIDGET (xwidget)->buffer;
 }
 
-DEFUN ("set-xwidget-plist", Fset_xwidget_plist, Sset_xwidget_plist,
-       2, 2, 0,
-       doc: /* Replace the plist of XWIDGET with PLIST.  Returns PLIST.  */)
+DEFUN ("set-xwidget-plist", Fset_xwidget_plist, Sset_xwidget_plist, 2, 2, 0, doc:	/* Replace the plist of XWIDGET with PLIST.  Returns PLIST.  */
+       )
   (register Lisp_Object xwidget, Lisp_Object plist)
 {
   CHECK_XWIDGET (xwidget);
@@ -1532,13 +1575,12 @@ DEFUN ("set-xwidget-plist", Fset_xwidget_plist, Sset_xwidget_plist,
   return plist;
 }
 
-DEFUN ("set-xwidget-query-on-exit-flag",
-       Fset_xwidget_query_on_exit_flag, Sset_xwidget_query_on_exit_flag,
-       2, 2, 0,
-       doc: /* Specify if query is needed for XWIDGET when Emacs is
-exited.  If the second argument FLAG is non-nil, Emacs will query the
-user before exiting or killing a buffer if XWIDGET is running.  This
-function returns FLAG. */)
+DEFUN ("set-xwidget-query-on-exit-flag", Fset_xwidget_query_on_exit_flag, Sset_xwidget_query_on_exit_flag, 2, 2, 0, doc:
+															/* Specify if query is needed for XWIDGET when Emacs is
+															   exited.  If the second argument FLAG is non-nil, Emacs will query the
+															   user before exiting or killing a buffer if XWIDGET is running.  This
+															   function returns FLAG. */
+)
   (Lisp_Object xwidget, Lisp_Object flag)
 {
   CHECK_XWIDGET (xwidget);
@@ -1546,10 +1588,8 @@ function returns FLAG. */)
   return flag;
 }
 
-DEFUN ("xwidget-query-on-exit-flag",
-       Fxwidget_query_on_exit_flag, Sxwidget_query_on_exit_flag,
-       1, 1, 0,
-       doc: /* Return the current value of query-on-exit flag for XWIDGET. */)
+DEFUN ("xwidget-query-on-exit-flag", Fxwidget_query_on_exit_flag, Sxwidget_query_on_exit_flag, 1, 1, 0, doc:	/* Return the current value of query-on-exit flag for XWIDGET. */
+       )
   (Lisp_Object xwidget)
 {
   CHECK_XWIDGET (xwidget);
@@ -1577,16 +1617,16 @@ syms_of_xwidget (void)
   defsubr (&Sset_xwidget_query_on_exit_flag);
   defsubr (&Sset_frame_visible);
 
-  #ifdef HAVE_WEBKIT_OSR
+#ifdef HAVE_WEBKIT_OSR
   defsubr (&Sxwidget_webkit_goto_uri);
   defsubr (&Sxwidget_webkit_execute_script);
   defsubr (&Sxwidget_webkit_get_title);
   DEFSYM (Qwebkit_osr, "webkit-osr");
-  #endif
+#endif
 
-  defsubr (&Sxwgir_xwidget_call_method  );
+  defsubr (&Sxwgir_xwidget_call_method);
   defsubr (&Sxwgir_require_namespace);
-  defsubr (&Sxwidget_size_request  );
+  defsubr (&Sxwidget_size_request);
   defsubr (&Sdelete_xwidget_view);
   defsubr (&Sxwidget_disable_plugin_for_mime);
 
@@ -1606,7 +1646,10 @@ syms_of_xwidget (void)
 
   /* Do not forget to update the docstring of make-xwidget if you add
      new types. */
-  DEFSYM (Qbutton, "Button"); //changed to match the gtk class because xwgir(experimental and not really needed)
+
+  /* Changed to match the gtk class because xwgir (experimental and not
+     really needed).  */
+  DEFSYM (Qbutton, "Button");
   DEFSYM (Qtoggle, "ToggleButton");
   DEFSYM (Qslider, "slider");
   DEFSYM (Qsocket, "socket");
@@ -1618,10 +1661,11 @@ syms_of_xwidget (void)
 
   DEFSYM (QCplist, ":plist");
 
-  DEFVAR_LISP ("xwidget-list", Vxwidget_list, doc: /*xwidgets list*/);
+  DEFVAR_LISP ("xwidget-list", Vxwidget_list, doc: /*xwidgets list */);
   Vxwidget_list = Qnil;
 
-  DEFVAR_LISP ("xwidget-view-list", Vxwidget_view_list, doc: /*xwidget views list*/);
+  DEFVAR_LISP ("xwidget-view-list", Vxwidget_view_list,
+	       doc: /* xwidget views list */);
   Vxwidget_view_list = Qnil;
 
   Fprovide (intern ("xwidget-internal"), Qnil);
@@ -1639,53 +1683,25 @@ syms_of_xwidget (void)
 int
 valid_xwidget_spec_p (Lisp_Object object)
 {
-  int valid_p = 0;
-
-  if (CONSP (object) && EQ (XCAR (object), Qxwidget))
-    {
-      /* Lisp_Object tem; */
-
-      /* for (tem = XCDR (object); CONSP (tem); tem = XCDR (tem)) */
-      /*   if (EQ (XCAR (tem), QCtype)) */
-      /*     { */
-      /*       tem = XCDR (tem); */
-      /*       if (CONSP (tem) && SYMBOLP (XCAR (tem))) */
-      /*         { */
-      /*        struct xwidget_type *type; */
-      /*        type = lookup_xwidget_type (XCAR (tem)); */
-      /*        if (type) */
-      /*          valid_p = type->valid_p (object); */
-      /*         } */
-
-      /*       break; */
-      /*     } */
-      //never mind type support for now
-      valid_p = 1;
-    }
-
-  return valid_p;
+  return CONSP (object) && EQ (XCAR (object), Qxwidget);
+  // Never mind type support for now.
 }
 
-
-
-/* find a value associated with key in spec */
-Lisp_Object
-xwidget_spec_value ( Lisp_Object spec, Lisp_Object  key,
-                     int *found)
+/* In SPEC, find a value associated with KEY.  */
+static Lisp_Object
+xwidget_spec_value (Lisp_Object spec, Lisp_Object key, int *found)
 {
-  Lisp_Object tail;
-
   eassert (valid_xwidget_spec_p (spec));
 
-  for (tail = XCDR (spec);
+  for (Lisp_Object tail = XCDR (spec);
        CONSP (tail) && CONSP (XCDR (tail)); tail = XCDR (XCDR (tail)))
     {
       if (EQ (XCAR (tail), key))
-        {
-          if (found)
-            *found = 1;
-          return XCAR (XCDR (tail));
-        }
+	{
+	  if (found)
+	    *found = 1;
+	  return XCAR (XCDR (tail));
+	}
     }
 
   if (found)
@@ -1693,25 +1709,26 @@ xwidget_spec_value ( Lisp_Object spec, Lisp_Object  key,
   return Qnil;
 }
 
-
 void
 xwidget_view_delete_all_in_window (struct window *w)
 {
-  struct xwidget_view* xv = NULL;
+  struct xwidget_view *xv = NULL;
   for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))
     {
-      if (XWIDGET_VIEW_P (XCAR (tail))) {
-        xv = XXWIDGET_VIEW (XCAR (tail));
-        if(XWINDOW (xv->w) == w) {
-          gtk_widget_destroy(xv->widgetwindow);
-          Vxwidget_view_list = Fdelq (XCAR (tail), Vxwidget_view_list);
-        }
-      }
+      if (XWIDGET_VIEW_P (XCAR (tail)))
+	{
+	  xv = XXWIDGET_VIEW (XCAR (tail));
+	  if (XWINDOW (xv->w) == w)
+	    {
+	      gtk_widget_destroy (xv->widgetwindow);
+	      Vxwidget_view_list = Fdelq (XCAR (tail), Vxwidget_view_list);
+	    }
+	}
     }
 }
 
-struct xwidget_view*
-xwidget_view_lookup (struct xwidget* xw, struct window *w)
+static struct xwidget_view *
+xwidget_view_lookup (struct xwidget *xw, struct window *w)
 {
   Lisp_Object xwidget, window, ret;
   XSETXWIDGET (xwidget, xw);
@@ -1722,38 +1739,39 @@ xwidget_view_lookup (struct xwidget* xw, struct window *w)
   return EQ (ret, Qnil) ? NULL : XXWIDGET_VIEW (ret);
 }
 
-struct xwidget*
-lookup_xwidget (Lisp_Object  spec)
+struct xwidget *
+lookup_xwidget (Lisp_Object spec)
 {
-  /* When a xwidget lisp spec is found initialize the C struct that is used in the C code.
-     This is done by redisplay so values change if the spec changes.
-     So, take special care of one-shot events
+  /* When a xwidget lisp spec is found initialize the C struct that is
+     used in the C code.  This is done by redisplay so values change
+     if the spec changes.  So, take special care of one-shot events.
 
-     TODO remove xwidget init from display spec. simply store an xwidget reference only and set
-     size etc when creating the xwidget, which should happen before insertion into buffer
-  */
+     TODO Remove xwidget init from display spec.  Simply store an
+     xwidget reference only and set size etc when creating the
+     xwidget, which should happen before insertion into buffer.  */
   int found = 0, found1 = 0, found2 = 0;
   Lisp_Object value;
   struct xwidget *xw;
 
   value = xwidget_spec_value (spec, QCxwidget, &found1);
-  xw = XXWIDGET(value);
+  xw = XXWIDGET (value);
 
   return xw;
 }
 
-/*set up detection of touched xwidget*/
+/* Set up detection of touched xwidget.  */
 void
 xwidget_start_redisplay (void)
 {
   for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))
     {
       if (XWIDGET_VIEW_P (XCAR (tail)))
-        XXWIDGET_VIEW (XCAR (tail))->redisplayed = 0;
+	XXWIDGET_VIEW (XCAR (tail))->redisplayed = 0;
     }
 }
 
-/* the xwidget was touched during redisplay, so it isnt a candidate for hiding*/
+/* The xwidget was touched during redisplay, so it isn't a candidate
+   for hiding.  */
 void
 xwidget_touch (struct xwidget_view *xv)
 {
@@ -1763,11 +1781,10 @@ xwidget_touch (struct xwidget_view *xv)
 int
 xwidget_touched (struct xwidget_view *xv)
 {
-  return  xv->redisplayed;
+  return xv->redisplayed;
 }
 
-/* redisplay has ended, now we should hide untouched xwidgets
-*/
+/* Redisplay has ended, now we should hide untouched xwidgets.  */
 void
 xwidget_end_redisplay (struct window *w, struct glyph_matrix *matrix)
 {
@@ -1776,7 +1793,6 @@ xwidget_end_redisplay (struct window *w, struct glyph_matrix *matrix)
   struct xwidget *xw;
   int area;
 
-
   xwidget_start_redisplay ();
   //iterate desired glyph matrix of window here, hide gtk widgets
   //not in the desired matrix.
@@ -1784,47 +1800,42 @@ xwidget_end_redisplay (struct window *w, struct glyph_matrix *matrix)
   //this only takes care of xwidgets in active windows.
   //if a window goes away from screen xwidget views wust be deleted
 
-  //  dump_glyph_matrix(matrix, 2);
+  //  dump_glyph_matrix (matrix, 2);
   for (i = 0; i < matrix->nrows; ++i)
     {
-      //    dump_glyph_row (MATRIX_ROW (matrix, i), i, glyphs);
+      // dump_glyph_row (MATRIX_ROW (matrix, i), i, glyphs);
       struct glyph_row *row;
       row = MATRIX_ROW (matrix, i);
       if (row->enabled_p != 0)
-        {
-          for (area = LEFT_MARGIN_AREA; area < LAST_AREA; ++area)
-            {
-              struct glyph *glyph = row->glyphs[area];
-              struct glyph *glyph_end = glyph + row->used[area];
-              for (; glyph < glyph_end; ++glyph)
-                {
-                  if (glyph->type == XWIDGET_GLYPH)
-                    {
-                      /*
-                        the only call to xwidget_end_redisplay is in dispnew
-                        xwidget_end_redisplay(w->current_matrix);
-                      */
-                      xwidget_touch (xwidget_view_lookup(glyph->u.xwidget,
-                                                         w));
-                    }
-                }
-            }
-        }
+	{
+	  for (area = LEFT_MARGIN_AREA; area < LAST_AREA; ++area)
+	    {
+	      struct glyph *glyph = row->glyphs[area];
+	      struct glyph *glyph_end = glyph + row->used[area];
+	      /* The only call to xwidget_end_redisplay is in dispnew.  */
+	      for (; glyph < glyph_end; ++glyph)
+		if (glyph->type == XWIDGET_GLYPH)
+		  xwidget_touch (xwidget_view_lookup (glyph->u.xwidget, w));
+	    }
+	}
     }
 
   for (Lisp_Object tail = Vxwidget_view_list; CONSP (tail); tail = XCDR (tail))
     {
-      if (XWIDGET_VIEW_P (XCAR (tail))) {
-        struct xwidget_view* xv = XXWIDGET_VIEW (XCAR (tail));
-
-        //"touched" is only meaningful for the current window, so disregard other views
-        if (XWINDOW (xv->w) == w) {
-          if (xwidget_touched(xv))
-            xwidget_show_view (xv);
-          else
-            xwidget_hide_view (xv);
-        }
-      }
+      if (XWIDGET_VIEW_P (XCAR (tail)))
+	{
+	  struct xwidget_view *xv = XXWIDGET_VIEW (XCAR (tail));
+
+	  /* "touched" is only meaningful for the current window, so
+	     disregard other views.  */
+	  if (XWINDOW (xv->w) == w)
+	    {
+	      if (xwidget_touched (xv))
+		xwidget_show_view (xv);
+	      else
+		xwidget_hide_view (xv);
+	    }
+	}
     }
 }
 
@@ -1839,15 +1850,15 @@ kill_buffer_xwidgets (Lisp_Object buffer)
       Vxwidget_list = Fdelq (xwidget, Vxwidget_list);
       /* TODO free the GTK things in xw */
       {
-        CHECK_XWIDGET (xwidget);
-        struct xwidget *xw = XXWIDGET (xwidget);
-        if (xw->widget_osr && xw->widgetwindow_osr)
-          {
-            gtk_widget_destroy(xw->widget_osr);
-            gtk_widget_destroy(xw->widgetwindow_osr);
-          }
+	CHECK_XWIDGET (xwidget);
+	struct xwidget *xw = XXWIDGET (xwidget);
+	if (xw->widget_osr && xw->widgetwindow_osr)
+	  {
+	    gtk_widget_destroy (xw->widget_osr);
+	    gtk_widget_destroy (xw->widgetwindow_osr);
+	  }
       }
     }
 }
 
-#endif  /* HAVE_XWIDGETS */
+#endif /* HAVE_XWIDGETS */
diff --git a/src/xwidget.h b/src/xwidget.h
index cbaddf6..e52b448 100644
--- a/src/xwidget.h
+++ b/src/xwidget.h
@@ -1,115 +1,120 @@
-#ifndef XWIDGET_H_INCLUDED
-#define XWIDGET_H_INCLUDED
+/* Embed GTK widgets inside Emacs buffers.
 
-void x_draw_xwidget_glyph_string (struct glyph_string *s);
-void syms_of_xwidget ();
+Copyright 2015 Free Software Foundation, Inc.
 
-//extern Lisp_Object Qxwidget;
+This file is part of GNU Emacs.
 
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
 
-int valid_xwidget_spec_p (Lisp_Object object) ;
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
 
-#include <gtk/gtk.h>
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef XWIDGET_H_INCLUDED
+#define XWIDGET_H_INCLUDED
 
+void x_draw_xwidget_glyph_string (struct glyph_string *);
+void syms_of_xwidget (void);
 
-/*
-each xwidget instance/model is described by this struct.
+int valid_xwidget_spec_p (Lisp_Object);
 
-lisp pseudovector.
+#include <gtk/gtk.h>
 
+/* Each xwidget instance/model is described by this struct.  */
 
- */
-struct xwidget{
+struct xwidget
+{
   struct vectorlike_header header;
-  Lisp_Object plist;//auxilliary data
-  Lisp_Object type;//the widget type
-  Lisp_Object buffer; //buffer where xwidget lives
-  Lisp_Object title;//a title that is used for button labels for instance
+  Lisp_Object plist;	// Auxiliary data.
+  Lisp_Object type;	// The widget type.
+  Lisp_Object buffer;	// Buffer where xwidget lives.
+  Lisp_Object title;	// A title used for button labels for instance.
 
-  //here ends the lisp part.
-  //"height" is the marker field
+  // Here ends the Lisp part.
+  // "height" is the marker field.
   int height;
   int width;
 
-  //for offscreen widgets, unused if not osr
-  GtkWidget* widget_osr;
-  GtkWidget* widgetwindow_osr;
-  //this is used if the widget (webkit) is to be wrapped in a scrolled window,
-  GtkWidget* widgetscrolledwindow_osr;
-  /* Non-nil means kill silently if Emacs is exited. */
+  // For offscreen widgets, unused if not osr.
+  GtkWidget *widget_osr;
+  GtkWidget *widgetwindow_osr;
+  // This is used if the widget (webkit) is to be wrapped in a scrolled window,
+  GtkWidget *widgetscrolledwindow_osr;
+  // Non-nil means kill silently if Emacs is exited.
   unsigned int kill_without_query : 1;
 
 };
 
+// struct for each xwidget view.
 
-//struct for each xwidget view
-struct xwidget_view {
+struct xwidget_view
+{
   struct vectorlike_header header;
   Lisp_Object model;
   Lisp_Object w;
 
-  //here ends the lisp part.
-  //"redisplayed" is the marker field
-  int redisplayed; //if touched by redisplay
+  // Here ends the lisp part.
+  // "redisplayed" is the marker field.
+  int redisplayed; // If touched by redisplay.
 
-  int hidden;//if the "live" instance isnt drawn
+  int hidden; // If the "live" instance isn't drawn.
 
-  GtkWidget* widget;
-  GtkWidget* widgetwindow;
-  GtkWidget* emacswindow;
+  GtkWidget *widget;
+  GtkWidget *widgetwindow;
+  GtkWidget *emacswindow;
   int x; int y;
   int clip_right; int clip_bottom; int clip_top; int clip_left;
-
-
   long handler_id;
 };
 
 /* Test for xwidget pseudovector*/
 #define XWIDGETP(x) PSEUDOVECTORP (x, PVEC_XWIDGET)
-#define XXWIDGET(a) (eassert (XWIDGETP(a)), \
-                     (struct xwidget *) XUNTAG(a, Lisp_Vectorlike))
+#define XXWIDGET(a) (eassert (XWIDGETP (a)), \
+                     (struct xwidget *) XUNTAG (a, Lisp_Vectorlike))
 
 #define CHECK_XWIDGET(x) \
   CHECK_TYPE (XWIDGETP (x), Qxwidgetp, x)
 
 /* Test for xwidget_view pseudovector */
 #define XWIDGET_VIEW_P(x) PSEUDOVECTORP (x, PVEC_XWIDGET_VIEW)
-#define XXWIDGET_VIEW(a) (eassert (XWIDGET_VIEW_P(a)), \
-                          (struct xwidget_view *) XUNTAG(a, Lisp_Vectorlike))
+#define XXWIDGET_VIEW(a) (eassert (XWIDGET_VIEW_P (a)), \
+                          (struct xwidget_view *) XUNTAG (a, Lisp_Vectorlike))
 
 #define CHECK_XWIDGET_VIEW(x) \
   CHECK_TYPE (XWIDGET_VIEW_P (x), Qxwidget_view_p, x)
 
 struct xwidget_type
 {
-  /* A symbol uniquely identifying the xwidget type, */
+  /* A symbol uniquely identifying the xwidget type.  */
   Lisp_Object *type;
 
   /* Check that SPEC is a valid image specification for the given
      image type.  Value is non-zero if SPEC is valid.  */
-  int (* valid_p) (Lisp_Object spec);
+  int (*valid_p) (Lisp_Object spec);
 
   /* Next in list of all supported image types.  */
   struct xwidget_type *next;
 };
 
-static struct xwidget_type *lookup_xwidget_type (Lisp_Object symbol);
-
-struct xwidget* xwidget_from_id(int id);
+struct xwidget *xwidget_from_id (int);
 
-//extern int xwidget_owns_kbd;
+void xwidget_start_redisplay (void);
+void xwidget_end_redisplay (struct window *, struct glyph_matrix *);
 
-void xwidget_start_redisplay();
-void xwidget_end_redisplay (struct window *w, struct glyph_matrix *matrix);
+void xwidget_touch (struct xwidget_view *);
 
-void xwidget_touch (struct xwidget_view *xw);
-
-//void assert_valid_xwidget_id(int id,char *str);
-
-struct xwidget* lookup_xwidget (Lisp_Object  spec);
+struct xwidget *lookup_xwidget (Lisp_Object);
 #define XG_XWIDGET "emacs_xwidget"
 #define XG_XWIDGET_VIEW "emacs_xwidget_view"
-void      xwidget_view_delete_all_in_window(  struct window *w );
+void xwidget_view_delete_all_in_window (struct window *);
+
+void kill_buffer_xwidgets (Lisp_Object);
 
-void kill_buffer_xwidgets (Lisp_Object buffer);
-#endif  /* XWIDGET_H_INCLUDED */
+#endif /* XWIDGET_H_INCLUDED */
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01  6:11   ` [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets Dmitry Gutov
  2015-02-01  8:50     ` joakim
@ 2015-02-01 15:36     ` Eli Zaretskii
  2015-02-01 15:51       ` David Engster
  2015-02-01 15:52       ` joakim
  1 sibling, 2 replies; 50+ messages in thread
From: Eli Zaretskii @ 2015-02-01 15:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: joakim, emacs-devel

> Date: Sun, 01 Feb 2015 08:11:24 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> Given the above, maybe you should revert the merge. Then squash all 
> commits in xwidget into one patch, amend it with proper ChangeLog 
> entries, and then do the merge.

Is it just me, or is someone else bothered by the fact that all the
first parents now follow Joakim's commits on his feature branch?

Joakim, did you per chance push to master from your branch?



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 10:53       ` Paul Eggert
@ 2015-02-01 15:46         ` joakim
  2015-02-01 16:09           ` Dmitry Gutov
  2015-02-03 22:38         ` joakim
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-01 15:46 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel, Dmitry Gutov

Paul Eggert <eggert@cs.ucla.edu> writes:

> joakim@verona.se wrote:
>>> Given the above, maybe you should revert the merge. Then squash all
>>> >commits in xwidget into one patch, amend it with proper ChangeLog
>>> >entries, and then do the merge.
>>> >
>> Perhaps that would be best.
>
> Yes, please do that.  Here are some other things to do before committing the merge.

Ok, so I don't botch it up worse, how would I do the revert exactly?

Then, how do I squash the commits properly?

For instance, should all commits to a certain directory be grouped, so a
changelog can be part of the commit message for that directory?

Is it not possible to edit the changelog to fix it?

Also parts of the patch was written by Gregoire, can I have 2 changelogs
in a commit message?

>
> * Reindent as per GNU standards.  Start with "indent -gnu" but it
> won't do a perfect job.  E.g., say "char *p" not "char* p".
>
> * Fit it into 80 columns.
>
> * Use GNU style for comments.  These should typically use complete,
> imperative sentences.
>
> * Configure with "./configure --enable-gcc-warnings --with-xwidgets
> --with-x-toolkit=gtk3" and fix all the warnings.
>
> * It's OK to assume C99 now.
>
> * Don't make functions extern unless they need to be extern.
> Compilers do a better job with static functions, typically.
>
> * Some of those function names are too long; please shorten them.
>
> * A lot of the printf statements look like they shouldn't be there.
>
> * There's some commented-out code that should be removed.
>
> * Omit pointer casts that aren't needed (when casting to and from void *).
>
> I started to look into all that and came up with the attached patch,
> relative to commit 9fe732a02afbe0b3d4a85d2bcae687900ab881f7; please
> have a look.  But the result still doesn't compile due to warnings and
> I'm sure I missed a lot of things.  I hope you can finish the job.
> (Also, the ChangeLog entries need to be written -- I started on that
> but it's a big job and it's something the author of the patch really
> should do.)
>
>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:36     ` Eli Zaretskii
@ 2015-02-01 15:51       ` David Engster
  2015-02-01 15:52       ` joakim
  1 sibling, 0 replies; 50+ messages in thread
From: David Engster @ 2015-02-01 15:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, joakim, Dmitry Gutov

Eli Zaretskii writes:
>> Date: Sun, 01 Feb 2015 08:11:24 +0200
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> 
>> Given the above, maybe you should revert the merge. Then squash all 
>> commits in xwidget into one patch, amend it with proper ChangeLog 
>> entries, and then do the merge.
>
> Is it just me, or is someone else bothered by the fact that all the
> first parents now follow Joakim's commits on his feature branch?

It's a bit weird, yes. Especially when you do 'gitk --all', all those
'merge from trunk' commits are now somehow registered as a separate
branch, leading to a completely crazy layout of the DAG which doesn't
even fit on my screen anymore. I've never seen anything like it. Can
someone explain what happened here?

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:36     ` Eli Zaretskii
  2015-02-01 15:51       ` David Engster
@ 2015-02-01 15:52       ` joakim
  2015-02-01 16:04         ` David Engster
                           ` (2 more replies)
  1 sibling, 3 replies; 50+ messages in thread
From: joakim @ 2015-02-01 15:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, Dmitry Gutov

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 01 Feb 2015 08:11:24 +0200
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> 
>> Given the above, maybe you should revert the merge. Then squash all 
>> commits in xwidget into one patch, amend it with proper ChangeLog 
>> entries, and then do the merge.
>
> Is it just me, or is someone else bothered by the fact that all the
> first parents now follow Joakim's commits on his feature branch?
>
> Joakim, did you per chance push to master from your branch?

The thing I thought I did was merging from the feature branch to master,
and then push master.

How do I dig myself out of this hole now then?

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:52       ` joakim
@ 2015-02-01 16:04         ` David Engster
  2015-02-01 16:04         ` Eli Zaretskii
  2015-02-01 16:05         ` Andreas Schwab
  2 siblings, 0 replies; 50+ messages in thread
From: David Engster @ 2015-02-01 16:04 UTC (permalink / raw)
  To: joakim; +Cc: Eli Zaretskii, Dmitry Gutov, emacs-devel

'joakim' writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Date: Sun, 01 Feb 2015 08:11:24 +0200
>>> From: Dmitry Gutov <dgutov@yandex.ru>
>>> 
>>> Given the above, maybe you should revert the merge. Then squash all 
>>> commits in xwidget into one patch, amend it with proper ChangeLog 
>>> entries, and then do the merge.
>>
>> Is it just me, or is someone else bothered by the fact that all the
>> first parents now follow Joakim's commits on his feature branch?
>>
>> Joakim, did you per chance push to master from your branch?
>
> The thing I thought I did was merging from the feature branch to master,
> and then push master.

To me, it looks like you merged master into your feature branch, and
then pushed it as master. That does not mean that you actually did this,
though. My best guess is that you did not merge to master using
'--no-ff', which resulted in a fast-forward merge without an explicit
merge commit. That's one of those Git "features", you know...

> How do I dig myself out of this hole now then?

You don't. What's pushed is pushed. :-)

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:52       ` joakim
  2015-02-01 16:04         ` David Engster
@ 2015-02-01 16:04         ` Eli Zaretskii
  2015-02-01 16:05         ` Andreas Schwab
  2 siblings, 0 replies; 50+ messages in thread
From: Eli Zaretskii @ 2015-02-01 16:04 UTC (permalink / raw)
  To: joakim; +Cc: dgutov, emacs-devel

> From: joakim@verona.se
> Date: Sun, 01 Feb 2015 16:52:33 +0100
> Cc: emacs-devel@gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> 
> The thing I thought I did was merging from the feature branch to master,
> and then push master.

Then maybe some Git guru could explain how this happened.

> How do I dig myself out of this hole now then?

I'll defer to the Git experts.  (There's a technique to swap parents
of a merge-commit, but I'm not sure it is needed in this case,
especially if you are going to revert.)



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:52       ` joakim
  2015-02-01 16:04         ` David Engster
  2015-02-01 16:04         ` Eli Zaretskii
@ 2015-02-01 16:05         ` Andreas Schwab
  2015-02-01 16:11           ` David Engster
  2 siblings, 1 reply; 50+ messages in thread
From: Andreas Schwab @ 2015-02-01 16:05 UTC (permalink / raw)
  To: joakim; +Cc: Eli Zaretskii, Dmitry Gutov, emacs-devel

joakim@verona.se writes:

> The thing I thought I did was merging from the feature branch to master,
> and then push master.

No, you did exactly the other way round:

commit 69815df
Merge: 4edad42 a2c32b0
Author: Joakim Verona <joakim@verona.se>
Date:   Sun Feb 1 00:37:46 2015 +0100

    Merge branch 'master' into xwidget

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 15:46         ` joakim
@ 2015-02-01 16:09           ` Dmitry Gutov
  2015-02-01 16:17             ` joakim
  2015-02-01 20:08             ` David Engster
  0 siblings, 2 replies; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 16:09 UTC (permalink / raw)
  To: joakim, Paul Eggert; +Cc: emacs-devel

On 02/01/2015 05:46 PM, joakim@verona.se wrote:

> Ok, so I don't botch it up worse, how would I do the revert exactly?

I'd do a history rewrite (check out the last master before the merge), 
then switch to xwidget, rebase it on top of master, then switch back to 
master and merge xwidget into it).

> Then, how do I squash the commits properly?

Just do 'git merge --squash xwidget' in the end. You can amend the 
resulting commit with ChangeLogs after, since it'll be just one commit.

And then do 'git push -f' (!!!). Someone else should make a ruling on 
this, though, but if we're going to transition on auto-generated 
ChangeLogs, we probably don't want to have ugly history.

Note that you shouldn't do 'git push -f' yourself anyway (and you won't 
be able to), so just create a rebased, ChangeLog-ged commit in xwidget 
branching from a2c32b0.

> For instance, should all commits to a certain directory be grouped, so a
> changelog can be part of the commit message for that directory?

As far as I'm concerned, just do one commit.

> Is it not possible to edit the changelog to fix it?

At smaller scales, it's possible, but you've got a truckload of commits 
there. If it's not too late, we should try to fix this properly.

> Also parts of the patch was written by Gregoire, can I have 2 changelogs
> in a commit message?

Make it two commits, then, if it's not too much trouble.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:05         ` Andreas Schwab
@ 2015-02-01 16:11           ` David Engster
  2015-02-01 16:15             ` Andreas Schwab
  0 siblings, 1 reply; 50+ messages in thread
From: David Engster @ 2015-02-01 16:11 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Eli Zaretskii, emacs-devel, joakim, Dmitry Gutov

Andreas Schwab writes:
> joakim@verona.se writes:
>
>> The thing I thought I did was merging from the feature branch to master,
>> and then push master.
>
> No, you did exactly the other way round:
>
> commit 69815df
> Merge: 4edad42 a2c32b0
> Author: Joakim Verona <joakim@verona.se>
> Date:   Sun Feb 1 00:37:46 2015 +0100
>
>     Merge branch 'master' into xwidget

Yes, but I'm willing to bet he actually *did* switch to 'master'
afterwards and did

  git merge xwidget

which was then however resolved as 'fast-forward', so you don't see it.

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:11           ` David Engster
@ 2015-02-01 16:15             ` Andreas Schwab
  2015-02-01 16:18               ` joakim
  2015-02-01 19:29               ` David Engster
  0 siblings, 2 replies; 50+ messages in thread
From: Andreas Schwab @ 2015-02-01 16:15 UTC (permalink / raw)
  To: David Engster; +Cc: Eli Zaretskii, emacs-devel, joakim, Dmitry Gutov

David Engster <deng@randomsample.de> writes:

> Andreas Schwab writes:
>> joakim@verona.se writes:
>>
>>> The thing I thought I did was merging from the feature branch to master,
>>> and then push master.
>>
>> No, you did exactly the other way round:
>>
>> commit 69815df
>> Merge: 4edad42 a2c32b0
>> Author: Joakim Verona <joakim@verona.se>
>> Date:   Sun Feb 1 00:37:46 2015 +0100
>>
>>     Merge branch 'master' into xwidget
>
> Yes, but I'm willing to bet he actually *did* switch to 'master'
> afterwards and did
>
>   git merge xwidget
>
> which was then however resolved as 'fast-forward', so you don't see it.

But only because he merged from master, which was wrong to begin with.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:09           ` Dmitry Gutov
@ 2015-02-01 16:17             ` joakim
  2015-02-01 16:30               ` Dmitry Gutov
  2015-02-01 20:08             ` David Engster
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-01 16:17 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Paul Eggert, emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 02/01/2015 05:46 PM, joakim@verona.se wrote:
>
>> Ok, so I don't botch it up worse, how would I do the revert exactly?
>
> I'd do a history rewrite (check out the last master before the merge),
> then switch to xwidget, rebase it on top of master, then switch back
> to master and merge xwidget into it).

But wouldnt the xwidget branch still be in master then?

At this point I would just like to revert all the changes in master.

>
>> Then, how do I squash the commits properly?
>
> Just do 'git merge --squash xwidget' in the end. You can amend the
> resulting commit with ChangeLogs after, since it'll be just one
> commit.
>
> And then do 'git push -f' (!!!). Someone else should make a ruling on
> this, though, but if we're going to transition on auto-generated
> ChangeLogs, we probably don't want to have ugly history.
>
> Note that you shouldn't do 'git push -f' yourself anyway (and you
> won't be able to), so just create a rebased, ChangeLog-ged commit in
> xwidget branching from a2c32b0.
>
>> For instance, should all commits to a certain directory be grouped, so a
>> changelog can be part of the commit message for that directory?
>
> As far as I'm concerned, just do one commit.
>
>> Is it not possible to edit the changelog to fix it?
>
> At smaller scales, it's possible, but you've got a truckload of
> commits there. If it's not too late, we should try to fix this
> properly.
>
>> Also parts of the patch was written by Gregoire, can I have 2 changelogs
>> in a commit message?
>
> Make it two commits, then, if it's not too much trouble.
>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:15             ` Andreas Schwab
@ 2015-02-01 16:18               ` joakim
  2015-02-01 19:29               ` David Engster
  1 sibling, 0 replies; 50+ messages in thread
From: joakim @ 2015-02-01 16:18 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Eli Zaretskii, emacs-devel, David Engster, Dmitry Gutov

Andreas Schwab <schwab@linux-m68k.org> writes:

> David Engster <deng@randomsample.de> writes:
>
>> Andreas Schwab writes:
>>> joakim@verona.se writes:
>>>
>>>> The thing I thought I did was merging from the feature branch to master,
>>>> and then push master.
>>>
>>> No, you did exactly the other way round:
>>>
>>> commit 69815df
>>> Merge: 4edad42 a2c32b0
>>> Author: Joakim Verona <joakim@verona.se>
>>> Date:   Sun Feb 1 00:37:46 2015 +0100
>>>
>>>     Merge branch 'master' into xwidget
>>
>> Yes, but I'm willing to bet he actually *did* switch to 'master'
>> afterwards and did
>>
>>   git merge xwidget
>>
>> which was then however resolved as 'fast-forward', so you don't see it.
>
> But only because he merged from master, which was wrong to begin with.

Yes this was probably what I did.

> Andreas.

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:17             ` joakim
@ 2015-02-01 16:30               ` Dmitry Gutov
  2015-02-01 19:48                 ` joakim
  2015-02-01 19:53                 ` Paul Eggert
  0 siblings, 2 replies; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 16:30 UTC (permalink / raw)
  To: joakim; +Cc: Paul Eggert, emacs-devel

On 02/01/2015 06:17 PM, joakim@verona.se wrote:
> But wouldnt the xwidget branch still be in master then?

Note the "check out the last master before the merge" step. That would 
be commit a2c32b0.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:15             ` Andreas Schwab
  2015-02-01 16:18               ` joakim
@ 2015-02-01 19:29               ` David Engster
  2015-02-01 19:39                 ` Dmitry Gutov
  2015-02-02  1:35                 ` Stephen J. Turnbull
  1 sibling, 2 replies; 50+ messages in thread
From: David Engster @ 2015-02-01 19:29 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Eli Zaretskii, Dmitry Gutov, joakim, emacs-devel

Andreas Schwab writes:
> David Engster <deng@randomsample.de> writes:
>> Andreas Schwab writes:
>>> joakim@verona.se writes:
>>>
>>>> The thing I thought I did was merging from the feature branch to master,
>>>> and then push master.
>>>
>>> No, you did exactly the other way round:
>>>
>>> commit 69815df
>>> Merge: 4edad42 a2c32b0
>>> Author: Joakim Verona <joakim@verona.se>
>>> Date:   Sun Feb 1 00:37:46 2015 +0100
>>>
>>>     Merge branch 'master' into xwidget
>>
>> Yes, but I'm willing to bet he actually *did* switch to 'master'
>> afterwards and did
>>
>>   git merge xwidget
>>
>> which was then however resolved as 'fast-forward', so you don't see it.
>
> But only because he merged from master, which was wrong to begin with.

It's a very easy mistake to make, and I'd rather say it is wrong that
Git defaults to fast-forward merges.

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:29               ` David Engster
@ 2015-02-01 19:39                 ` Dmitry Gutov
  2015-02-01 19:56                   ` Eli Zaretskii
  2015-02-02  1:35                 ` Stephen J. Turnbull
  1 sibling, 1 reply; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 19:39 UTC (permalink / raw)
  To: David Engster, Andreas Schwab; +Cc: Eli Zaretskii, joakim, emacs-devel

On 02/01/2015 09:29 PM, David Engster wrote:

> It's a very easy mistake to make, and I'd rather say it is wrong that
> Git defaults to fast-forward merges.

Maybe admin/notes/git-workflow should have a section about feature 
branches, and prescribe 'git merge --no-ff' when merging a long-running 
feature branch.

However I wouldn't say that the forest of merges if the biggest problem 
we have. At least a sufficiently smart script can filter out those.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:30               ` Dmitry Gutov
@ 2015-02-01 19:48                 ` joakim
  2015-02-01 19:53                 ` Paul Eggert
  1 sibling, 0 replies; 50+ messages in thread
From: joakim @ 2015-02-01 19:48 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Paul Eggert, emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 02/01/2015 06:17 PM, joakim@verona.se wrote:
>> But wouldnt the xwidget branch still be in master then?
>
> Note the "check out the last master before the merge" step. That would
> be commit a2c32b0.

Ok, so I tried following your recipy, withouth really understanding why
it would help.

So I did:

git checkout -b master_xwfix a2c32b0
git rebase master_xwfix

... here I got into a series of conflict fixes. Since the commit scripts
are more scrupulous now, there are many whitespace conflicts and so on,
and when I got to a too long commit message conflict, I decided I'm
probbly doing something wrong.

Is there some other way to revert the merge?

I would like to revert, fix up the branch according to the various
advice I've gotten, and only then re-attempt a merge.




-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:30               ` Dmitry Gutov
  2015-02-01 19:48                 ` joakim
@ 2015-02-01 19:53                 ` Paul Eggert
  2015-02-01 19:59                   ` joakim
  2015-02-01 20:05                   ` Dmitry Gutov
  1 sibling, 2 replies; 50+ messages in thread
From: Paul Eggert @ 2015-02-01 19:53 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

Dmitry Gutov wrote:
> On 02/01/2015 06:17 PM, joakim@verona.se wrote:
>> But wouldnt the xwidget branch still be in master then?
>
> Note the "check out the last master before the merge" step. That would be commit
> a2c32b0.

I did that just now in commit 241260cc2819e5df254ad85953588b06388ade61, which 
reverts all the xwidget-related changes since that commit.  This should let 
people install independent patches without worrying about the xwidgets merge for 
now.  Another option would be to rewrite the master history; I've never been 
comfortable with that, but please feel free to do it that way if you prefer (and 
please don't forget to keep the non-xwidget-related changes since that commit, 
currently limited to lisp/ldefs-boot.el).



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:39                 ` Dmitry Gutov
@ 2015-02-01 19:56                   ` Eli Zaretskii
  2015-02-01 20:41                     ` Dmitry Gutov
  0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2015-02-01 19:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: schwab, joakim, deng, emacs-devel

> Date: Sun, 01 Feb 2015 21:39:17 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org,
>  joakim@verona.se
> 
> However I wouldn't say that the forest of merges if the biggest problem 
> we have.

When did you last look at "git log --graph"?



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:53                 ` Paul Eggert
@ 2015-02-01 19:59                   ` joakim
  2015-02-01 20:05                   ` Dmitry Gutov
  1 sibling, 0 replies; 50+ messages in thread
From: joakim @ 2015-02-01 19:59 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Dmitry Gutov wrote:
>> On 02/01/2015 06:17 PM, joakim@verona.se wrote:
>>> But wouldnt the xwidget branch still be in master then?
>>
>> Note the "check out the last master before the merge" step. That would be commit
>> a2c32b0.
>
> I did that just now in commit
> 241260cc2819e5df254ad85953588b06388ade61, which reverts all the
> xwidget-related changes since that commit.  This should let people
> install independent patches without worrying about the xwidgets merge
> for now.  Another option would be to rewrite the master history; I've
> never been comfortable with that, but please feel free to do it that
> way if you prefer (and please don't forget to keep the
> non-xwidget-related changes since that commit, currently limited to
> lisp/ldefs-boot.el).
>

Thank you very much! Can I go back to my cave and fix the xwidget branch
now?

Also, how did you actually achieve the revert?

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:53                 ` Paul Eggert
  2015-02-01 19:59                   ` joakim
@ 2015-02-01 20:05                   ` Dmitry Gutov
  1 sibling, 0 replies; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 20:05 UTC (permalink / raw)
  To: Paul Eggert, joakim; +Cc: emacs-devel

On 02/01/2015 09:53 PM, Paul Eggert wrote:

 > Another option would be to rewrite the master
> history; I've never been comfortable with that, but please feel free to
> do it that way if you prefer (and please don't forget to keep the
> non-xwidget-related changes since that commit, currently limited to
> lisp/ldefs-boot.el).

I can try my hand at rewriting history, but IIRC the current repository 
rejects non-fast-forward pushes, so just your permission won't be enough.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 16:09           ` Dmitry Gutov
  2015-02-01 16:17             ` joakim
@ 2015-02-01 20:08             ` David Engster
  2015-02-01 20:18               ` Dmitry Gutov
  1 sibling, 1 reply; 50+ messages in thread
From: David Engster @ 2015-02-01 20:08 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Paul Eggert, joakim, emacs-devel

Dmitry Gutov writes:
> And then do 'git push -f' (!!!). Someone else should make a ruling on
> this, though, but if we're going to transition on auto-generated
> ChangeLogs, we probably don't want to have ugly history.

'push -f' is not an option.

Ugly history happens. It's a nuisance, but the generated Changelogs will
have to be edited anyway (at least that's my experience from generating
Changelogs for CEDET). But I think we should have a method to override
the Changelog from the commit message; 'git-notes' would be an option
for that.

I fail to see what is gained by reverting the changes from this
merge. It makes the history even more messy.

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 20:08             ` David Engster
@ 2015-02-01 20:18               ` Dmitry Gutov
  2015-02-01 20:21                 ` David Engster
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 20:18 UTC (permalink / raw)
  To: David Engster; +Cc: Paul Eggert, joakim, emacs-devel

On 02/01/2015 10:08 PM, David Engster wrote:

> I fail to see what is gained by reverting the changes from this
> merge. It makes the history even more messy.

But that allows up to get those changes later in commit(s) with 
message(s) that contain proper ChangeLog entries.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 20:18               ` Dmitry Gutov
@ 2015-02-01 20:21                 ` David Engster
  0 siblings, 0 replies; 50+ messages in thread
From: David Engster @ 2015-02-01 20:21 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Paul Eggert, joakim, emacs-devel

Dmitry Gutov writes:
> On 02/01/2015 10:08 PM, David Engster wrote:
>
>> I fail to see what is gained by reverting the changes from this
>> merge. It makes the history even more messy.
>
> But that allows up to get those changes later in commit(s) with
> message(s) that contain proper ChangeLog entries.

But the old commits are still there, so the script will generate
changelogs for them as well. So I think we should rather attach notes to
these commits with proper changelogs.

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:56                   ` Eli Zaretskii
@ 2015-02-01 20:41                     ` Dmitry Gutov
  2015-02-02  3:30                       ` Eli Zaretskii
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-01 20:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: schwab, joakim, deng, emacs-devel

On 02/01/2015 09:56 PM, Eli Zaretskii wrote:

> When did you last look at "git log --graph"?

My preferred alias for 'git log' contains the '--graph' argument, but I 
haven't pulled the new changes yet.

Even if we don't do the rewrite, the forest will subside as new commits 
are created.

On the other hand, we have this bunch of changes that 
(organization-wise) is a mess and shouldn't have been pushed without a 
proper review, and maybe even going through an integration server.

Fast-forward merges are a minor problem, relatively speaking.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 19:29               ` David Engster
  2015-02-01 19:39                 ` Dmitry Gutov
@ 2015-02-02  1:35                 ` Stephen J. Turnbull
  2015-02-02  1:57                   ` Dmitry Gutov
  1 sibling, 1 reply; 50+ messages in thread
From: Stephen J. Turnbull @ 2015-02-02  1:35 UTC (permalink / raw)
  To: David Engster
  Cc: Eli Zaretskii, emacs-devel, Andreas Schwab, joakim, Dmitry Gutov

David Engster writes:

 > > But only because he merged from master, which was wrong to begin with.
 > 
 > It's a very easy mistake to make, and I'd rather say it is wrong that
 > Git defaults to fast-forward merges.

If you don't like this default, write a better VCS that takes over the
world, or volunteer to maintain a fork and get Emacs to adopt it.

But on second thought I don't understand how this happened.  ISTM it
should be easy to check accurately for preservation of mainline (the
current head in the remote parent should be a leftmost descendent of
the proposed new head).  I thought that check was already implemented
on Savannah?

If on wants to, that check can actually be done locally (of course you
need to first git fetch the upstream branch, and there is as usual a
race condition in the interval until you push).



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  1:35                 ` Stephen J. Turnbull
@ 2015-02-02  1:57                   ` Dmitry Gutov
  2015-02-02  2:12                     ` Stephen J. Turnbull
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-02  1:57 UTC (permalink / raw)
  To: Stephen J. Turnbull, David Engster
  Cc: Eli Zaretskii, Andreas Schwab, joakim, emacs-devel

On 02/02/2015 03:35 AM, Stephen J. Turnbull wrote:

> If on wants to, that check can actually be done locally (of course you
> need to first git fetch the upstream branch, and there is as usual a
> race condition in the interval until you push).

Why do we need to fetch first? Just check against origin/master. If it's 
outdated, the push will most likely fail anyway.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  1:57                   ` Dmitry Gutov
@ 2015-02-02  2:12                     ` Stephen J. Turnbull
  2015-02-02  2:21                       ` Dmitry Gutov
  0 siblings, 1 reply; 50+ messages in thread
From: Stephen J. Turnbull @ 2015-02-02  2:12 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov writes:
 > On 02/02/2015 03:35 AM, Stephen J. Turnbull wrote:
 > 
 > > If on wants to, that check can actually be done locally (of course you
 > > need to first git fetch the upstream branch, and there is as usual a
 > > race condition in the interval until you push).
 > 
 > Why do we need to fetch first? Just check against origin/master. If it's 
 > outdated, the push will most likely fail anyway.

It didn't this time.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  2:12                     ` Stephen J. Turnbull
@ 2015-02-02  2:21                       ` Dmitry Gutov
  2015-02-02  3:45                         ` Stephen J. Turnbull
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Gutov @ 2015-02-02  2:21 UTC (permalink / raw)
  To: Stephen J. Turnbull; +Cc: emacs-devel

On 02/02/2015 04:12 AM, Stephen J. Turnbull wrote:
>   > Why do we need to fetch first? Just check against origin/master. If it's
>   > outdated, the push will most likely fail anyway.
>
> It didn't this time.

That means origin/master on Joakim's machine wasn't outdated. Which, 
without someone implementing the actual check, doesn't give us anything.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 20:41                     ` Dmitry Gutov
@ 2015-02-02  3:30                       ` Eli Zaretskii
  2015-02-02  8:03                         ` David Engster
  0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2015-02-02  3:30 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: schwab, deng, joakim, emacs-devel

> Date: Sun, 01 Feb 2015 22:41:59 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: schwab@linux-m68k.org, joakim@verona.se, deng@randomsample.de,
> 	emacs-devel@gnu.org
> 
> On 02/01/2015 09:56 PM, Eli Zaretskii wrote:
> 
> > When did you last look at "git log --graph"?
> 
> My preferred alias for 'git log' contains the '--graph' argument, but I 
> haven't pulled the new changes yet.

It didn't start yesterday.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  2:21                       ` Dmitry Gutov
@ 2015-02-02  3:45                         ` Stephen J. Turnbull
  0 siblings, 0 replies; 50+ messages in thread
From: Stephen J. Turnbull @ 2015-02-02  3:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov writes:
 > On 02/02/2015 04:12 AM, Stephen J. Turnbull wrote:

 > >   > Why do we need to fetch first? Just check against
 > >   > origin/master. If it's outdated, the push will most likely
 > >   > fail anyway.
 > >
 > > It didn't this time.
 > 
 > That means origin/master on Joakim's machine wasn't outdated. Which, 
 > without someone implementing the actual check, doesn't give us anything.

The check isn't hard, it's just

    current=$(git rev-parse origin/master)
    base=$(git rev-list --first-parent origin/master^..HEAD | tail -1)
    test "$current" = "$base"

(untested).  Probably we should be a little more careful about
specifying what is proposed for push on the user side than just
"HEAD", but that gives the basic computation.

Turning that into a hook for receive-pack is something that may be
equally trivial, but I don't have the time (or the interest) to
investigate hook-writing.

In any case, the point of doing the check locally is to avoid a failed
push which is annoying and upsetting, especially to users who are used
to a different workflow or VCS.  If you're doing the check offline or
otherwise the fetch of the branch you want to push to fails, sure,
fall back to the current state.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  3:30                       ` Eli Zaretskii
@ 2015-02-02  8:03                         ` David Engster
  2015-02-02 10:53                           ` Ulrich Mueller
  2015-02-02 16:30                           ` Eli Zaretskii
  0 siblings, 2 replies; 50+ messages in thread
From: David Engster @ 2015-02-02  8:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, schwab, joakim, Dmitry Gutov

Eli Zaretskii writes:
>> Date: Sun, 01 Feb 2015 22:41:59 +0200
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Cc: schwab@linux-m68k.org, joakim@verona.se, deng@randomsample.de,
>
>> 	emacs-devel@gnu.org
>> 
>> On 02/01/2015 09:56 PM, Eli Zaretskii wrote:
>> 
>> > When did you last look at "git log --graph"?
>> 
>> My preferred alias for 'git log' contains the '--graph' argument, but I 
>> haven't pulled the new changes yet.
>
> It didn't start yesterday.

I think we can at least partly fix this by simply doing what Joakim did,
but the other way round. We create a branch from the last proper
mainline commit Joakim merged into his branch (a2c32b0cfc), do a no-ff
merge of master into it and then do a fast-forward merge back:

- git checkout -b dagfix a2c32b0cfc9f6d3410e2832d8ea0d4f1df576d1e

- git merge --no-ff master

- git checkout master

- git merge dagfix

Opinions?

-David



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  8:03                         ` David Engster
@ 2015-02-02 10:53                           ` Ulrich Mueller
  2015-02-02 16:30                           ` Eli Zaretskii
  1 sibling, 0 replies; 50+ messages in thread
From: Ulrich Mueller @ 2015-02-02 10:53 UTC (permalink / raw)
  To: David Engster; +Cc: Eli Zaretskii, Dmitry Gutov, schwab, joakim, emacs-devel

>>>>> On Mon, 02 Feb 2015, David Engster wrote:

> I think we can at least partly fix this by simply doing what Joakim did,
> but the other way round. We create a branch from the last proper
> mainline commit Joakim merged into his branch (a2c32b0cfc), do a no-ff
> merge of master into it and then do a fast-forward merge back:

> - git checkout -b dagfix a2c32b0cfc9f6d3410e2832d8ea0d4f1df576d1e

> - git merge --no-ff master

> - git checkout master

> - git merge dagfix

> Opinions?

That's probably the best what can be done without rewriting the
history. It should be done soon, though.

Ulrich



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02  8:03                         ` David Engster
  2015-02-02 10:53                           ` Ulrich Mueller
@ 2015-02-02 16:30                           ` Eli Zaretskii
  2015-02-02 23:19                             ` Stefan Monnier
  1 sibling, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2015-02-02 16:30 UTC (permalink / raw)
  To: David Engster; +Cc: emacs-devel, schwab, joakim, dgutov

> From: David Engster <deng@randomsample.de>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  schwab@linux-m68k.org,  joakim@verona.se,  emacs-devel@gnu.org
> Date: Mon, 02 Feb 2015 09:03:58 +0100
> 
> >> On 02/01/2015 09:56 PM, Eli Zaretskii wrote:
> >> 
> >> > When did you last look at "git log --graph"?
> >> 
> >> My preferred alias for 'git log' contains the '--graph' argument, but I 
> >> haven't pulled the new changes yet.
> >
> > It didn't start yesterday.
> 
> I think we can at least partly fix this by simply doing what Joakim did,
> but the other way round. We create a branch from the last proper
> mainline commit Joakim merged into his branch (a2c32b0cfc)

When I said "didn't start yesterday", I meant Joakim wasn't the first
one to introduce such merges into the repository.  I see several more
merge-commits with parents in reverse order, if I look far enough back.

> - git checkout -b dagfix a2c32b0cfc9f6d3410e2832d8ea0d4f1df576d1e
> 
> - git merge --no-ff master
> 
> - git checkout master
> 
> - git merge dagfix
> 
> Opinions?

That will only fix the last instance of such merges.  But maybe we
don't care.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-02 16:30                           ` Eli Zaretskii
@ 2015-02-02 23:19                             ` Stefan Monnier
  0 siblings, 0 replies; 50+ messages in thread
From: Stefan Monnier @ 2015-02-02 23:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dgutov, schwab, joakim, David Engster, emacs-devel

> That will only fix the last instance of such merges.  But maybe we
> don't care.

In my experience, Git makes it very easy to get the parents "in the
wrong order" (contrary to Bazaar which paid special attention to it).
The upside is that Git is not very much affected by the order of
parents, in most cases.  So it's probably not worth worrying about.


        Stefan



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-01 10:53       ` Paul Eggert
  2015-02-01 15:46         ` joakim
@ 2015-02-03 22:38         ` joakim
  2015-02-03 23:42           ` Paul Eggert
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-03 22:38 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel, Dmitry Gutov

Paul Eggert <eggert@cs.ucla.edu> writes:

> joakim@verona.se wrote:
>>> Given the above, maybe you should revert the merge. Then squash all
>>> >commits in xwidget into one patch, amend it with proper ChangeLog
>>> >entries, and then do the merge.
>>> >
>> Perhaps that would be best.
>
> Yes, please do that.  Here are some other things to do before committing the merge.
>
> * Reindent as per GNU standards.  Start with "indent -gnu" but it
> won't do a perfect job.  E.g., say "char *p" not "char* p".
>
> * Fit it into 80 columns.
>
> * Use GNU style for comments.  These should typically use complete,
> imperative sentences.
>
> * Configure with "./configure --enable-gcc-warnings --with-xwidgets
> --with-x-toolkit=gtk3" and fix all the warnings.

I started fixing a bunch of the warnings, and it improves the code, so
thanks for the hint. I commited to the branch.

Is there a way to get the same warning, but not have compilation error
out though? There seems to be a bunch of gtk warnings I cant really do
anything about.

  --enable-gtk-deprecation-warnings

seems to be good as well.




>
> * It's OK to assume C99 now.
>
> * Don't make functions extern unless they need to be extern.
> Compilers do a better job with static functions, typically.
>
> * Some of those function names are too long; please shorten them.
>
> * A lot of the printf statements look like they shouldn't be there.
>
> * There's some commented-out code that should be removed.
>
> * Omit pointer casts that aren't needed (when casting to and from void *).
>
> I started to look into all that and came up with the attached patch,
> relative to commit 9fe732a02afbe0b3d4a85d2bcae687900ab881f7; please
> have a look.  But the result still doesn't compile due to warnings and
> I'm sure I missed a lot of things.  I hope you can finish the job.
> (Also, the ChangeLog entries need to be written -- I started on that
> but it's a big job and it's something the author of the patch really
> should do.)
>
>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-03 22:38         ` joakim
@ 2015-02-03 23:42           ` Paul Eggert
  2015-02-04 15:59             ` joakim
  0 siblings, 1 reply; 50+ messages in thread
From: Paul Eggert @ 2015-02-03 23:42 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

joakim@verona.se wrote:
> There seems to be a bunch of gtk warnings I cant really do anything about.

You should be able to fix them all.  The attached patch did that for me, on 
Ubuntu 14.10 anyway.  This patch uses the usual GNU Emacs style in the code I 
fixed; similar fixes are needed for the rest of the xwidget code, but one thing 
at a time.)

>    --enable-gtk-deprecation-warnings
>
> seems to be good as well.

That's disabled by default due to its false alarms.  It is helpful to enable it 
occasionally, and you can still build despite the warnings by using 'make 
WERROR_CFLAGS='.  I don't recommend this for ordinary development, though.

[-- Attachment #2: xwidget-warn.diff --]
[-- Type: text/x-patch, Size: 4208 bytes --]

diff --git a/src/xwidget.c b/src/xwidget.c
index 5c816cf..45f2507 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -97,6 +97,13 @@
 
 #include "xwidget.h"
 
+/* Convert STRING, a string constant, to a type acceptable as glib data.  */
+static char *
+gstr (char const *string)
+{
+  return (char *) string;
+}
+
 //TODO embryo of lisp allocators for xwidgets
 //TODO xwidget* should be Lisp_xwidget*
 static struct xwidget*
@@ -146,7 +153,7 @@ gboolean webkit_osr_navigation_policy_decision_requested_callback(WebKitWebView
                                                         WebKitWebPolicyDecision   *policy_decision,
                                                                   gpointer                   user_data);
 
-static GtkWidget* xwgir_create(unsigned char* class, unsigned char* namespace);
+static GtkWidget *xwgir_create (char *, char *);
 
 
 
@@ -223,9 +230,14 @@ TYPE is a symbol which can take one of the following values:
       }
       if(EQ(xw->type, Qsocket_osr))
           xw->widget_osr = gtk_socket_new();
-      if(!NILP (Fget(xw->type, QCxwgir_class)))
-          xw->widget_osr = xwgir_create(SDATA(Fcar(Fcdr(Fget(xw->type, QCxwgir_class)))),
-                                        SDATA(Fcar(Fget(xw->type, QCxwgir_class))));
+
+      Lisp_Object xwgir_class = Fget (xw->type, QCxwgir_class);
+      if (!NILP (xwgir_class))
+	{
+	  char *class = SSDATA (Fcar (Fcdr (xwgir_class)));
+	  char *namespace = SSDATA (XCDR (xwgir_class));
+	  xw->widget_osr = xwgir_create (class, namespace);
+	}
 
       gtk_widget_set_size_request (GTK_WIDGET (xw->widget_osr), xw->width, xw->height);
 
@@ -610,7 +622,9 @@ DEFUN ("xwgir-require-namespace", Fxwgir_require_namespace, Sxwgir_require_names
   return Qt;
 }
 
-GtkWidget* xwgir_create(unsigned char* class, unsigned char* namespace){
+GtkWidget *
+xwgir_create (char *class, char *namespace)
+{
   //TODO this is more or less the same as xwgir-call-method, so should be refactored
   //create a gtk widget, given its name
   //find the constructor
@@ -838,16 +852,20 @@ xwidget_init_view (struct xwidget *xww,
   //widget creation
   if(EQ(xww->type, Qbutton))
     {
-      xv->widget = gtk_button_new_with_label (XSTRING(xww->title)->data);
+      xv->widget = gtk_button_new_with_label (SSDATA (xww->title));
       g_signal_connect (G_OBJECT (xv->widget), "clicked",
                         G_CALLBACK (buttonclick_handler), xv); // the view rather than the model
     } else if (EQ(xww->type, Qtoggle)) {
-    xv->widget = gtk_toggle_button_new_with_label (XSTRING(xww->title)->data);
+    xv->widget = gtk_toggle_button_new_with_label (SSDATA (xww->title));
     //xv->widget = gtk_entry_new ();//temp hack to experiment with key propagation TODO entry widget is useful for testing
   } else if (EQ(xww->type, Qsocket)) {
     xv->widget = gtk_socket_new ();
-    g_signal_connect_after(xv->widget, "plug-added", G_CALLBACK(xwidget_plug_added), "plug added");
-    g_signal_connect_after(xv->widget, "plug-removed", G_CALLBACK(xwidget_plug_removed), "plug removed");
+    g_signal_connect_after (xv->widget, "plug-added",
+			    G_CALLBACK (xwidget_plug_added),
+			    gstr ("plug added"));
+    g_signal_connect_after (xv->widget, "plug-removed",
+			    G_CALLBACK (xwidget_plug_removed),
+			    gstr ("plug removed"));
     //TODO these doesnt help
     gtk_widget_add_events(xv->widget, GDK_KEY_PRESS);
     gtk_widget_add_events(xv->widget, GDK_KEY_RELEASE);
@@ -856,7 +874,9 @@ xwidget_init_view (struct xwidget *xww,
       //gtk_hscale_new (GTK_ADJUSTMENT(gtk_adjustment_new (0.0, 0.0, 100.0, 1.0, 10.0, 10.0)));
       gtk_hscale_new_with_range ( 0.0, 100.0, 10.0);
     gtk_scale_set_draw_value (GTK_SCALE (xv->widget), FALSE);	//i think its emacs role to show text and stuff, so disable the widgets own text
-    xv->handler_id = g_signal_connect_after(xv->widget, "value-changed", G_CALLBACK(xwidget_slider_changed), "slider changed");
+    xv->handler_id = g_signal_connect_after (xv->widget, "value-changed",
+					     G_CALLBACK (xwidget_slider_changed),
+					     gstr ("slider changed"));
   } else if (EQ(xww->type, Qcairo)) {
     //Cairo view
     //uhm cairo is differentish in gtk 3.

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-03 23:42           ` Paul Eggert
@ 2015-02-04 15:59             ` joakim
  2015-02-04 18:57               ` joakim
  0 siblings, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-04 15:59 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> joakim@verona.se wrote:
>> There seems to be a bunch of gtk warnings I cant really do anything about.
>
> You should be able to fix them all.  The attached patch did that for
> me, on Ubuntu 14.10 anyway.  This patch uses the usual GNU Emacs style
> in the code I fixed; similar fixes are needed for the rest of the
> xwidget code, but one thing at a time.)

Cool, with your advice I was able to fix all the warnings!
I pushed such a patch (similar to yours) to the repo now.

Next I will run gnu indent.

>
>>    --enable-gtk-deprecation-warnings
>>
>> seems to be good as well.
>
> That's disabled by default due to its false alarms.  It is helpful to
> enable it occasionally, and you can still build despite the warnings
> by using 'make WERROR_CFLAGS='.  I don't recommend this for ordinary
> development, though.
>
>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-04 15:59             ` joakim
@ 2015-02-04 18:57               ` joakim
  2015-02-05  0:38                 ` Paul Eggert
  0 siblings, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-04 18:57 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

joakim@verona.se writes:

> Paul Eggert <eggert@cs.ucla.edu> writes:
>
>> joakim@verona.se wrote:
>>> There seems to be a bunch of gtk warnings I cant really do anything about.
>>
>> You should be able to fix them all.  The attached patch did that for
>> me, on Ubuntu 14.10 anyway.  This patch uses the usual GNU Emacs style
>> in the code I fixed; similar fixes are needed for the rest of the
>> xwidget code, but one thing at a time.)
>
> Cool, with your advice I was able to fix all the warnings!
> I pushed such a patch (similar to yours) to the repo now.
>
> Next I will run gnu indent.

Running indent didn't work out too well.

I get stuff like this:

xwidget.c: In function ‘Fmake_xwidget’:
xwidget.c:221:3: error: ‘Vxwidget_list’ undeclared (first use in this function)
   Vxwidget_list = Fcons (val, Vxwidget_list);
   ^


I'm guessing gnu indent breaks some Emacs build macrology.

Any hints?

>
>>
>>>    --enable-gtk-deprecation-warnings
>>>
>>> seems to be good as well.
>>
>> That's disabled by default due to its false alarms.  It is helpful to
>> enable it occasionally, and you can still build despite the warnings
>> by using 'make WERROR_CFLAGS='.  I don't recommend this for ordinary
>> development, though.
>>
>>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-04 18:57               ` joakim
@ 2015-02-05  0:38                 ` Paul Eggert
  2015-02-05 15:54                   ` joakim
  2015-02-09 11:56                   ` joakim
  0 siblings, 2 replies; 50+ messages in thread
From: Paul Eggert @ 2015-02-05  0:38 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

joakim@verona.se wrote:
> I'm guessing gnu indent breaks some Emacs build macrology.

Yes, GNU indent doesn't understand DEFUN or DEFVAR_LISP.  Those, you need to 
reindent by hand.  Please see the patch in my earlier message for a good style 
for doing that, at:

http://lists.gnu.org/archive/html/emacs-devel/2015-02/msg00012.html

That patch also contains several other ideas worth considering.  If you don't 
understand any part of that patch please ask.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-05  0:38                 ` Paul Eggert
@ 2015-02-05 15:54                   ` joakim
  2015-02-05 16:17                     ` Paul Eggert
  2015-02-09 11:56                   ` joakim
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-05 15:54 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> joakim@verona.se wrote:
>> I'm guessing gnu indent breaks some Emacs build macrology.
>
> Yes, GNU indent doesn't understand DEFUN or DEFVAR_LISP.  Those, you
> need to reindent by hand.  Please see the patch in my earlier message
> for a good style for doing that, at:
>
> http://lists.gnu.org/archive/html/emacs-devel/2015-02/msg00012.html
>
> That patch also contains several other ideas worth considering.  If
> you don't understand any part of that patch please ask.
>

Is it possible to somehow regenerate your patch against whats in the
xwidget branch now?

I fixed a number of things, including using gnu indent, and the src has
now diverged from the state of your patch.

If its not possible to regenerate the patch I will work through it by
hand.

Some questions on the patch though:

- Are you supposed to have a space after # on ifdefed macros?

-  you mentioned some names are long, I suppose like
 webkit_osr_mime_type_policy_typedecision_requested_callback

These are generated algorithmically from signal names. Do you have an
idea for another algorithm?




-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-05 15:54                   ` joakim
@ 2015-02-05 16:17                     ` Paul Eggert
  2015-02-09 11:50                       ` joakim
  0 siblings, 1 reply; 50+ messages in thread
From: Paul Eggert @ 2015-02-05 16:17 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

On 02/05/2015 07:54 AM, joakim@verona.se wrote:
> Is it possible to somehow regenerate your patch against whats in the
> xwidget branch now?

Sorry, my patch was all done by hand.

> Some questions on the patch though:
>
> - Are you supposed to have a space after # on ifdefed macros?

Nowadays it's considered nicer to indent there, e.g., see "#ifdef 
MAIN_PROGRAM" in lisp.h.  Previously we didn't bother and a lot of the 
old code is still around, but we're talking new code here....

> -  you mentioned some names are long, I suppose like
>   webkit_osr_mime_type_policy_typedecision_requested_callback
>
> These are generated algorithmically from signal names. Do you have an
> idea for another algorithm?
>

The names should be static, so there's no need to worry about keeping 
them globally unique in the Emacs source code; they need to be unique 
only in xwidgets.c.  For too-long names like that, how about using the 
same name as the g_signal_comment string, replacing "-" with "_"?  E.g., 
mime_type_policy_decision_requested.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-05 16:17                     ` Paul Eggert
@ 2015-02-09 11:50                       ` joakim
  0 siblings, 0 replies; 50+ messages in thread
From: joakim @ 2015-02-09 11:50 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 02/05/2015 07:54 AM, joakim@verona.se wrote:
>> Is it possible to somehow regenerate your patch against whats in the
>> xwidget branch now?
>
> Sorry, my patch was all done by hand.
>
>> Some questions on the patch though:
>>
>> - Are you supposed to have a space after # on ifdefed macros?
>
> Nowadays it's considered nicer to indent there, e.g., see "#ifdef
> MAIN_PROGRAM" in lisp.h.  Previously we didn't bother and a lot of the
> old code is still around, but we're talking new code here....
>
>> -  you mentioned some names are long, I suppose like
>>   webkit_osr_mime_type_policy_typedecision_requested_callback
>>
>> These are generated algorithmically from signal names. Do you have an
>> idea for another algorithm?
>>
>
> The names should be static, so there's no need to worry about keeping
> them globally unique in the Emacs source code; they need to be unique
> only in xwidgets.c.  For too-long names like that, how about using the
> same name as the g_signal_comment string, replacing "-" with "_"?
> E.g., mime_type_policy_decision_requested.
>

I did this in the xwidget_mvp branch, except I prefixed and postfixed
with webkit_*_cb. So I can shorten them a bit further if really needed.



-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-05  0:38                 ` Paul Eggert
  2015-02-05 15:54                   ` joakim
@ 2015-02-09 11:56                   ` joakim
  2015-02-09 19:47                     ` Paul Eggert
  1 sibling, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-09 11:56 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> joakim@verona.se wrote:
>> I'm guessing gnu indent breaks some Emacs build macrology.
>
> Yes, GNU indent doesn't understand DEFUN or DEFVAR_LISP.  Those, you
> need to reindent by hand.  Please see the patch in my earlier message
> for a good style for doing that, at:
>
> http://lists.gnu.org/archive/html/emacs-devel/2015-02/msg00012.html
>
> That patch also contains several other ideas worth considering.  If
> you don't understand any part of that patch please ask.
>

I'm working through the patch, but its a bit slow going.

There seemed to be some spurious diffs:
--- a/src/coding.c
+++ b/src/coding.c
@@ -5985,7 +5985,7 @@ bool
 raw_text_coding_system_p (struct coding_system *coding)
 {
   return (coding->decoder == decode_coding_raw_text
-	  && coding->encoder == encode_coding_raw_text) ? true : false;
+	  && coding->encoder == encode_coding_raw_text);
 }


And these:
-  next_element_from_stretch
+  next_element_from_stretch,
 #ifdef HAVE_XWIDGETS
-  ,next_element_from_xwidget
+  next_element_from_xwidget,

is it really correct to move the coma out of the ifdef?
I remember having trouble with that when I wrote the code originally.

My latest patch is in the xwidget_mvp branch, 

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-09 11:56                   ` joakim
@ 2015-02-09 19:47                     ` Paul Eggert
  2015-02-09 20:24                       ` joakim
  0 siblings, 1 reply; 50+ messages in thread
From: Paul Eggert @ 2015-02-09 19:47 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

On 02/09/2015 03:56 AM, joakim@verona.se wrote:
> --- a/src/coding.c
> +++ b/src/coding.c
> @@ -5985,7 +5985,7 @@ bool
>   raw_text_coding_system_p (struct coding_system *coding)
>   {
>     return (coding->decoder == decode_coding_raw_text
> -	  && coding->encoder == encode_coding_raw_text) ? true : false;
> +	  && coding->encoder == encode_coding_raw_text);
>   }
>

Generally speaking, if EXPR is a boolean expression, it's simpler and 
clearer to write 'EXPR' than to write 'EXPR ? true : false'. Also, it 
makes for clearer indenting in this case.

> And these:
> -  next_element_from_stretch
> +  next_element_from_stretch,
>   #ifdef HAVE_XWIDGETS
> -  ,next_element_from_xwidget
> +  next_element_from_xwidget,
>
> is it really correct to move the coma out of the ifdef?

Yes, in C99, which Emacs is now assuming.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-09 19:47                     ` Paul Eggert
@ 2015-02-09 20:24                       ` joakim
  2015-02-09 22:20                         ` Paul Eggert
  0 siblings, 1 reply; 50+ messages in thread
From: joakim @ 2015-02-09 20:24 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 02/09/2015 03:56 AM, joakim@verona.se wrote:
>> --- a/src/coding.c
>> +++ b/src/coding.c
>> @@ -5985,7 +5985,7 @@ bool
>>   raw_text_coding_system_p (struct coding_system *coding)
>>   {
>>     return (coding->decoder == decode_coding_raw_text
>> -	  && coding->encoder == encode_coding_raw_text) ? true : false;
>> +	  && coding->encoder == encode_coding_raw_text);
>>   }
>>
>
> Generally speaking, if EXPR is a boolean expression, it's simpler and
> clearer to write 'EXPR' than to write 'EXPR ? true : false'. Also, it
> makes for clearer indenting in this case.

My concern was that I have made no changees AFAIK to coding.c. So how
did it wind up in the patch you provided?


>> And these:
>> -  next_element_from_stretch
>> +  next_element_from_stretch,
>>   #ifdef HAVE_XWIDGETS
>> -  ,next_element_from_xwidget
>> +  next_element_from_xwidget,
>>
>> is it really correct to move the coma out of the ifdef?
>
> Yes, in C99, which Emacs is now assuming.

It feels weird, but well okay then. And its really considered better
form?

>

-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-09 20:24                       ` joakim
@ 2015-02-09 22:20                         ` Paul Eggert
  2015-02-10 18:27                           ` joakim
  0 siblings, 1 reply; 50+ messages in thread
From: Paul Eggert @ 2015-02-09 22:20 UTC (permalink / raw)
  To: joakim; +Cc: emacs-devel

On 02/09/2015 12:24 PM, joakim@verona.se wrote:
> My concern was that I have made no changees AFAIK to coding.c. So how
> did it wind up in the patch you provided?

I don't recall, but let's not worry about it.

>
>>> >>And these:
>>> >>-  next_element_from_stretch
>>> >>+  next_element_from_stretch,
>>> >>   #ifdef HAVE_XWIDGETS
>>> >>-  ,next_element_from_xwidget
>>> >>+  next_element_from_xwidget,
>>> >>
>>> >>is it really correct to move the coma out of the ifdef?
>> >
>> >Yes, in C99, which Emacs is now assuming.
> It feels weird, but well okay then. And its really considered better
> form?
>

I prefer it, because it lets one put #if/#endif brackets around any enum 
value without worrying about whether the value is listed first or last 
or in the middle.  The style with leading "," is less consistent, in 
that it cannot be used with the first enum value.



^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets
  2015-02-09 22:20                         ` Paul Eggert
@ 2015-02-10 18:27                           ` joakim
  0 siblings, 0 replies; 50+ messages in thread
From: joakim @ 2015-02-10 18:27 UTC (permalink / raw)
  To: Paul Eggert; +Cc: emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 02/09/2015 12:24 PM, joakim@verona.se wrote:
>> My concern was that I have made no changees AFAIK to coding.c. So how
>> did it wind up in the patch you provided?
>
> I don't recall, but let's not worry about it.
>
>>
>>>> >>And these:
>>>> >>-  next_element_from_stretch
>>>> >>+  next_element_from_stretch,
>>>> >>   #ifdef HAVE_XWIDGETS
>>>> >>-  ,next_element_from_xwidget
>>>> >>+  next_element_from_xwidget,
>>>> >>
>>>> >>is it really correct to move the coma out of the ifdef?
>>> >
>>> >Yes, in C99, which Emacs is now assuming.
>> It feels weird, but well okay then. And its really considered better
>> form?
>>
>
> I prefer it, because it lets one put #if/#endif brackets around any
> enum value without worrying about whether the value is listed first or
> last or in the middle.  The style with leading "," is less consistent,
> in that it cannot be used with the first enum value.
>

Ok, I have applied most of the ideas in you patch now, in the
xwidget_mvp branch. I might have missed something, but I think
everything is done.


-- 
Joakim Verona



^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2015-02-10 18:27 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20150201003025.18138.95966@vcs.savannah.gnu.org>
     [not found] ` <E1YHiQk-0004jf-UH@vcs.savannah.gnu.org>
2015-02-01  6:11   ` [Emacs-diffs] xwidget 9fe732a 2/2: Better changelog for xwidgets Dmitry Gutov
2015-02-01  8:50     ` joakim
2015-02-01 10:53       ` Paul Eggert
2015-02-01 15:46         ` joakim
2015-02-01 16:09           ` Dmitry Gutov
2015-02-01 16:17             ` joakim
2015-02-01 16:30               ` Dmitry Gutov
2015-02-01 19:48                 ` joakim
2015-02-01 19:53                 ` Paul Eggert
2015-02-01 19:59                   ` joakim
2015-02-01 20:05                   ` Dmitry Gutov
2015-02-01 20:08             ` David Engster
2015-02-01 20:18               ` Dmitry Gutov
2015-02-01 20:21                 ` David Engster
2015-02-03 22:38         ` joakim
2015-02-03 23:42           ` Paul Eggert
2015-02-04 15:59             ` joakim
2015-02-04 18:57               ` joakim
2015-02-05  0:38                 ` Paul Eggert
2015-02-05 15:54                   ` joakim
2015-02-05 16:17                     ` Paul Eggert
2015-02-09 11:50                       ` joakim
2015-02-09 11:56                   ` joakim
2015-02-09 19:47                     ` Paul Eggert
2015-02-09 20:24                       ` joakim
2015-02-09 22:20                         ` Paul Eggert
2015-02-10 18:27                           ` joakim
2015-02-01 15:36     ` Eli Zaretskii
2015-02-01 15:51       ` David Engster
2015-02-01 15:52       ` joakim
2015-02-01 16:04         ` David Engster
2015-02-01 16:04         ` Eli Zaretskii
2015-02-01 16:05         ` Andreas Schwab
2015-02-01 16:11           ` David Engster
2015-02-01 16:15             ` Andreas Schwab
2015-02-01 16:18               ` joakim
2015-02-01 19:29               ` David Engster
2015-02-01 19:39                 ` Dmitry Gutov
2015-02-01 19:56                   ` Eli Zaretskii
2015-02-01 20:41                     ` Dmitry Gutov
2015-02-02  3:30                       ` Eli Zaretskii
2015-02-02  8:03                         ` David Engster
2015-02-02 10:53                           ` Ulrich Mueller
2015-02-02 16:30                           ` Eli Zaretskii
2015-02-02 23:19                             ` Stefan Monnier
2015-02-02  1:35                 ` Stephen J. Turnbull
2015-02-02  1:57                   ` Dmitry Gutov
2015-02-02  2:12                     ` Stephen J. Turnbull
2015-02-02  2:21                       ` Dmitry Gutov
2015-02-02  3:45                         ` Stephen J. Turnbull

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).