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))