vimprobable-users — Mailing list for Vimprobable users

Re: [Vimprobable-users] Editor patch evolution

[Hannes S. <ha...@yl...>]

Hello Markus!

Markus Demleitner <msd...@ar...> wrote:
> (talking about
> which: If I'd like to get this stuff into the upstream git, what
> would I do?)

As announced on IRC last week, I'm planning to merge the mature patches
(like yours) in a week or so and make a new release.

Hannes

Re: [Vimprobable-users] Editor patch evolution

[Markus D. <msd...@ar...>]

Hi list,

On Sat, Oct 06, 2012 at 12:07:08PM +0200, Alexander Foremny wrote:
> Possibly one could introduce an option in vimprobablerc which
> overrides the environment if it is specified? This seems to be common
> in other UNIX applications.
Well, I'm not sure if all that configurability is not a bit of an
overkill for something like that, but it's not much code, so here's a
patch that

(a) evaluates TMPDIR, if set, and
(b) lets you set a variable tempdir in .vimprobablerc (or
interactively)

The patch applies on top of last week's editor patch (talking about
which: If I'd like to get this stuff into the upstream git, what
would I do?)

Cheers,

         Markus

Re: [Vimprobable-users] Editor patch evolution

[Alexander F. <ale...@gm...>]

Possibly one could introduce an option in vimprobablerc which
overrides the environment if it is specified? This seems to be common
in other UNIX applications.

Regards,
Alexander Foremny

2012/10/6 Markus Demleitner <msd...@ar...>:
> Hi Serge,
>
> On Fri, Oct 05, 2012 at 02:48:50PM +0000, Serge E. Hallyn wrote:
>> Quoting Markus Demleitner (msd...@ar...):
>> > During a longish train ride today, I polished up the editor patch a
>> > bit.  For one, the editor now runs asynchronously, so vimprobable
>> Do you mind making the directory for the tempfiles configurable?  I'm
>> currently hardcoding it to getenv("HOME")/Downloads rather than using /tmp,
>> because that is the only directory to which I allow vimprobable and its
>> children write access.  It'd be nice if it could simply be set in
>> vimprobablerc.
>
> Well, the canonical way to do this is to inspect the TMPDIR
> environment variable.  I'd like that better than to introduce a
> special setting in vimprobablerc.  The downside is that you'd have to
> use a little wrapper script if you want application-specific TMPDIRs.
>
> Would that work for you?
>
> Cheers,
>
>        Markus
>
>
> ------------------------------------------------------------------------------
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> http://p.sf.net/sfu/newrelic-dev2dev
> _______________________________________________
> Vimprobable-users mailing list
> Vim...@li...
> https://lists.sourceforge.net/lists/listinfo/vimprobable-users

Re: [Vimprobable-users] Editor patch evolution

[Markus D. <msd...@ar...>]

Hi Serge,

On Fri, Oct 05, 2012 at 02:48:50PM +0000, Serge E. Hallyn wrote:
> Quoting Markus Demleitner (msd...@ar...):
> > During a longish train ride today, I polished up the editor patch a
> > bit.  For one, the editor now runs asynchronously, so vimprobable
> Do you mind making the directory for the tempfiles configurable?  I'm
> currently hardcoding it to getenv("HOME")/Downloads rather than using /tmp,
> because that is the only directory to which I allow vimprobable and its
> children write access.  It'd be nice if it could simply be set in
> vimprobablerc.

Well, the canonical way to do this is to inspect the TMPDIR
environment variable.  I'd like that better than to introduce a
special setting in vimprobablerc.  The downside is that you'd have to
use a little wrapper script if you want application-specific TMPDIRs.

Would that work for you?

Cheers,

       Markus

Re: [Vimprobable-users] Editor patch evolution

[Serge E. H. <se...@ha...>]

Quoting Markus Demleitner (msd...@ar...):
> Hi,
> 
> During a longish train ride today, I polished up the editor patch a
> bit.  For one, the editor now runs asynchronously, so vimprobable
> doesn't just sit there as long as the editor runs.  To keep people
> from futzing around in the original text field, it is now grayed out
> while the editor is active.
> 
> I've also fixed the editing behaviour in the presence of " characters
> in the content.  I'm not quite happy with this, though -- javascript
> escaping must be more complex than this.  Can anyone point to a
> comprehensive and authoritative answer as to how javascript expects
> its string liteals?
> 
> Attached are two patches; one should apply on top of the patch
> yesterday, the other should apply on a clean 1.1.1 release.
> 
> Any feedback is welcome.  Is there anyone I should my git
> format-patch to assuming people like this?

Hi,

Very nice, thanks!

Do you mind making the directory for the tempfiles configurable?  I'm
currently hardcoding it to getenv("HOME")/Downloads rather than using /tmp,
because that is the only directory to which I allow vimprobable and its
children write access.  It'd be nice if it could simply be set in
vimprobablerc.

> Cheers,
> 
>       Markus

> diff --git a/Makefile b/Makefile
> index 7493657..772a30c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -17,7 +17,7 @@ INSTALL = $(BINDIR)/$(TARGET) $(MANINSTALL)
>  # DEBUG build?  Off by default
>  V_DEBUG = 0
>  
> -CFLAGS += `pkg-config --cflags $(LIBS)` -D_XOPEN_SOURCE=500
> +CFLAGS += `pkg-config --cflags $(LIBS)`
>  LDFLAGS += `pkg-config --libs $(LIBS)` -lX11 -lXext
>  
>  # TA:  This is a pretty stringent list of warnings to bail on!
> diff --git a/keymap.h b/keymap.h
> index 86aea55..7ba5d0b 100644
> --- a/keymap.h
> +++ b/keymap.h
> @@ -114,9 +114,6 @@ Key keys[] = {
>      { 0,                    GDK_semicolon,  GDK_T,          input,      {.s = ";T"} },
>      { 0,                    GDK_semicolon,  GDK_W,          input,      {.s = ";W"} },
>  
> -    /* this needs to be a binding using CTRL for obvious reasons */
> -    { GDK_CONTROL_MASK,     0,              GDK_t,          open_editor,{} },
> -
>      { 0,                    GDK_VoidSymbol, GDK_Escape,     set,        {ModeNormal} },
>      { GDK_CONTROL_MASK,     GDK_VoidSymbol, GDK_bracketleft,set,        {ModeNormal} },
>      { GDK_CONTROL_MASK,     0,              GDK_z,          set,        {ModePassThrough} },
> diff --git a/main.c b/main.c
> index 36e0edb..b3407ba 100644
> --- a/main.c
> +++ b/main.c
> @@ -11,10 +11,7 @@
>  */
>  
>  #include <X11/Xlib.h>
> -#include <sys/types.h>
> -#include <sys/wait.h>
>  #include <errno.h>
> -#include <stdlib.h>
>  #include "includes.h"
>  #include "vimprobable.h"
>  #include "utilities.h"
> @@ -64,8 +61,6 @@ static gboolean complete(const Arg *arg);
>  static gboolean descend(const Arg *arg);
>  gboolean echo(const Arg *arg);
>  static gboolean focus_input(const Arg *arg);
> -static gboolean open_editor(const Arg *arg);
> -void _resume_from_editor(GPid child_pid, int status, gpointer data);
>  static gboolean input(const Arg *arg);
>  static gboolean navigate(const Arg *arg);
>  static gboolean number(const Arg *arg);
> @@ -437,9 +432,6 @@ webview_keypress_cb(WebKitWebView *webview, GdkEventKey *event) {
>              g_free(a.s);
>              a.i = ModeNormal;
>              return set(&a);
> -        } else if (CLEAN(event->state) & GDK_CONTROL_MASK) {
> -            /* keybindings of non-printable characters */
> -            if (process_keypress(event) == TRUE) return TRUE;
>          }
>      case ModePassThrough:
>          if (IS_ESCAPE(event)) {
> @@ -1829,182 +1821,6 @@ view_source(const Arg * arg) {
>      return TRUE;
>  }
>  
> -/* open an external editor defined by the protocol handler for
> -vimprobableedit on a text box or similar */
> -static gboolean
> -open_editor(const Arg *arg) {
> -    char *text = NULL;
> -    gboolean success;
> -    GPid child_pid;
> -    gchar *value = NULL, *message = NULL, *tag = NULL, *edit_url = NULL;
> -    gchar *temp_file_name = g_strdup("/tmp/vimprobableeditXXXXXX");
> -    int temp_file_handle = -1;
> -
> -    /* check if active element is suitable for text editing */
> -    jsapi_evaluate_script("document.activeElement.tagName", &value, &message);
> -    if (value == NULL)
> -        return FALSE;
> -    tag = g_strdup(value);
> -    if (strcmp(tag, "INPUT") == 0) {
> -        /* extra check: type == text */
> -        jsapi_evaluate_script("document.activeElement.type", &value, &message);
> -        if (strcmp(value, "text") != 0) {
> -            g_free(value);
> -            g_free(message);
> -            return FALSE;
> -        }
> -    } else if (strcmp(tag, "TEXTAREA") != 0) {
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> -    }
> -    jsapi_evaluate_script("document.activeElement.value", &value, &message);
> -    text = g_strdup(value);
> -    if (text == NULL) {
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> -    }
> -
> -    /* write text into temporary file */
> -    temp_file_handle = mkstemp(temp_file_name);
> -    if (temp_file_handle == -1) {
> -        message = g_strdup_printf("Could not create temporary file: %s",
> -            strerror(errno));
> -        give_feedback(message);
> -        g_free(value);
> -        g_free(message);
> -        g_free(text);
> -        return FALSE;
> -    }
> -    if (write(temp_file_handle, text, strlen(text)) != strlen(text)) {
> -        message = g_strdup_printf("Short write to temporary file: %s",
> -            strerror(errno));
> -        give_feedback(message);
> -        g_free(value);
> -        g_free(message);
> -        g_free(text);
> -        return FALSE;
> -	}
> -    close(temp_file_handle);
> -    g_free(text);
> -
> -    /* spawn editor */
> -    edit_url = g_strdup_printf("vimprobableedit:%s", temp_file_name);
> -    success = open_handler_pid(edit_url, &child_pid);
> -    g_free(edit_url);
> -    if (!success) {
> -        give_feedback("External editor open failed (no handler for"
> -            " vimprobableedit protocol?)");
> -        unlink(temp_file_name);
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> -    }
> -   
> -    /* mark the active text box as "under processing" */
> -    jsapi_evaluate_script(
> -        "document.activeElement.disabled = true;"
> -        "document.activeElement.originalBackground = "
> -        "   document.activeElement.style.background;"
> -        "document.activeElement.style.background = '#aaaaaa';"
> -        ,&value, &message);
> -
> -    g_child_watch_add(child_pid, _resume_from_editor, temp_file_name);
> -
> -    /* temp_file_name is freed in _resume_from_editor */
> -    g_free(value);
> -    g_free(message);
> -    g_free(tag);
> -    return TRUE;
> -}
> -
> -
> -/* pick up from where open_editor left the work to the glib event loop.
> -
> -This is called when the external editor exits. 
> -
> -The data argument points to allocated memory containing the temporary file
> -name. */
> -void 
> -_resume_from_editor(GPid child_pid, int child_status, gpointer data) {
> -    FILE *fp;
> -    GString *set_value_js = g_string_new(
> -        "document.activeElement.value = \"");
> -    g_spawn_close_pid(child_pid);
> -    gchar *value = NULL, *message = NULL;
> -    gchar *temp_file_name = data;
> -    gchar buffer[BUF_SIZE] = ""; 
> -    gchar *buf_ptr = buffer;
> -    int char_read;
> -
> -    jsapi_evaluate_script(
> -        "document.activeElement.disabled = true;"
> -        "document.activeElement.style.background = '#aaaaaa';"
> -        ,&value, &message);
> -
> -    if (child_status) {
> -        give_feedback("External editor returned with non-zero status,"
> -            " discarding edits.");
> -        goto error_exit;
> -    }
> -
> -    /* re-read the new contents of the file and put it into the HTML element */
> -    if (!access(temp_file_name, R_OK) == 0) {
> -        message = g_strdup_printf("Could not access temporary file: %s",
> -            strerror(errno));
> -        goto error_exit;
> -    }
> -    fp = fopen(temp_file_name, "r");
> -    if (fp == NULL) {
> -        /* this would be too weird to even emit an error message */
> -        goto error_exit;
> -    }
> -    jsapi_evaluate_script("document.activeElement.value = '';", 
> -        &value, &message);
> -
> -    while (EOF != (char_read = fgetc(fp))) {
> -        if (char_read == '\n') {
> -            *buf_ptr++ = '\\';
> -            *buf_ptr++ = 'n';
> -        } else if (char_read == '"') {
> -            *buf_ptr++ = '\\';
> -            *buf_ptr++ = '"';
> -        } else {
> -            *buf_ptr++ = char_read;
> -        }
> -        /* ship out as the buffer when space gets tight.  This has
> -        fuzz to save on thinking, plus we have enough space for the
> -        trailing "; in any case. */
> -        if (buf_ptr-buffer>=BUF_SIZE-10) {
> -            *buf_ptr = 0;
> -            g_string_append(set_value_js, buffer);
> -            buf_ptr = buffer;
> -        }
> -    }
> -    *buf_ptr++ = '"';
> -    *buf_ptr++ = ';';
> -    *buf_ptr = 0;
> -    g_string_append(set_value_js, buffer);
> -    fclose(fp);
> -
> -    jsapi_evaluate_script(set_value_js->str, &value, &message);
> -
> -    /* Fall through, error and normal exit are identical */
> -error_exit:
> -    jsapi_evaluate_script(
> -        "document.activeElement.disabled = false;"
> -        "document.activeElement.style.background ="
> -        "   document.activeElement.originalBackground;"
> -        ,&value, &message);
> -
> -    g_string_free(set_value_js, TRUE);
> -    unlink(temp_file_name);
> -    g_free(temp_file_name);
> -    g_free(value);
> -    g_free(message);
> -}
> -
>  static gboolean
>  focus_input(const Arg *arg) {
>      static Arg a;
> diff --git a/utilities.c b/utilities.c
> index fcef30b..6ee63d1 100644
> --- a/utilities.c
> +++ b/utilities.c
> @@ -795,22 +795,13 @@ void make_uri_handlers_list(URIHandler *uri_handlers, int length)
>      }
>  }
>  
> -
> -/* spawn a child process handling a protocol encoded in URI.
> -
> -On success, pid will contain the pid of the spawned child.
> -If you pass NULL as child_pid, glib will reap the child. */
>  gboolean
> -open_handler_pid(char *uri, GPid *child_pid) {
> +open_handler(char *uri) {
>      char *argv[64];
>      char *p = NULL, *arg, arg_temp[MAX_SETTING_SIZE], *temp, temp2[MAX_SETTING_SIZE] = "", *temp3;
>      int j;
>      GList *l;
> -    GSpawnFlags flags = G_SPAWN_SEARCH_PATH; 
>  
> -    if (child_pid) {
> -        flags |= G_SPAWN_DO_NOT_REAP_CHILD;
> -    }
>      p = strchr(uri, ':');
>      if (p) {
>      	if (dynamic_uri_handlers != NULL) {
> @@ -830,7 +821,7 @@ open_handler_pid(char *uri, GPid *child_pid) {
>                                  	strncat(arg_temp, temp3, 1);
>                                  	temp3++;
>                              	}
> -                            	strcat(arg_temp, arg+1);
> +                            	strcat(arg_temp, arg);
>                              	temp3++;
>                              	temp3++;
>                              	strcat(arg_temp, temp3);
> @@ -842,8 +833,7 @@ open_handler_pid(char *uri, GPid *child_pid) {
>                          	j++;
>                      	}
>                      	argv[j] =  NULL;
> -                    	g_spawn_async(NULL, argv, NULL, flags,
> -                    	    NULL, NULL, child_pid, NULL);
> +                    	g_spawn_async(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, NULL, NULL);
>                  	}
>                  	return TRUE;
>              	}
> @@ -853,8 +843,3 @@ open_handler_pid(char *uri, GPid *child_pid) {
>      return FALSE;
>  }
>  
> -
> -gboolean
> -open_handler(char *uri) {
> -	return open_handler_pid(uri, NULL);
> -}
> diff --git a/utilities.h b/utilities.h
> index 3b532d2..f9ac1ba 100644
> --- a/utilities.h
> +++ b/utilities.h
> @@ -35,4 +35,4 @@ char *find_uri_for_searchengine(const char *handle);
>  void make_searchengines_list(Searchengine *searchengines, int length);
>  void make_uri_handlers_list(URIHandler *uri_handlers, int length);
>  gboolean open_handler(char *uri);
> -gboolean open_handler_pid(char *uri, GPid *child_pid);
> +
> diff --git a/vimprobable.h b/vimprobable.h
> index f5d3e64..5a6c2df 100644
> --- a/vimprobable.h
> +++ b/vimprobable.h
> @@ -181,9 +181,6 @@ enum ConfigFileError {
>  #define             HISTORY_STORAGE_FILENAME    "%s/vimprobable/history", config_base
>  #define             CLOSED_URL_FILENAME         "%s/vimprobable/closed", config_base
>  
> -/* external editor - temporary file */
> -#define             TEMPFILE                    "/tmp/vimprobable_edit"
> -
>  /* Command size */
>  #define	            COMMANDSIZE	                1024
>  
> @@ -194,6 +191,3 @@ enum ConfigFileError {
>  
>  /* completion list size */
>  #define             MAX_LIST_SIZE               40
> -
> -/* Size of (some) I/O buffers */
> -#define             BUF_SIZE                    1024

> diff --git a/Makefile b/Makefile
> index fe39667..7493657 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -15,7 +15,7 @@ MANINSTALL = $(addprefix $(MANDIR)/man1/,$(MAN1)) \
>  INSTALL = $(BINDIR)/$(TARGET) $(MANINSTALL)
>  
>  # DEBUG build?  Off by default
> -V_DEBUG = 1
> +V_DEBUG = 0
>  
>  CFLAGS += `pkg-config --cflags $(LIBS)` -D_XOPEN_SOURCE=500
>  LDFLAGS += `pkg-config --libs $(LIBS)` -lX11 -lXext
> diff --git a/main.c b/main.c
> index a35e638..5acf509 100644
> --- a/main.c
> +++ b/main.c
> @@ -65,6 +65,7 @@ static gboolean descend(const Arg *arg);
>  gboolean echo(const Arg *arg);
>  static gboolean focus_input(const Arg *arg);
>  static gboolean open_editor(const Arg *arg);
> +void _resume_from_editor(GPid child_pid, int status, gpointer data);
>  static gboolean input(const Arg *arg);
>  static gboolean navigate(const Arg *arg);
>  static gboolean number(const Arg *arg);
> @@ -1832,14 +1833,11 @@ view_source(const Arg * arg) {
>  vimprobableedit on a text box or similar */
>  static gboolean
>  open_editor(const Arg *arg) {
> -    FILE *fp;
> -    char *text = NULL, s[255] = "";
> +    char *text = NULL;
>      gboolean success;
> -    GString *new_text = g_string_new("");
>      GPid child_pid;
> -    int child_status;
>      gchar *value = NULL, *message = NULL, *tag = NULL, *edit_url = NULL;
> -    gchar temp_file_name[] = "/tmp/vimprobableeditXXXXXX";
> +    gchar *temp_file_name = g_strdup("/tmp/vimprobableeditXXXXXX");
>      int temp_file_handle = -1;
>  
>      /* check if active element is suitable for text editing */
> @@ -1888,7 +1886,7 @@ open_editor(const Arg *arg) {
>          g_free(text);
>          return FALSE;
>  	}
> -	close(temp_file_handle);
> +    close(temp_file_handle);
>      g_free(text);
>  
>      /* spawn editor */
> @@ -1903,59 +1901,97 @@ open_editor(const Arg *arg) {
>          g_free(message);
>          return FALSE;
>      }
> -    
> -    /* Wait for the child to exit */
> -    /* TODO: use g_child_watch_add and make the rest a callback */
> -    while (waitpid(child_pid, &child_status, 0)) {
> -        if (errno!=EINTR) {
> -            break;
> -        }
> -    }
> -    g_spawn_close_pid (child_pid);
> +   
> +    /* mark the active text box as "under processing" */
> +    jsapi_evaluate_script(
> +        "document.activeElement.disabled = true;"
> +        "document.activeElement.originalBackground = "
> +        "   document.activeElement.style.background;"
> +        "document.activeElement.style.background = '#aaaaaa';"
> +        ,&value, &message);
> +
> +    g_child_watch_add(child_pid, _resume_from_editor, temp_file_name);
> +
> +    /* temp_file_name is freed in _resume_from_editor */
> +    g_free(value);
> +    g_free(message);
> +    g_free(tag);
> +    return TRUE;
> +}
> +
> +
> +/* pick up from where open_editor left the work to the glib event loop.
> +
> +This is called when the external editor exits. 
> +
> +The data argument points to allocated memory containing the temporary file
> +name. */
> +void 
> +_resume_from_editor(GPid child_pid, int child_status, gpointer data) {
> +    FILE *fp;
> +    GString *new_text = g_string_new("");
> +    g_spawn_close_pid(child_pid);
> +    gchar *value = NULL, *message = NULL;
> +    gchar *temp_file_name = data;
> +    gchar buffer[255] = "";
> +
> +    jsapi_evaluate_script(
> +        "document.activeElement.disabled = true;"
> +        "document.activeElement.style.background = '#aaaaaa';"
> +        ,&value, &message);
>  
>      if (child_status) {
>          give_feedback("External editor returned with non-zero status,"
>              " discarding edits.");
> -        unlink(temp_file_name);
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> +        goto error_exit;
>      }
>  
>      /* re-read the new contents of the file and put it into the HTML element */
>      if (!access(temp_file_name, R_OK) == 0) {
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> +        message = g_strdup_printf("Could not access temporary file: %s",
> +            strerror(errno));
> +        goto error_exit;
>      }
>      fp = fopen(temp_file_name, "r");
>      if (fp == NULL) {
> -        g_free(value);
> -        g_free(message);
> -        return FALSE;
> +        /* this would be too weird to even emit an error message */
> +        goto error_exit;
>      }
> -    jsapi_evaluate_script("document.activeElement.value = '';", &value, &message);
> +    jsapi_evaluate_script("document.activeElement.value = '';", 
> +        &value, &message);
>      new_text = g_string_append(new_text, "\"");
> -    while (fgets(s, 254, fp)) {
> -        if (s[strlen(s)-1] == '\n') {
> +    while (fgets(buffer, 254, fp)) {
> +        if (buffer[strlen(buffer)-1] == '\n') {
>              /* encode line breaks into the string as Javascript does not like actual line breaks */
> -            new_text = g_string_append_len(new_text, s, strlen(s) - 1);
> +            new_text = g_string_append_len(
> +                new_text, buffer, strlen(buffer) - 1);
>              new_text = g_string_append(new_text, "\\n");
>          } else {
> -            new_text = g_string_append(new_text, s);
> +            new_text = g_string_append(new_text, buffer);
>          }
>      }
>      new_text = g_string_append(new_text, "\"");
>      fclose(fp);
> -    jsapi_evaluate_script(g_strconcat("document.activeElement.value = ", new_text->str, ";", NULL), &value, &message);
> +    /* FIXME: Is the memory returned by g_strconcat actually freed? */
> +    jsapi_evaluate_script(g_strconcat("document.activeElement.value = ", 
> +        new_text->str, ";", NULL), &value, &message);
> +
> +    /* Fall through, error and normal exit are identical */
> +error_exit:
> +    if (new_text) {
> +        g_string_free(new_text, TRUE);
> +    }
>  
> -    /* done */
> -    g_string_free(new_text, TRUE);
> +    jsapi_evaluate_script(
> +        "document.activeElement.disabled = false;"
> +        "document.activeElement.style.background ="
> +        "   document.activeElement.originalBackground;"
> +        ,&value, &message);
> +
> +    unlink(temp_file_name);
> +    g_free(temp_file_name);
>      g_free(value);
>      g_free(message);
> -    g_free(tag);
> -    unlink(temp_file_name);
> -    return TRUE;
>  }
>  
>  static gboolean

> ------------------------------------------------------------------------------
> Got visibility?
> Most devs has no idea what their production app looks like.
> Find out how fast your code is with AppDynamics Lite.
> http://ad.doubleclick.net/clk;262219671;13503038;y?
> http://info.appdynamics.com/FreeJavaPerformanceDownload.html

> _______________________________________________
> Vimprobable-users mailing list
> Vim...@li...
> https://lists.sourceforge.net/lists/listinfo/vimprobable-users

[Vimprobable-users] Editor patch evolution

[Daniel C. <dan...@gm...>]

Hannes Schüller <ha...@yl...> wrote:
> This would imply a jump in the required Webkit version. Currently, the
> browser still compiles fine witih 1.2.x.
> 
> Hannes

That's a reason. I thinks I should have a look in the right version of webkit
before hacking a patch:)

Daniel

Re: [Vimprobable-users] Editor patch evolution

[Hannes S. <ha...@yl...>]

Hi!

Daniel Carl <dan...@gm...> wrote:
> I'm not sure about the consequences, but I think it would be a better
> approach to use webkit functions like
> webkit_dom_html_text_area_element_set_value() instead of evaluating
> JavaScript. I suppose that using the c functions direct could be a
> little faster.

This would imply a jump in the required Webkit version. Currently, the
browser still compiles fine witih 1.2.x.

Hannes

[Vimprobable-users] Editor patch evolution

[Daniel C. <dan...@gm...>]

Hi Markus!

This patch works fine for me too.
I'm not sure about the consequences, but I think it would be a better approach
to use webkit functions like webkit_dom_html_text_area_element_set_value()
instead of evaluating JavaScript. I suppose that using the c functions direct
could be a little faster.

I attached a patch to show what I mean. This is only a sample, but it should
be also possible to remove jsapi calls from _resume_from_editor() too.

Daniel

[Vimprobable-users] [PATCH] Decreased COMMANDSIZE

[Daniel C. <dan...@gm...>]

Hi!

I'm not sure, but it seems the constant COMMANDSIZE should not need to be
1024. At the moment we have only 41 Commands and I think we don't put other
commands on runtime into the array.

So I decreased the value to 41 to have a smaller binary.

Daniel

Re: [Vimprobable-users] Editor patch evolution

[Jason R. <jas...@gm...>]

On 28/09/12 at 07:06pm, Markus Demleitner wrote:
> Hi,
> 
> During a longish train ride today, I polished up the editor patch a
> bit.  For one, the editor now runs asynchronously, so vimprobable
> doesn't just sit there as long as the editor runs.  To keep people
> from futzing around in the original text field, it is now grayed out
> while the editor is active.
> 
Once again, thanks Markus - works as advertised!

The greyed-out text box is a nice touch.

Cheers,

/J  

-- 

http://jasonwryan.com/  [GnuPG Key: B1BD4E40]

Re: [Vimprobable-users] Editor patch, revised

[Markus D. <msd...@ar...>]

Hi Hannes,

On Fri, Sep 28, 2012 at 05:48:34PM +0200, Hannes Schüller wrote:
> This is exactly the problem I've sketched in the conclusion post of the
> old thread: The symmetric spawn operation which I initially wrote is
> probably not suited for quite a number of editors which will return
> after the opening operation immediately. Hence my plan/suggestion to
> use an asymmetric operation and watch the file for changes through
> another process instead. Which, of course, would be much more complex
> to implement.
> 
> The major question for me here (which I think should be decided before
> any further coding) is whether we do need asymmetric operation or not.
> If not, we will need to document this in the wiki as well as possible
> which editors will work and which won't.

My opinion on this is that our most likely use cases are vim -gf or
an xterm -e <whatever>; both of them work fine, so I'd guess we're
way beyond 80/20 with what we have.

While I'd guess watching the file would be fairly easy after the
patches I just sent around (I suspect it would just be an additional
watch), it results in IMHO unresolvable functional trouble -- I think
we agree that only one "head" can edit the text at a time.  Thus, we
need to lock the browser text box while the editor "runs" and release
the lock when it exits.  By just looking at the temp file, we can't
tell when that happens, and if the editor itself detaches, we don't
know its pid either.

So, yes, I'd say what doesn't work (vim -g, urxvtc -- which is not
your average terminal emulator anyway --, what else?) on the wiki and
keep things more or less as they are.

Cheers, 

      Markus

[Vimprobable-users] Editor patch evolution

[Markus D. <msd...@ar...>]

Hi,

During a longish train ride today, I polished up the editor patch a
bit.  For one, the editor now runs asynchronously, so vimprobable
doesn't just sit there as long as the editor runs.  To keep people
from futzing around in the original text field, it is now grayed out
while the editor is active.

I've also fixed the editing behaviour in the presence of " characters
in the content.  I'm not quite happy with this, though -- javascript
escaping must be more complex than this.  Can anyone point to a
comprehensive and authoritative answer as to how javascript expects
its string liteals?

Attached are two patches; one should apply on top of the patch
yesterday, the other should apply on a clean 1.1.1 release.

Any feedback is welcome.  Is there anyone I should my git
format-patch to assuming people like this?


Cheers,

      Markus

Re: [Vimprobable-users] Editor patch, revised

[Hannes S. <ha...@yl...>]

Hi,

thanks, Markus, for this patch.

Jason Ryan <jas...@gm...> wrote:
> > I've taken Hannes' patch for the external editor and changed it to
> > emply the handler mechanism.  Basically, you say
> > 
> > handler vimprobableedit vim -gf %s
> > 
> > in your vimprobablerc, and when hitting ^t in a text field, you'll
> > get a gvim window.
> >   
> I don't have gvim installed, so I used:
> handler vimprobableedit urxvtc -e vim %s
> 
> and the term opens, I compose my message and then, when I exit the
> term, the file exists in /tmp/vimprobableXXXX but nothing is printed
> to the text field in vimprobable.

This is exactly the problem I've sketched in the conclusion post of the
old thread: The symmetric spawn operation which I initially wrote is
probably not suited for quite a number of editors which will return
after the opening operation immediately. Hence my plan/suggestion to
use an asymmetric operation and watch the file for changes through
another process instead. Which, of course, would be much more complex
to implement.

The major question for me here (which I think should be decided before
any further coding) is whether we do need asymmetric operation or not.
If not, we will need to document this in the wiki as well as possible
which editors will work and which won't.

Hannes

Re: [Vimprobable-users] Editor patch, revised

[Jason R. <jas...@gm...>]

On 28/09/12 at 10:19am, Markus Demleitner wrote:
> Hi Jason,
> 
> On Fri, Sep 28, 2012 at 07:30:12PM +1200, Jason Ryan wrote:
> > On 27/09/12 at 10:55pm, Markus Demleitner wrote:
> > > handler vimprobableedit vim -gf %s
> > > 
> > > in your vimprobablerc, and when hitting ^t in a text field, you'll
> > > get a gvim window.
> > > 
> > I don't have gvim installed, so I used:
> > handler vimprobableedit urxvtc -e vim %s
> > 
> > and the term opens, I compose my message and then, when I exit the term,
> > the file exists in /tmp/vimprobableXXXX but nothing is printed to the
> > text field in vimprobable.
> Well, that's because urxvtc exits immedately after it's contacted the
> daemon.  vimprobable thinks all is fine, reads back the (unchanged)
> data and decides all is dandy.
> 
> Try 
> 
> handler vimprobableedit urxvt -e vim %s
> 
> and things should work.  I don't think there's a good way to catch
> these kinds of instant-fork problems without ugly heuristics (warn when
> the child process took less than 100 ms? 1000 ms? 5000 ms? Meh).

That works nicely, thanks Markus.  

> While I'm thinking about that: I'd like to give visual feedback that
> the edit field is "off limits" while the external editor is active.
> I'd try that with a few lines of javascript fed to webkit (switch the
> element to "disabled", maybe), but if anyone has a better idea, I'd
> appreciate that.
> 
> > > If people are happy with the way this works, I'd go ahead and
> > > document it (plus probable bug fixing); I guess the vim -gf shold be
> > > a builtin default, shouldn't it?
> > > 
> > Wouldn't ${EDITOR:-vi} be more appropriate? Just a thought???
> The trouble with EDITOR is that it'll usually be a console editor
> (not long ago there were systems that defaulted to EDITOR=ed and
> VISUAL=vi...).  If I'm spawning a console editor, things will really
> be confusing, and default confusing is not something I'd like to
> build in.
> 
> So, given the vimprobable's target audience I'd say we have a choice
> between no default and vim -gf.
  
The default mail handler that ships is mutt, so maybe the audience will be OK
with a console editor :)

Thanks again for the work on this patch: it is much appreciated!

Cheers,

/J

-- 

http://jasonwryan.com/  [GnuPG Key: B1BD4E40]

Re: [Vimprobable-users] Editor patch, revised

[Markus D. <msd...@ar...>]

Hi Jason,

On Fri, Sep 28, 2012 at 07:30:12PM +1200, Jason Ryan wrote:
> On 27/09/12 at 10:55pm, Markus Demleitner wrote:
> > handler vimprobableedit vim -gf %s
> > 
> > in your vimprobablerc, and when hitting ^t in a text field, you'll
> > get a gvim window.
> > 
> I don't have gvim installed, so I used:
> handler vimprobableedit urxvtc -e vim %s
> 
> and the term opens, I compose my message and then, when I exit the term,
> the file exists in /tmp/vimprobableXXXX but nothing is printed to the
> text field in vimprobable.
Well, that's because urxvtc exits immedately after it's contacted the
daemon.  vimprobable thinks all is fine, reads back the (unchanged)
data and decides all is dandy.

Try 

handler vimprobableedit urxvt -e vim %s

and things should work.  I don't think there's a good way to catch
these kinds of instant-fork problems without ugly heuristics (warn when
the child process took less than 100 ms? 1000 ms? 5000 ms? Meh).

While I'm thinking about that: I'd like to give visual feedback that
the edit field is "off limits" while the external editor is active.
I'd try that with a few lines of javascript fed to webkit (switch the
element to "disabled", maybe), but if anyone has a better idea, I'd
appreciate that.

> > If people are happy with the way this works, I'd go ahead and
> > document it (plus probable bug fixing); I guess the vim -gf shold be
> > a builtin default, shouldn't it?
> > 
> Wouldn't ${EDITOR:-vi} be more appropriate? Just a thought???
The trouble with EDITOR is that it'll usually be a console editor
(not long ago there were systems that defaulted to EDITOR=ed and
VISUAL=vi...).  If I'm spawning a console editor, things will really
be confusing, and default confusing is not something I'd like to
build in.

So, given the vimprobable's target audience I'd say we have a choice
between no default and vim -gf.

Cheers,

        Markus

Re: [Vimprobable-users] Editor patch, revised

[Jason R. <jas...@gm...>]

On 27/09/12 at 10:55pm, Markus Demleitner wrote:
> Hi,
Hi Markus - thanks for the patch.
> 
> I've taken Hannes' patch for the external editor and changed it to
> emply the handler mechanism.  Basically, you say
> 
> handler vimprobableedit vim -gf %s
> 
> in your vimprobablerc, and when hitting ^t in a text field, you'll
> get a gvim window.
> 
I don't have gvim installed, so I used:
handler vimprobableedit urxvtc -e vim %s

and the term opens, I compose my message and then, when I exit the term,
the file exists in /tmp/vimprobableXXXX but nothing is printed to the
text field in vimprobable.

> This also fixes a bug in which the ":" protocol separator did not get
> stripped from the argument in open_handler (but frankly, that code is
> extremely spooky -- does anyone actually understand what all the temp
> magic is supposed to do?).
> 
> One "architectural" question: I'm potentially emitting quit a bit of
> diagnostics -- is give_feedback the right function for this?  Should
> I do something else yet?
> 
> If people are happy with the way this works, I'd go ahead and
> document it (plus probable bug fixing); I guess the vim -gf shold be
> a builtin default, shouldn't it?
> 
Wouldn't ${EDITOR:-vi} be more appropriate? Just a thought…
  
Cheers,

/J

-- 

http://jasonwryan.com/  [GnuPG Key: B1BD4E40]

[Vimprobable-users] Editor patch, revised

[Markus D. <msd...@ar...>]

Hi,

I've taken Hannes' patch for the external editor and changed it to
emply the handler mechanism.  Basically, you say

handler vimprobableedit vim -gf %s

in your vimprobablerc, and when hitting ^t in a text field, you'll
get a gvim window.

This also fixes a bug in which the ":" protocol separator did not get
stripped from the argument in open_handler (but frankly, that code is
extremely spooky -- does anyone actually understand what all the temp
magic is supposed to do?).

One "architectural" question: I'm potentially emitting quit a bit of
diagnostics -- is give_feedback the right function for this?  Should
I do something else yet?

If people are happy with the way this works, I'd go ahead and
document it (plus probable bug fixing); I guess the vim -gf shold be
a builtin default, shouldn't it?


While I'm here:  I'd like to add a toggle syntax for boolean flags,
probably along the lines of vim{perator}'s set inv{option}; this is
to support stuff like

map <C-S> set invscripts

or so.  Would anyone consider this feature bloat?


Even more featurecreepy would be to have indicators of script and
proxy in the status line (little S and P chars at the very right, I'd
guess).  Are their any strong feelings about that?

Cheers,

         Markus

[Vimprobable-users] [PATCH] Allow loading of user javascripts

[Daniel C. <dan...@gm...>]

> I use also attached script file to block flash on all pages and to enable it
> via mouseclick (script is from another brower project - but I can't find it
> anymore).

The script cames from the uzbl project http://www.uzbl.org/wiki/flashblock

Daniel

[Vimprobable-users] [PATCH] Allow loading of user javascripts

[Daniel C. <dan...@gm...>]

Hi!

I asked some times ago for the possibility to run userdefinded javascripts on
every page load.

Here is a patch to do this. If someone else is interested in it I will updated
the manpages too.

I use also attached script file to block flash on all pages and to enable it
via mouseclick (script is from another brower project - but I can't find it
anymore).

Daniel

Re: [Vimprobable-users] [Ticket #9] Edit text areas using $EDITOR

[Daniel C. <dan...@gm...>]

Hi!

Hannes Schüller <ha...@yl...> wrote:
> Agreed. My plan for this patch is this:
> 
> - Have the editor command defined in a way like the external protocol
>   handlers are (whether this will need to be run through :set will need
>   to be decided; I'd say it's a fairly static setting).

I think it's nearly a static setting too. But in my opinion it's not a
performance issue to have the ability to set it to the runtime via :set.
Implementing it in as via :set, should be easier to implement, because we
don't need to write more code and have not to introduce a new complicated way
to set personal settings.

I think to make this changeable via :set is also a good choice even if the
setting is static (one time declared in vimprobablerc) and a default value via
config.h

Daniel

[Vimprobable-users] [PATCH] Fix keybinding for cancel load in manpage

[Daniel C. <dan...@gm...>]

Hi!

Very small patch for the vimprobable manpage.
Daniel

[Vimprobable-users] [PATCH] Fix for none printed messages on commands called from inputbox

[Daniel C. <dan...@gm...>]

Hi!

In the current master the error messages aren't printed if there where called
unknown commands from inputbox. This patch removes the focus from inputbox
after a command was called to allow the echo function to print the messages to
the inputbox.
I haven't tested this patch much, so I hope someone else can have a look at
it.

Daniel

[Vimprobable-users] [PATCH] New function echo_message()

[Daniel C. <dan...@gm...>]

Hi!

The attached patch removes the give_feedback function by a new a litte more
flexible echo_message function. The give_feedback function was nearly unused
in the code. But I often found code like

Arg a;
a.i = Info;
a.s = g_strdup_printf("Message %s", message);
echo(&a);
g_free(a.s);

so I removed this code by a call to the new function too. This makes the code
a little smaller and also the compiled binary (on my machine by 3% or 4066
bytes). I'm not sure if I forgot some calls to free, but the check with
valgrind caused nearly the same output like the vimprobable version from
current master, so I'm confident that the patch works.

Comments are welcome!

Daniel

Re: [Vimprobable-users] [PATCH] Command to open inspector

[Hannes S. <ha...@yl...>]

Hi!

Daniel Carl <dan...@gm...> wrote:
> I added the command to open the web instector by :inspect so there is
> no need to use right mouse click to open it. Maybe this patch make no
> sense for the most users, but it introduces no big overhead, so I
> think it could be useful for others too.

I think this patch makes a lot of sense. I will merge it into the next
release.

Thank you, Daniel!

Hannes

[Vimprobable-users] [PATCH] Command to open inspector

[Daniel C. <dan...@gm...>]

Hi!

I added the command to open the web instector by :inspect so there is no need
to use right mouse click to open it. Maybe this patch make no sense for the
most users, but it introduces no big overhead, so I think it could be useful
for others too.

Daniel