[geary/bug/728002-webkit2: 56/140] Minor code cleanup.

[Michael Gratton <mjog src gnome org>]

commit 977bb2827a7a3deffed1faefb13edd406e0164e4
Author: Michael James Gratton <mike vee net>
Date:   Mon Jan 2 10:51:26 2017 +1100

    Minor code cleanup.

 src/client/composer/composer-web-view.vala |   32 +++++++++++++++++++++++----
 test/js/composer-page-state-test.vala      |   20 ++++++++--------
 ui/composer-web-view.js                    |   12 +++++-----
 3 files changed, 43 insertions(+), 21 deletions(-)
---
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 22de6bd..2b1e3ae 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -211,6 +211,9 @@ public class ComposerWebView : ClientWebView {
             "geary.getText();", null
         );
 
+        const int MAX_BREAKABLE_LEN = 72; // F=F recommended line limit
+        const int MAX_UNBREAKABLE_LEN = 998; // SMTP line limit
+
         string body_text = WebKitUtil.to_string(result);
         string[] lines = body_text.split("\n");
         GLib.StringBuilder flowed = new GLib.StringBuilder.sized(body_text.length);
@@ -220,33 +223,52 @@ public class ComposerWebView : ClientWebView {
             // special, so leave that alone.
             if (line != "-- ")
                 line = line.chomp();
+
+            // Determine quoting depth by counting the the number of
+            // QUOTE_MARKERs present, and build a quote prefix for it.
             int quote_level = 0;
             while (line[quote_level] == Geary.RFC822.Utils.QUOTE_MARKER)
                 quote_level += 1;
             line = line[quote_level:line.length];
             string prefix = quote_level > 0 ? string.nfill(quote_level, '>') + " " : "";
-            int max_len = 72 - prefix.length;
 
+            // Check to see if the line (with quote prefix) is longer
+            // than the recommended limit, if so work out where to do
+            int max_breakable = MAX_BREAKABLE_LEN - prefix.length;
+            int max_unbreakable = MAX_UNBREAKABLE_LEN - prefix.length;
             do {
                 int start_ind = 0;
+
+                // Space stuff if needed
                 if (quote_level == 0 &&
                     (line.has_prefix(">") || line.has_prefix("From"))) {
                     line = " " + line;
                     start_ind = 1;
                 }
 
+                // Check to see if we need to break the line, if so
+                // determine where to do it.
                 int cut_ind = line.length;
-                if (cut_ind > max_len) {
-                    string beg = line[0:max_len];
+                if (cut_ind > max_breakable) {
+                    // Line needs to be broken, look for the last
+                    // useful place to break before before the
+                    // max recommended length.
+                    string beg = line[0:max_breakable];
                     cut_ind = beg.last_index_of(" ", start_ind) + 1;
                     if (cut_ind == 0) {
+                        // No natural places to break found, so look
+                        // for place further along, and if that is
+                        // also not found then break on the SMTP max
+                        // line length.
                         cut_ind = line.index_of(" ", start_ind) + 1;
                         if (cut_ind == 0)
                             cut_ind = line.length;
-                        if (cut_ind > 998 - prefix.length)
-                            cut_ind = 998 - prefix.length;
+                        if (cut_ind > max_unbreakable)
+                            cut_ind = max_unbreakable;
                     }
                 }
+
+                // Actually break the line
                 flowed.append(prefix + line[0:cut_ind] + "\n");
                 line = line[cut_ind:line.length];
             } while (line.length > 0);
diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala
index 433d4ba..e23325b 100644
--- a/test/js/composer-page-state-test.vala
+++ b/test/js/composer-page-state-test.vala
@@ -24,10 +24,10 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
         try {
             assert(run_javascript(@"window.geary.getHtml();") == html + "<br><br>");
         } catch (Geary.JS.Error err) {
-            print("Geary.JS.Error: %s", err.message);
+            print("Geary.JS.Error: %s\n", err.message);
             assert_not_reached();
         } catch (Error err) {
-            print("WKError: %s", err.message);
+            print("WKError: %s\n", err.message);
             assert_not_reached();
         }
     }
@@ -37,10 +37,10 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
         try {
             assert(run_javascript(@"window.geary.getText();") == "para\n\n\n\n");
         } catch (Geary.JS.Error err) {
-            print("Geary.JS.Error: %s", err.message);
+            print("Geary.JS.Error: %s\n", err.message);
             assert_not_reached();
         } catch (Error err) {
-            print("WKError: %s", err.message);
+            print("WKError: %s\n", err.message);
             assert_not_reached();
         }
     }
@@ -67,10 +67,10 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
             assert(run_javascript(@"window.geary.getText();") ==
                    
@"pre\n\n$(q_marker)quote1\n$(q_marker)\n$(q_marker)$(q_marker)quote2\n$(q_marker)$(q_marker)\npost\n\n\n\n");
         } catch (Geary.JS.Error err) {
-            print("Geary.JS.Error: %s", err.message);
+            print("Geary.JS.Error: %s\n", err.message);
             assert_not_reached();
         } catch (Error err) {
-            print("WKError: %s", err.message);
+            print("WKError: %s\n", err.message);
             assert_not_reached();
         }
     }
@@ -101,10 +101,10 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
             assert(run_javascript(@"ComposerPageState.resolveNesting('$(js_cosy_quote2)', $(js_values));") ==
                    @"foo\n$(q_marker)quote1\n$(q_marker)quote2\nbar");
         } catch (Geary.JS.Error err) {
-            print("Geary.JS.Error: %s", err.message);
+            print("Geary.JS.Error: %s\n", err.message);
             assert_not_reached();
         } catch (Error err) {
-            print("WKError: %s", err.message);
+            print("WKError: %s\n", err.message);
             assert_not_reached();
         }
     }
@@ -120,10 +120,10 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
             assert(run_javascript("ComposerPageState.quoteLines('line1\\nline2');") ==
                    @"$(q_marker)line1\n$(q_marker)line2");
         } catch (Geary.JS.Error err) {
-            print("Geary.JS.Error: %s", err.message);
+            print("Geary.JS.Error: %s\n", err.message);
             assert_not_reached();
         } catch (Error err) {
-            print("WKError: %s", err.message);
+            print("WKError: %s\n", err.message);
             assert_not_reached();
         }
     }
diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js
index 26e16f9..e48090d 100644
--- a/ui/composer-web-view.js
+++ b/ui/composer-web-view.js
@@ -22,7 +22,7 @@ ComposerPageState.prototype = {
     init: function() {
         PageState.prototype.init.apply(this, []);
 
-        var state = this;
+        let state = this;
         document.addEventListener("click", function(e) {
             if (e.target.tagName == "A") {
                 state.linkClicked(e.target);
@@ -33,9 +33,9 @@ ComposerPageState.prototype = {
         // Search for and remove a particular styling when we quote
         // text. If that style exists in the quoted text, we alter it
         // slightly so we don't mess with it later.
-        var nodeList = document.querySelectorAll(
+        let nodeList = document.querySelectorAll(
             "blockquote[style=\"margin: 0 0 0 40px; border: none; padding: 0px;\"]");
-        for (var i = 0; i < nodeList.length; ++i) {
+        for (let i = 0; i < nodeList.length; ++i) {
             nodeList.item(i).setAttribute(
                 "style",
                 "margin: 0 0 0 40px; padding: 0px; border:none;"
@@ -46,13 +46,13 @@ ComposerPageState.prototype = {
         document.body.focus();
 
         // Set cursor at appropriate position
-        var cursor = document.getElementById("cursormarker");
+        let cursor = document.getElementById("cursormarker");
         if (cursor != null) {
-            var range = document.createRange();
+            let range = document.createRange();
             range.selectNodeContents(cursor);
             range.collapse(false);
 
-            var selection = window.getSelection();
+            let selection = window.getSelection();
             selection.removeAllRanges();
             selection.addRange(range);
             cursor.parentNode.removeChild(cursor);