opto / nostalgy-xpi

nostalgy addon updated for Thunderbird 68 and now Thunderbird 78 - 115. Original code by Alain Frisch.
Other
119 stars 9 forks source link

save command with immediate return saves message in undesirable place #198

Open gwhitney opened 1 year ago

gwhitney commented 1 year ago

In nostalgy 3.6.11, invoking the save command and then immediately hitting return ends up moving the message, but it was very difficult to figure out where the message was going. (It turns out it was the first match for the empty pattern, which depends on things like the order of your accounts, the names of the folders in whatever is the first account, etc. -- so not very useful.)

In the history of Nostalgy, at some point perhaps in one of Alain Frisch's versions or one of yours, invoking the save command and then immediately hitting return simply saved the message in the same place as the last save. That was super useful, and it got pretty hard-wired into my fingers (so returning to Thunderbird a few years later, I find I am still doing it). So I'm filing this issue as a request to restore that behavior instead of the current non-useful behavior.

Better yet, here is a patch below on 3.6.11 as shipped that implements the requested change. I am not filing this as a PR because (a) this repository is clearly not in sync with the releases, so it's a bit hard to know how to make an appropriate PR, and (b) this version uses the placeholder text to show where the message will go if you just hit return. That makes the placeholder text pretty long. Better would be if the folder list popup came up showing just that one folder, the most recent used. But I was not familiar enough with the thunderbird toolkit being used to get that to happen -- I got the NostalgyGetAutoCompleteValues function to be returning an array of that one string when initially called with the empty string, but that unfortunately did not make the popup show (well, it seemed to be posted, but with very little height and not showing anything). But feel free to use the patch as supplied if you think using the placeholder is OK, or let me know if there is any other way I can help restore the old "immediate return saves in same folder as last time" behavior. Thanks!

diff -u -r nostalgy.orig/chrome/content/folders.js nostalgy/chrome/content/folders.js
--- nostalgy.orig/chrome/content/folders.js 2022-06-24 19:35:06.000000000 -0400
+++ nostalgy/chrome/content/folders.js  2022-11-18 16:28:32.803432256 -0500
@@ -340,12 +340,19 @@

 // Resolve a string coming from a completion box
 // 1. check whether uri comes from the completion list (cropped exact uri)
-// 2. if not, assume the uri has been typed in by the user
+// 2. If not, if the uri is empty, revert to the most recent folder
+// 3. Otherwise, assume the uri has been typed in by the user
 //    and take the first matching folder

 function NostalgyResolveFolder(uri) {
   var ret = NostalgyFindFolderCropped(uri);
-  if (ret) { return ret; } else { return (NostalgyFirstCompletion(uri)); }
+  if (ret) { return ret; }
+  if ((uri === '' || uri === ' ') && nostalgy_recent_folders.length) { 
+    ret = NostalgyFindFolderExact(
+      nostalgy_recent_folders[nostalgy_recent_folders.length - 1])
+    if (ret) { return ret; }
+  }
+  return NostalgyFirstCompletion(uri);
 }

 function NostalgyFirstCompletion(uri) {
diff -u -r nostalgy.orig/chrome/content/nostalgy.js nostalgy/chrome/content/nostalgy.js
--- nostalgy.orig/chrome/content/nostalgy.js    2022-06-24 20:06:30.000000000 -0400
+++ nostalgy/chrome/content/nostalgy.js 2022-11-18 16:36:48.735992322 -0500
@@ -537,7 +537,13 @@
   if (!nostalgy_focus_saved) { nostalgy_focus_saved = NostalgyEBI("messagepane").contentWindow; }

   if (!nostalgy_in_message_window) TB_statusTextBox.hidden = true;
-  nostalgy_folderBox.setAttribute("placeholder", lab + ",  'space' shows history");
+  var placeholder = lab
+  if (nostalgy_recent_folders.length) {
+    placeholder += ' '
+    placeholder += nostalgy_recent_folders[nostalgy_recent_folders.length - 1];
+  }
+  placeholder += ' [SPC shows history]'
+  nostalgy_folderBox.setAttribute("placeholder", placeholder)

   nostalgy_search_folder_options.require_file = require_file;
   nostalgy_cmdLabel.value = "";// lab;
salzrat commented 1 year ago

Actually I was wondering about the behavior for some time. I remember in a previous version, a history was always shown, and just hitting "enter" would then save it to the first shown folder, which I found great. Later, this didn't work anymore and I couldn't figure out where it went.

Now, I just see a "space shows history", and it's completely unclear where the message might end up. This is indeed not useful, and i'd like to have back the behavior that the history is immediately shown and the first item is chosen upon "Enter".

To get the behavior you want, you can just press "S" (capital s) instead of "s".

gwhitney commented 1 year ago

Now, I just see a "space shows history", and it's completely unclear where the message might end up.

Invoking save and hitting space works just fine for me in 3.6.11 -- all of the most recent folders pop up in the usual completions box and I can cycle through them, just like the behavior you are describing you want. Except for me then hitting enter after the space doesn't necessarily work, I have to hit up arrow or tab and then enter. So it is several keystrokes to get to the last used folder.

To get the behavior you want, you can just press "S" (capital s) instead of "s".

For me, that saves the message to the "predicted" folder, which is useful in addition. But sometimes the prediction is not correct, and when I want to go through a bunch of messages and put them all in the same folder, it is convenient to have a sure-fire way to save to the last used folder. That's why I like invoking save and just hitting enter to guarantee that it will go to the same folder as the last save.

I am sure that opto could end up with some behavior here that would satisfy both of us; hopefully the patch above is helpful. And in the meantime does it not work for you to invoke save, then hit space, and then cycle through the options with the arrow keys/tab key?

salzrat commented 1 year ago

I wonder what the "suggested" folder actually is. If you turn off "use statistical prediction" and "suggest folder of related conversation", I assume it does resort to using the last used folder, but maybe @opto can clarify? The target folder for "shift" is also shown in the right side of the status bar.

Currently, it's inconvenient that nothing at all is shown when pressing "s". Having to click an extra space seems inconvenient, so I usually end up typing the beginning of the folder name again.

gwhitney commented 1 year ago

wonder what the "suggested" folder actually is

Not sure what it would be if you turned off both of the features you mention but for folks who find them useful, "suggested folder" is different from "most recent folder" and it is handy to have quick ways to get each.

Currently, it's inconvenient that nothing at all is shown when pressing "s

I completely agree. As I mentioned, I tried for a long time to get the most recent folder to show up in the usual completion space immediately when save is invoked. If I had succeeded, I'd be happy to change it to put the whole recent history list up but with the last used primed for selection with "enter". Part of the difficulty I encountered is that the code for the completion box isn't in the extension; it's apparently a Thunderbird extension to input fields, and in fact it seems like it's legacy/deprecated -- I couldn't find very clear current documentation on it. I am guessing opto encountered similar difficulties, explaining why he made the recent history post by invoking save and then hitting space. So the only place I had to show the most recent was the placeholder, where there's only room for the single most recent.

salzrat commented 1 year ago

I have the statistical prediction turned off and for me it mostly works.

Indeed, showing the history (maybe limited to a certain number of entries, like 4) of most recent moves, with the most recent one first, immediately on pressing "s" would satisfy us both :) And what exactly happens on shift-s should also be clearly documented...

gwhitney commented 1 year ago

OK, well at the end of this comment is a minimal, hackish patch to 3.6.11 that does exactly as @salzrat requests: almost immediately after save is invoked, the popup comes up with the "history" (which may include suggestions, depending on the settings). I say "hackish" because it works simply by faking a space input to the folder search box, but as all of the other functionality of that box is coded to ignore an initial space, it works pretty well. One important caveat is that if you invoke save and then hit enter before the autocomplete has a chance to finish and post the "history" (definitely possible if you are typing quickly), then nostalgy reverts to its behavior for specifying the empty string as the folder, which is the non-useful behavior mentioned at the top of this issue. So shortly I will post a third patch which combines this one with the above fix for empty folder input, but also reorders the "history" popup so that the most recently used folder is always at the top, so that the behaviors will be consistent. If there are "suggestions" besides the history, I will put them at the bottom where they are easily accessible with up arrow.

Patch per @salzrat's request:

diff -u -r nostalgy.orig/chrome/content/nostalgy.js round3Nostalgy/chrome/content/nostalgy.js
--- nostalgy.orig/chrome/content/nostalgy.js    2022-06-24 20:06:30.000000000 -0400
+++ round3Nostalgy/chrome/content/nostalgy.js   2022-11-19 13:28:08.786469308 -0500
@@ -589,7 +589,10 @@
     //    box.popup.setAttribute("nomaxresults", "true");
     //   box.popup.setAttribute("_normalMaxRows", "8");
     //     box.popup.maxResults =function () {return 8};
-    listener = box.controller.QueryInterface(Components.interfaces.nsIAutoCompleteObserver);
+    // listener = box.controller.QueryInterface(Components.interfaces.nsIAutoCompleteObserver);
+    box.controller.resetInternalState();
+    box.value = ' ';
+    box.controller.handleText();
   }
   else { // XPFE
     // box.mAutoCompleteObserver uses a flawed equality check so we have to replace it.
@@ -601,9 +604,9 @@
     };
     // Reset internal state
     box.currentSearchString = "";
+    NostalgyAutocompleteComponent().startSearch("", box.searchParam, null, listener);
   }

-  NostalgyAutocompleteComponent().startSearch("", box.searchParam, null, listener);
 }

 function NostalgyCreateTag(name) {
gwhitney commented 1 year ago

OK, as promised, a "final" patch that (a) reorders the popped-up history so that the most recent folder is at the top and becomes the default; (b) pops up the history just after activating the folder search box; and (c) uses the most recent folder if "enter" is hit in the folder search box before anything else has been typed and before the history has managed to pop up. The (a) part is new, the (b) part is identical to the patch just above, and the (c) part is half of the first patch in I suggested -- the part about changing the placeholder is no longer necessary, as the placeholder is not visible long enough to read, and the default folder (i.e., most recent) comes up in the input box right away.

diff -u -r nostalgy.orig/chrome/content/folders.js round4Nostalgy/chrome/content/folders.js
--- nostalgy.orig/chrome/content/folders.js 2022-06-24 19:35:06.000000000 -0400
+++ round4Nostalgy/chrome/content/folders.js    2022-11-19 14:19:00.118440506 -0500
@@ -181,33 +181,34 @@
     var f = function (folder) { add_folder(NostalgyFolderName(folder)); };

     if (text == " ") {
+      // We put the recent folders in first in reverse order so that
+      // the most recent will be at the top and hence be the default completion
+      values = Array.from(nostalgy_recent_folders).reverse();
+      nb = values.length;
+      // And now we add any additional suggested folders up to the max_size
       var added_count = 0;
       var predictedFolders = null;
+      var predictedToAdd = [];
       if (nostalgy_completion_options.use_statistical_prediction) {
         try { predictedFolders = NostalgyPredict.predict_folder(nostalgy_recent_folders_max_size); }
         catch (ex) { }
-        if (predictedFolders != null && predictedFolders.length > 0)
-          for (var j = 0; j < predictedFolders.length; j++)
-            if (added_count < nostalgy_recent_folders_max_size) {
-              f(predictedFolders[j]);
+        if (predictedFolders != null && predictedFolders.length > 0) {
+          for (var j = 0; j < predictedFolders.length; j++) {
+            var predictedName = NostalgyCrop(
+              NostalgyFolderName(predictedFolders[j]));
+            if (!(values.includes(predictedName))) {
+              predictedToAdd.unshift(predictedName);
               added_count++;
+              if (added_count >= nostalgy_recent_folders_max_size) {
+                break;
+              }
             }
+          }
+          values.push.apply(values, predictedToAdd);
+          nb = values.length;
+        }
       }
-      for (var j = 0; j < nostalgy_recent_folders.length; j++) {
-        var found = 0;
-        if (nostalgy_completion_options.use_statistical_prediction &&
-          predictedFolders != null && predictedFolders.length > 0)
-          for (var i = 0; i < predictedFolders.length; i++)
-            if (NostalgyFolderName(predictedFolders[i]) == nostalgy_recent_folders[j])
-              found = 1;
-        if (found == 0 && added_count < nostalgy_recent_folders_max_size) {
-          add_folder(nostalgy_recent_folders[j]);
-          added_count++;
-        };
-
-      };
-      //values = [];
-    } else {
+    } else { // we have some text to match
       nostalgy_search_folder_options.do_tags =
         nostalgy_completion_options.always_include_tags ||
         (text.substr(0, 1) == ":");
@@ -340,12 +341,19 @@

 // Resolve a string coming from a completion box
 // 1. check whether uri comes from the completion list (cropped exact uri)
-// 2. if not, assume the uri has been typed in by the user
+// 2. If not, if the uri is empty, revert to the most recent folder
+// 3. Otherwise, assume the uri has been typed in by the user
 //    and take the first matching folder

 function NostalgyResolveFolder(uri) {
   var ret = NostalgyFindFolderCropped(uri);
-  if (ret) { return ret; } else { return (NostalgyFirstCompletion(uri)); }
+  if (ret) { return ret; }
+  if ((uri === '' || uri === ' ') && nostalgy_recent_folders.length) {
+    ret = NostalgyFindFolderExact(
+      nostalgy_recent_folders[nostalgy_recent_folders.length - 1])
+    if (ret) { return ret; }
+  }
+  return NostalgyFirstCompletion(uri);
 }

 function NostalgyFirstCompletion(uri) {
diff -u -r nostalgy.orig/chrome/content/nostalgy.js round4Nostalgy/chrome/content/nostalgy.js
--- nostalgy.orig/chrome/content/nostalgy.js    2022-06-24 20:06:30.000000000 -0400
+++ round4Nostalgy/chrome/content/nostalgy.js   2022-11-19 14:33:19.146429592 -0500
@@ -589,7 +589,10 @@
     //    box.popup.setAttribute("nomaxresults", "true");
     //   box.popup.setAttribute("_normalMaxRows", "8");
     //     box.popup.maxResults =function () {return 8};
-    listener = box.controller.QueryInterface(Components.interfaces.nsIAutoCompleteObserver);
+    // listener = box.controller.QueryInterface(Components.interfaces.nsIAutoCompleteObserver);
+    box.controller.resetInternalState();
+    box.value = ' ';
+    box.controller.handleText();
   }
   else { // XPFE
     // box.mAutoCompleteObserver uses a flawed equality check so we have to replace it.
@@ -601,9 +604,9 @@
     };
     // Reset internal state
     box.currentSearchString = "";
+    NostalgyAutocompleteComponent().startSearch("", box.searchParam, null, listener);
   }

-  NostalgyAutocompleteComponent().startSearch("", box.searchParam, null, listener);
 }

 function NostalgyCreateTag(name) {
opto commented 1 year ago

thanks for all of that. I will look into it and see what I can integrate into Nostalgy++.

As you well estimate, the popup box is not documented at all and oneneeds to go into the code to get an idea what it does. It also has changed as compared to Alain's use.

shift-S: if I remember correctly, without looking at the code, at the moment, it shows the last used folder if no rule, otherwise it shows the rule destination.

I agree, a good feature request would be to be ableto override the rule suggestion.