Change folder with description in addition to path #1516
Conversation
ghost
assigned 
index.c
Outdated
| if (m) | ||
| { | ||
| magic = m->magic; | ||
| buf = mutt_str_strdup(m->path); |
There was a problem hiding this comment.
Yeah, that's a leak. I suggest:
| buf = mutt_str_strdup(m->path); | |
| mutt_str_strfcpy(buf, m->path, buflen); |
There was a problem hiding this comment.
Unfortunately, that will not solve our memory leak without introducing a potential seg-fault. There is a high probably that buf will be shorter than m->path if buf is a description. We will not have enough room to store the path.
There was a problem hiding this comment.
Ah, I see.
buflen isn't the length of the string in buf, but the length of the buffer -- which is 1024 bytes.
To be suitable for a mailbox path, this needs changing in mutt_index_menu():
-char buf[LONG_STRING];
+char buf[PATH_MAX];There was a problem hiding this comment.
The suggested changes resolved the problem. I have amended my commit.
austin-ray
force-pushed
the
devel/issue/1470
branch
from
8f7420b
to
e09796f
Compare
|
I've just made one slight change: I've reused the Mailbox variable, If the user's searching by description, it'll be The |
I swear I looked for an existing
That was my gut-instinct as well. On a slightly off-topic note, have we considered expanding |
I certainly hadn't until you raise this PR. The rules for matching descriptions might have to be tighter, so that we can use brief strings without matching lots of mailboxes. Please investigate. |
Mailbox: find mailbox by description Introduce `mutt_find_mailbox_desc()`, which takes a pointer to a description, and tries to find a mailbox that corresponds to it. With the introduction of `named-mailboxes` and removal of `virtual-mailboxes` special cases, users may prefer to operate with descriptions instead of (potentially) ugly paths. We can no longer assume that a buffer is a path and require a method to find by description. This is not included as an `mxapi` function since `desc` is common to all mailboxes so the backends do not need implement their own function. Index: try description when changing mailboxes Since mailbox descriptions are more prevalent with `named-mailboxes` and `virtual-mailbox` special cases being removed, some users will try to change folders with a description instead of a path. This commit modifies `main_change_folder()` to look for a mailbox with a given description if path probing fails.
flatcap
force-pushed
the
devel/issue/1470
branch
from
189d5f2
to
6b9065a
Compare
What does this PR do?
Adds the ability to change folders based on description instead of path. With the introduction of
named-mailboxesand removal ofvirtual-mailboxes's special cases, we cannot assume that the user will always provide a path.Instead of bailing when path probing fails, try to find a mailbox with a matching description. If successful, overwrite the buffer with its path.
Areas to review before merging:
mutt_str_strdup()allocates memory, but I have not included aFREE()call. I believe twoFREE()calls are needed: before thereturnon line 620. and after themutt_str_replace()call on line 625. Replacing themutt_str_strdup()with a pointer tom->pathmay obviate any newFREE()calls.This may not be an issue since descriptions would result in bailing before attempting to execute a hook. Since this PR makes sure
bufis a path, not a description, we shouldn't run into any issues. However, further analysis is warranted to confirm whether or not this is an issue.What are the relevant issue numbers?
#1470