Quoting Markus Demleitner ([email protected]):
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/vimprobable-users
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/vimprobable-users
