Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various cleanups while tracking down an issue #186

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions fancy-settings/source/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ this.i18n = {
"en": "Edit with Emacs is an extension that allows you to edit textareas and other editable text elements\
of a web-page with your favourite editor. For this to work you need to be running an \"edit server\"\
on your local machine. This is because extensions in Chrome(ium) cannot directly start new programs. For Emacs users \
it is recommended you use the use supplied native <a href=\"/servers/edit-server.el\">edit-server.el</a>.\
Alternativley you can track the latest version through the <a href=\"http://melpa.milkbox.net/\">MELPA</a>\
it is recommended you use the supplied native <a href=\"/servers/edit-server.el\">edit-server.el</a>.\
Alternativeley you can track the latest version through the <a href=\"http://melpa.milkbox.net/\">MELPA</a>\
package archive.\
</p>\
<p>\
Save the file to somewhere visible to your your Emacs <code>load-path</code> (~/.emacs.d/lisp is popular) and add the following\
Save the file to somewhere visible to your Emacs <code>load-path</code> (~/.emacs.d/lisp is popular) and add the following\
to your <code>.emacs</code>:\
</p>\
<pre>\
Expand Down
15 changes: 9 additions & 6 deletions javascript/context_menu.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
/* -*- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
(function(){

var edit_msg = {};
var edit_msg = null;

function menuClicked(info, tab) {
if (edit_msg) {
var tab_port = chrome.tabs.connect(tab.id);
edit_msg.pageUrl = info.pageUrl;
console.debug("menuClicked with edit_msg:", edit_msg);
handleContentMessages(edit_msg, tab_port);
edit_msg = null;
} else {
console.error("menuClicked called while edit_msg is null");
}
}


var menu_enabled = false;
var menu_is_installed = false;

function enableContextMenu() {
if (!menu_enabled) {
if (!menu_is_installed) {
chrome.contextMenus.removeAll();
chrome.contextMenus.create({
title: "Edit with Emacs",
Expand All @@ -24,13 +27,13 @@
menuClicked(info, tab);
}
});
menu_enabled = true;
menu_is_installed = true;
}
}

function disableContextMenu() {
chrome.contextMenus.removeAll();
menu_enabled = false;
menu_is_installed = false;
}

// Initialize the context menu based on stored options.
Expand Down
7 changes: 7 additions & 0 deletions javascript/textareas.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ function sendTextArea(text_tracker) {
edit_msg.msg = "edit";
edit_msg.text = text_tracker.getContent();
edit_msg.id = text_tracker.edit_id;
console.debug("sendTextArea: sending message:", edit_msg);
port.postMessage(edit_msg);
}

Expand Down Expand Up @@ -429,17 +430,21 @@ browser_runtime.onConnect.addListener(function(iport) {
iport.onMessage.addListener(localMessageHandler);
});

console.debug("About to fetch our configuration from the background process");

/*
To start the whole process off we first need to fetch our configuration
from the background process.
*/
port.postMessage({msg: "config"});

console.debug("About to install contextmenu event listener on document");

// Inform the background process whenever the user opens
// the context menu on an editable element.
document.addEventListener("contextmenu", (function(event) {
var elem = event.srcElement;
console.debug("contextmenu event called on elem: ", elem);
if (elem && elem.getAttribute("edit_id")) {
var edit_msg = getEmptyMessage();
edit_msg.msg = "edit";
Expand All @@ -452,3 +457,5 @@ document.addEventListener("contextmenu", (function(event) {
browser_sendMessage(request);
}
}));

console.debug("Edit with Emacs has finished initializing");
3 changes: 2 additions & 1 deletion javascript/xmlcomms.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ chrome.commands.onCommand.addListener(function(command) {
// Package up the text to be edited and send it to the edit server
function handleContentMessages(msg, tab_port)
{
console.log("handleContentMessages called:"+JSON.stringify(msg));
console.debug("handleContentMessages called:", msg);
console.assert(msg.text);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the user visible effect here? Besides you can have empty text areas so what are we trying to catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty text areas have an empty string as their content. this is checking for null.

it happens when the contextmenu event listener is not triggered, and the message contains no text. (or worse: with HEAD it uses the previous content)

the user visible effect is that nothing happens, besides the log in the console. not ideal, but IMO better than the previous behavior. at least people with knowledge can hone in much faster on the actual issue.

also, i'm suggesting to get rid of the current console.log stuff, and use console.debug where appropriate, and let the users decide in their console config how much log they want to see.

it took me hours to understand what was happening. with these changes it should be minutes at most.

var cmd = msg.msg;
var id = msg.id;
var text = msg.text;
Expand Down
6 changes: 5 additions & 1 deletion servers/edit-server.el
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ non-nil, then STRING is also echoed to the message line."
;; data in the buffer and process it in different phases, which
;; requires us to keep track of the processing state.
(with-current-buffer (process-buffer proc)
(edit-server-log proc "Got HTTP request string of length `%s'" (length string))
(insert string)
(setq edit-server-received
(+ edit-server-received (string-bytes string)))
Expand All @@ -415,6 +416,7 @@ non-nil, then STRING is also echoed to the message line."
(save-excursion
(goto-char (point-min))
(when (re-search-forward "^Content-Length:\\s-+\\([0-9]+\\)" nil t)
(edit-server-log proc "Found Content-Length: %s" (match-string 1))
(setq edit-server-content-length
(string-to-number (match-string 1)))))
;; look for "x-url" header
Expand All @@ -434,6 +436,7 @@ non-nil, then STRING is also echoed to the message line."
(when (re-search-forward "\\(\r?\n\\)\\{2\\}" nil t)
;; HTTP headers are pure ASCII (1 char = 1 byte), so we can subtract
;; the buffer position from the count of received bytes
(edit-server-log proc "Found body offset at: %s" (match-end 0))
(setq edit-server-received
(- edit-server-received (- (match-end 0) (point-min))))
;; discard headers - keep only HTTP content in buffer
Expand Down Expand Up @@ -540,7 +543,8 @@ and save the network process for the final call back"
(when existing-buffer
(kill-ring-save (point-min) (point-max)))

(edit-server-log proc "copying new data into buffer")
(edit-server-log proc "copying new data into buffer, length: %s"
(- (point-max) (point-min)))
(copy-to-buffer buffer (point-min) (point-max))

(with-current-buffer buffer
Expand Down