On Wed, Nov 18, 2020 at 12:09:08AM +0000, Gregory Heytings wrote:
Thanks for all your comments! Here is the updated patch, in which I implemented all your suggestions.

Great, thank you Gregory! Unfortunately, after testing and another round of review, there are a few more things I saw that should be fixed.


--- a/curs_lib.c
+++ b/curs_lib.c
@@ -775,6 +775,13 @@ void mutt_reflow_windows (void)
   MuttMessageWindow->row_offset = LINES - 1;
memcpy (MuttIndexWindow, MuttStatusWindow, sizeof (mutt_window_t));
+
+  if (! option (OPTSTATUSVISIBLE))
+  {
+    MuttStatusWindow->rows = 0;
+    MuttStatusWindow->cols = 0;
+  }

Remove the cols=0 setting. The status window is used as a template in the pager, and the cols=0 will cause the index_status_window cols to be 0 too.


+static char *mutt_filter_status_line (char *s)
+{
+  BUFFER *command; FILE *filter; char *ps, *rs; size_t ps_size; int r; pid_t 
pid;

Mutt typically doesn't mix variable definitions of different types on a
single line.  I'd like to see these broken into five separate lines.


+
+  rs = safe_strdup (s);
+
+  if (!StatusFilter)
+    return rs;
+

It's too bad that s is always safe_strdup'ed now, even if there is no StatusFilter. It's no big deal, so you don't need to change it. Would there be a way to just return NULL in that case, without making the logic more complicated. If not, then don't worry about it.


+  else
+  {
+    if (ps != NULL) FREE (&ps);

FREE() deals with a NULL value, so there is no need for the NULL check. (Plus this would need to be on two different lines).


--- a/init.h
+++ b/init.h
@@ -4166,6 +4166,15 @@ struct option_t MuttVars[] = {
   /* L10N:
      $status_format default value
   */
+  { "status_filter", DT_STR, R_NONE, {.p=&StatusFilter}, {.p=0} },
+  /*
+  ** .pp
+  ** When set, specifies a command used to filter the status line, which is
+  ** displayed in the main status bar if $$status_visible is set.
+  ** This command does not filter the status line of the mini-index that is
+  ** displayed when $$pager_index_lines is non-zero.
+  ** .pp

You should mention that the status line is passed as an argument, and that the string may contain %s, which will be quoted for the user. See $query_command for some sample documentation.


Please add a commit message to your patch. Feel free to use a git patch format, to make setting you as the Author automatic too.


Lastly, unfortunately after more testing, the pager window layout logic did not work correctly with a status window that can now be hidden.

I believe this will fix the problem, but I need to take another look tomorrow when I'm not so tired. If you have a chance to test against various permutations I would appreciate it.

modified   pager.c
@@ -1687,13 +1687,16 @@ static void pager_menu_redraw (MUTTMENU *pager_menu)
       if (option (OPTSTATUSONTOP))
       {
         memcpy (rd->index_status_window, MuttStatusWindow, 
sizeof(mutt_window_t));
+        rd->index_status_window->rows = 1;
+
+        rd->index_window->row_offset = 1;
memcpy (rd->pager_status_window, MuttIndexWindow, sizeof(mutt_window_t));
-        rd->pager_status_window->rows = 1;
+        rd->pager_status_window->rows = option (OPTSTATUSVISIBLE) ? 1 : 0;
         rd->pager_status_window->row_offset += rd->index_window->rows;
- rd->pager_window->rows -= rd->index_window->rows + rd->pager_status_window->rows;
-        rd->pager_window->row_offset += rd->index_window->rows + 
rd->pager_status_window->rows;
+        rd->pager_window->rows -= rd->index_window->rows + 
rd->index_status_window->rows;
+        rd->pager_window->row_offset += rd->index_window->rows + 
rd->index_status_window->rows;
       }
       else
       {

--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA

Attachment: signature.asc
Description: PGP signature

Reply via email to