Hoi.

[2020-03-19 12:45] Philipp Takacs <phil...@bureaucracy.de>
>
> I have also looked at folder_read and found it't a bit ugly ;-)

A good indicator for starting to rework. :-)

> I have rewritten it with scandir. This saves about 40 loc and improves
> the reading flow for this funktion.

Sounds good!


Some comments:

> Also noticed the other flag, not shoure if we need this realy. It's
> only used in folder and I don't see the point of knowing that there
> are other files in a folder.

We should be a bit careful at that point to understand the
situation. Here are some aspects to think about:

- Files starting with a comma. These were backups of deleted messages
(see the section on the trash folder in my master's thesis). Thus
they are considered MH files here but not further regarded. I'm
not sure if the code had been this way all the time, because there
used to the a `--with-hash-prefix' option (or similar) that
replaced the comma with a hash symbol as the prefix for deleted
messages backups. Anyways, we don't have such comma files anymore.
Mmh uses the trash folder for that case, i.e. we can get rid of
the comma prefix file handling ... possibly. Does anyone know the
situation in nmh: have they switched to a trash folder as well?
Because we're still (basically) mail storage compatible to nmh (I
think), we should not be fully ignorant of that situation. But we
could handle those backup files as `other' files. 

- `Other' files seem to be files in the mail storage that have
nothing to do with MH. E.g. any random file someone created in the
mail storage without any connection to mmh. Normally mmh would
simply ignore them, i.e. act as if they wouldn't be there at all.
There's one place, however, where `other' files are relevant:
rmf(1). We should not delete a mail folder that includes `other'
files. It seems this check is missing there. Hence, I would keep
the `other' files marker. It adds few complexity and can be
useful. Plus, we should use it in rmf(1).


> +static int others;
> +
> +static int
> +msgcmp(const struct dirent **d1, const struct dirent **d2)
> +{
> +     size_t l1, l2;
> +
> +     l1 = strlen((*d1)->d_name);
> +     l2 = strlen((*d2)->d_name);
> +     if (l1 < l2) {
> +             return -1;
> +     }
> +     if (l1 > l2) {
> +             return 1;
> +     }
> +     return strcmp((*d1)->d_name, (*d2)->d_name);
> +}

Isn't that a strange comparison algorithm, for first compare
string lengths and only if eaqual their content? Here does mmh
sort by length of folder name?


> +static int
> +msgfilter(const struct dirent *e)
> +{
> +     int i;
> +     if (e->d_name[0] == '.' || e->d_name[0] == ',') {

About this comma: see above!

> +             return 0;
> +     }
> +     for (i = 0; e->d_name[i]; i++) {
> +             //if ((i > 1 && e->d_name[i] < '0') || e->d_name[i] < '1' || 
> e->d_name[i] > '9') {

C99-style comment ... looks to be not finished at this point.

> +             if (e->d_name[i] < '0' || e->d_name[i] > '9') {
> +                     others = 1;
> +                     return 0;
> +             }
> +     }
> +     return 1;
> +}


Wow, it feels so good to have some touch with the mmh development
again! I'm really looking forward to dig into the code as well.
Hoping I get that managed soon.


meillo

Reply via email to