Re: New patch: index_format variable for showing attachment
Paul Walker wrote:
> These comments are based just on looking at your patch, rather than where
> they fit into mutt, so please correct me if I'm wrong on that basis.
It's fine. I'm a beginner in C and this is my first contribution to
the open source community, so I want to learn.
> > +WHERE char *MimeMultiString;
>
> Why make the string configurable...? 's', 'S', etc. aren't configurable, as
> far as I remember, and this is much closer to them than it is to to_chars
> and co.
I though about that, and came up with a configurable string. I don't
personally gonna use any other than just "#" or "@", but I thought
that maybe someone else want to...
> > + * %g = display # if message is mime multipart (has attachment)
>
> Particularly when this bit says that it will be "#". ;-)
Ye ye ye. :)
> > + if ((i = strlen (MimeMultiString)) == 0)
>
> You don't use 'i' again below, as far as I can see from your patch, so no
> need for it.
Ah, that's true. I will remove that. It's a leftover from an earlier version.
> > + if (is_multipart (hdr->content))
> > + snprintf (dest, destlen, MimeMultiString);
> > + else
> > + {
> > + for (i = 0 ; i < strlen (MimeMultiString) && i < destlen ; i++)
> > + strcat (dest, " ");
> > + }
> > + break;
>
> This all gets a lot simpler if you don't make mimemultistring variable, but
> just a single char.
Yes, I know. Read above.
If there is a chance that this will be imported into the official
mutt, I can send a new modified version based on only one char...
--
www.getfirefox.com