<<< Date Index >>>     <<< Thread Index >>>

inconsistent expansion of mailbox shortcuts



If your $spoolfile points to a Maildir, you cannot
tab-complete "!" correctly.

 :set spoolfile=/somewhere/Maildir
 c!<tab><tab><tab>

will give you "!///".

This is caused by a conflict between mutt_complete()
and mutt_expand_path().

"+" and "=" always point to a directory, not a file.
So "+foo" may/should expand to "/somewhere/Mail/foo"
even when your $folder is set to "/somewhere/Mail".
Note it doesn't end with "/".

But ~ < > ! - ^ and @ not always mean a directory.
Actually, mutt_expand_path() expands "!foo" to
"/somewhere/Maildirfoo" instead of "/somewhere/Maildir/foo"
if your $spoolfile is "/somewhere/Maildir" (without
a trailing slash, again).

Reading through complete.c, I thought that it was good
to expand these special shortcut chars and tell users
what they are doing while completing.

So here is a PoC patch.

-- 
tamo
diff -r 5ea9f336dd60 complete.c
--- a/complete.c        Mon Oct 20 11:50:38 2008 -0700
+++ b/complete.c        Thu Nov 06 03:38:03 2008 +0900
@@ -39,85 +39,84 @@
  */
 int mutt_complete (char *s, size_t slen)
 {
-  char *p;
+  char *p, *orig_dirend = NULL;
   DIR *dirp = NULL;
   struct dirent *de;
   int i ,init=0;
   size_t len;
   char dirpart[_POSIX_PATH_MAX], exp_dirpart[_POSIX_PATH_MAX];
   char filepart[_POSIX_PATH_MAX];
-#ifdef USE_IMAP
-  char imap_path[LONG_STRING];
 
   dprint (2, (debugfile, "mutt_complete: completing %s\n", s));
 
   /* we can use '/' as a delimiter, imap_complete rewrites it */
-  if (*s == '=' || *s == '+' || *s == '!')
+  if (*s == '=' || *s == '+')
   {
-    if (*s == '!')
-      p = NONULL (Spoolfile);
-    else
-      p = NONULL (Maildir);
-
-    mutt_concat_path (imap_path, p, s+1, sizeof (imap_path));
+    if (!(orig_dirend = strrchr (s, '/')))
+      orig_dirend = s;
+    mutt_concat_path (exp_dirpart, NONULL (Maildir),
+       *(s + 1) ? s + 1 : "/", sizeof (exp_dirpart));
+    mutt_expand_path (exp_dirpart, sizeof (exp_dirpart));
+  }
+  /* These will be expanded without a trailing slash.
+   * So don't use mutt_concat_path here.
+   * e.g. set spoolfile=/foo/bar
+   *      and "!baz" will become "/foo/barbaz"
+   * Note: These may be expanded into dirpart. */
+  else if (strchr ("~<>!-^@", *s) != NULL)
+  {
+    if (!(orig_dirend = strrchr (s, '/')))
+    {
+      if (*s == '@')
+       dprint (1, (debugfile, "mutt_complete(): alias completion hasn't been 
implemented yet\n"));
+      else if (*s == '~')
+       dprint (1, (debugfile, "mutt_complete(): uid completion hasn't been 
implemented yet\n"));
+    }
+    strfcpy (exp_dirpart, s, sizeof (exp_dirpart));
+    mutt_expand_path (exp_dirpart, sizeof (exp_dirpart));
   }
   else
-    strfcpy (imap_path, s, sizeof(imap_path));
+    strfcpy (exp_dirpart, s, sizeof (exp_dirpart));
 
-  if (mx_is_imap (imap_path))
-    return imap_complete (s, slen, imap_path);
-#endif
-  
-  if (*s == '=' || *s == '+' || *s == '!')
+  if ((p = strrchr (exp_dirpart, '/')))
   {
-    dirpart[0] = *s;
-    dirpart[1] = 0;
-    if (*s == '!')
-      strfcpy (exp_dirpart, NONULL (Spoolfile), sizeof (exp_dirpart));
-    else
-      strfcpy (exp_dirpart, NONULL (Maildir), sizeof (exp_dirpart));
-    if ((p = strrchr (s, '/')))
+    if (p == exp_dirpart) /* absolute path */
     {
-      char buf[_POSIX_PATH_MAX];
-      if (mutt_concatn_path (buf, sizeof(buf), exp_dirpart, 
strlen(exp_dirpart), s + 1, (size_t)(p - s - 1)) == NULL) {
-             return -1;
-      }
-      strfcpy (exp_dirpart, buf, sizeof (exp_dirpart));
-      mutt_substrcpy(dirpart, s, p+1, sizeof(dirpart));
-      strfcpy (filepart, p + 1, sizeof (filepart));
+      p = exp_dirpart + 1;
+      if (orig_dirend)
+       mutt_substrcpy (dirpart, s, orig_dirend + 1, sizeof (dirpart));
+      else
+       strfcpy (dirpart, "/", sizeof (dirpart));
+      exp_dirpart[0] = 0;
+      strfcpy (filepart, p, sizeof (filepart));
+      dirp = opendir (dirpart);
     }
     else
-      strfcpy (filepart, s + 1, sizeof (filepart));
-    dirp = opendir (exp_dirpart);
+    {
+#ifdef USE_IMAP
+      if (mx_is_imap (exp_dirpart))
+       return imap_complete (s, slen, exp_dirpart);
+#endif
+      p++;
+      if (orig_dirend)
+       mutt_substrcpy (dirpart, s, orig_dirend + 1, sizeof (dirpart));
+      else
+       mutt_substrcpy (dirpart, exp_dirpart, p, sizeof (dirpart));
+      strfcpy (filepart, p, sizeof (filepart));
+      *p = '\0';
+      dirp = opendir (exp_dirpart);
+    }
   }
   else
   {
-    if ((p = strrchr (s, '/')))
-    {
-      if (p == s) /* absolute path */
-      {
-       p = s + 1;
-       strfcpy (dirpart, "/", sizeof (dirpart));
-       exp_dirpart[0] = 0;
-       strfcpy (filepart, p, sizeof (filepart));
-       dirp = opendir (dirpart);
-      }
-      else
-      {
-       mutt_substrcpy(dirpart, s, p, sizeof(dirpart));
-       strfcpy (filepart, p + 1, sizeof (filepart));
-       strfcpy (exp_dirpart, dirpart, sizeof (exp_dirpart));
-       mutt_expand_path (exp_dirpart, sizeof (exp_dirpart));
-       dirp = opendir (exp_dirpart);
-      }
-    }
-    else
-    {
-      /* no directory name, so assume current directory. */
-      dirpart[0] = 0;
-      strfcpy (filepart, s, sizeof (filepart));
-      dirp = opendir (".");
-    }
+    /* no directory name, so assume current directory. */
+    dirpart[0] = 0;
+    strfcpy (filepart, s, sizeof (filepart));
+#ifdef USE_IMAP
+    if (mx_is_imap (filepart))
+      return imap_complete (s, slen, filepart);
+#endif
+    dirp = opendir (".");
   }
 
   if (dirp == NULL)
@@ -184,15 +183,13 @@
   }
   closedir (dirp);
 
+  if (!init)
+    return -1;
+
   if (dirpart[0])
-  {
-    strfcpy (s, dirpart, slen);
-    if (mutt_strcmp ("/", dirpart) != 0 && dirpart[0] != '=' && dirpart[0] != 
'+')
-      strfcpy (s + strlen (s), "/", slen - strlen (s));
-    strfcpy (s + strlen (s), filepart, slen - strlen (s));
-  }
+    mutt_concat_path (s, dirpart, filepart, slen);
   else
     strfcpy (s, filepart, slen);
 
-  return (init ? 0 : -1);
+  return 0;
 }
diff -r 5ea9f336dd60 lib.c
--- a/lib.c     Mon Oct 20 11:50:38 2008 -0700
+++ b/lib.c     Thu Nov 06 03:38:04 2008 +0900
@@ -976,7 +976,8 @@
 {
   const char *fmt = "%s/%s";
   
-  if (!*fname || (*dir && dir[strlen(dir)-1] == '/'))
+  if (!*fname || (*dir && dir[strlen(dir)-1] == '/')
+      || ((dir[0] == '+' || dir[0] == '=') && !dir[1]))
     fmt = "%s%s";
   
   snprintf (d, l, fmt, dir, fname);