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

Re: inconsistent expansion of mailbox shortcuts



* Thu Nov  6 2008 TAKAHASHI Tamotsu <ttakah@xxxxxxxxxxxxxxxxx>
> 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().

I'm afraid this bug was introduced on 2000-06-15:

| 2000-06-15 21:37:07  roessler  (roessler)
|   * complete.c: patch-1.3.2.bbell.complete.1 - autocomplete "!",
|   which is helpful in certain IMAP contexts.

http://marc.info/?l=mutt-dev&m=95851994403173&w=2

I think we should change the current implementation
to more intuitive behavior because even TLR and BB
didn't understand the "!" expansion correctly.


> "+" 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).

I suggest we treat < > ! - and ^ as directories, i.e. expand
"!foo" to "${spoolfile}/foo" instead of "${spoolfile}foo".
See the attached patch. (I don't like the browser.c part,
and it is not necessary.)

We use "~username" as a completely different path
from "~/username", but I think nobody has used "!foo"
as a different directory from "!/foo".
So this change should be safe.


...Or take the following way:

> 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 8087be7178cd complete.c
--- a/complete.c        Sat Nov 15 20:24:46 2008 -0800
+++ b/complete.c        Sun Nov 16 22:37:35 2008 +0900
@@ -43,7 +43,7 @@
   DIR *dirp = NULL;
   struct dirent *de;
   int i ,init=0;
-  size_t len;
+  size_t len, offset = mutt_strncmp ("!!", s, 2) ? 1 : 2;
   char dirpart[_POSIX_PATH_MAX], exp_dirpart[_POSIX_PATH_MAX];
   char filepart[_POSIX_PATH_MAX];
 #ifdef USE_IMAP
@@ -52,14 +52,11 @@
   dprint (2, (debugfile, "mutt_complete: completing %s\n", s));
 
   /* we can use '/' as a delimiter, imap_complete rewrites it */
-  if (*s == '=' || *s == '+' || *s == '!')
+  if (strchr ("=+!<>-^", *s))
   {
-    if (*s == '!')
-      p = NONULL (Spoolfile);
-    else
-      p = NONULL (Maildir);
-
-    mutt_concat_path (imap_path, p, s+1, sizeof (imap_path));
+    mutt_substrcpy (exp_dirpart, s, s + offset + 1, sizeof (exp_dirpart));
+    mutt_expand_path (exp_dirpart, sizeof (exp_dirpart));
+    mutt_concat_path (imap_path, exp_dirpart, s + offset, sizeof (imap_path));
   }
   else
     strfcpy (imap_path, s, sizeof(imap_path));
@@ -68,18 +65,18 @@
     return imap_complete (s, slen, imap_path);
 #endif
   
-  if (*s == '=' || *s == '+' || *s == '!')
+  if (strchr ("=+!<>-^", *s))
   {
-    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));
+    mutt_substrcpy (dirpart, s, s + offset + 1, sizeof (dirpart));
+    strfcpy (exp_dirpart, dirpart, sizeof (exp_dirpart));
+    mutt_expand_path (exp_dirpart, sizeof (exp_dirpart));
     if ((p = strrchr (s, '/')))
     {
       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) {
+      if (mutt_concatn_path (buf, sizeof(buf),
+           exp_dirpart, strlen(exp_dirpart),
+           s + offset, (size_t)(p - s - offset)) == NULL)
+      {
              return -1;
       }
       strfcpy (exp_dirpart, buf, sizeof (exp_dirpart));
@@ -87,7 +84,7 @@
       strfcpy (filepart, p + 1, sizeof (filepart));
     }
     else
-      strfcpy (filepart, s + 1, sizeof (filepart));
+      strfcpy (filepart, s + offset, sizeof (filepart));
     dirp = opendir (exp_dirpart);
   }
   else
@@ -185,12 +182,7 @@
   closedir (dirp);
 
   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);
 
diff -r 8087be7178cd lib.c
--- a/lib.c     Sat Nov 15 20:24:46 2008 -0800
+++ b/lib.c     Sun Nov 16 22:37:35 2008 +0900
@@ -975,8 +975,10 @@
 char *mutt_concat_path (char *d, const char *dir, const char *fname, size_t l)
 {
   const char *fmt = "%s/%s";
+  size_t dlen = mutt_strlen (dir);
   
-  if (!*fname || (*dir && dir[strlen(dir)-1] == '/'))
+  if (!*fname || (dlen && dir[dlen - 1] == '/') ||
+      !strcmp ("!!", dir) || (dlen == 1 && strchr ("=+!<>-^", dir[0])))
     fmt = "%s%s";
   
   snprintf (d, l, fmt, dir, fname);
diff -r 8087be7178cd muttlib.c
--- a/muttlib.c Sat Nov 15 20:24:46 2008 -0800
+++ b/muttlib.c Sun Nov 16 22:37:35 2008 +0900
@@ -432,14 +432,14 @@
       
       case '>':
       {
-       strfcpy (p, NONULL(Inbox), sizeof (p));
+       snprintf (p, sizeof (p), "%s%s", NONULL (Inbox), *(s + 1) ? "/" : "");
        tail = s + 1;
       }
       break;
       
       case '<':
       {
-       strfcpy (p, NONULL(Outbox), sizeof (p));
+       snprintf (p, sizeof (p), "%s%s", NONULL (Outbox), *(s + 1) ? "/" : "");
        tail = s + 1;
       }
       break;
@@ -448,12 +448,12 @@
       {
        if (*(s+1) == '!')
        {
-         strfcpy (p, NONULL(LastFolder), sizeof (p));
+         snprintf (p, sizeof (p), "%s%s", NONULL (LastFolder), *(s + 2) ? "/" 
: "");
          tail = s + 2;
        }
        else 
        {
-         strfcpy (p, NONULL(Spoolfile), sizeof (p));
+         snprintf (p, sizeof (p), "%s%s", NONULL (Spoolfile), *(s + 1) ? "/" : 
"");
          tail = s + 1;
        }
       }
@@ -461,14 +461,14 @@
       
       case '-':
       {
-       strfcpy (p, NONULL(LastFolder), sizeof (p));
+       snprintf (p, sizeof (p), "%s%s", NONULL (LastFolder), *(s + 1) ? "/" : 
"");
        tail = s + 1;
       }
       break;
       
       case '^':        
       {
-       strfcpy (p, NONULL(CurrentFolder), sizeof (p));
+       snprintf (p, sizeof (p), "%s%s", NONULL (CurrentFolder), *(s + 1) ? "/" 
: "");
        tail = s + 1;
       }
       break;
diff -r 8087be7178cd browser.c
--- a/browser.c Sat Nov 15 20:24:46 2008 -0800
+++ b/browser.c Sun Nov 16 22:37:35 2008 +0900
@@ -560,6 +560,11 @@
 
   if (*f)
   {
+    /* treat as a directory, not a file */
+    if (!mutt_strcmp ("!!", f) ||
+       (strchr ("!<>-^", *f) && !*(f + 1)))
+      safe_strcat (f, flen, "/");
+
     mutt_expand_path (f, flen);
 #ifdef USE_IMAP
     if (mx_is_imap (f))