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

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