Re: [mmh] Previous-Sequence

2020-03-22 Thread markus schnalke
Hoi.

[2020-03-22 13:31] Philipp Takacs 
> [2020-03-22 07:53] markus schnalke 
> > [2020-03-22 00:21] Philipp Takacs 
> > > [2020-03-21 09:54] markus schnalke 
> > > > [2019-12-15 17:55] Philipp Takacs 
> > > > >
> > > > > During that I found the Previous-Sequence again. I don't see a reason
> > > > > why we need this feature. The usecases, I see, for the feature are 
> > > > > better
> > > > > handled with mark or the backlog of your shell. But I don't use this
> > > > > feature.
> > > 
> > > During reading some old mails on the nmh mailing list I found a use case.
> > > Put mails in a sequence after a refile[0].
> >
> > > [0] 
> > > https://lists.nongnu.org/archive/html/nmh-workers/2005-12/msg00086.html
> >
> > That's a valid usecase. But I don't think that we need to have the
> > previous sequence to cover that. The refiled messages will always
> > be at the end of the destination folder. Thus, you can solve it
> > this way as well:
> >
> > b=`mhpath b +dest`
> > refile -src +src ...msg... +dest
> > mark -seq pseq +dest "$b"-l
> 
> You missed a b=`basename b`, because mhpath gives you the full path and
> mark will take msgs not paths. But the concept is clear.

Yes. (I didn't test the code, just wrote from memory.)

> > As it is a rare usecase and there is a workaround, I'd rather
> > document the ``workaround'' (actually not really a workaround but
> > rather a different approach to solve the problem) in the man
> > page.
> 
> I don't think this use case is this rare. I often let my mails in the
> inbox just because I'm to lacy to move them away and set it unread. But
> this is not a argument for keeping the Previous-Sequence.

I wonder if we understood each other correctly. I think it is rare
to need the messages you've just refiled to be put automatically in
a sequence in the new folder. This is the usecase mentioned above.
You first refile the messages and then do something else with the
same messages in the new folder.

If you know beforehand that you want to do so, you can create that
sequence in the destination folder with little overhead: Either
remember the `b' message of the destination folder before you
refile and use `OLDb-l' afterwards, or find out how many messages
you gonna refile and use `l:THATNUMBER' afterwards.

(I'd *rather* add a `-verbose' switch to refile(1), which outputs
how many messages were refiled, plus source and destination
message numbers, and maybe the sequences they were part of in
the source folder (the latter is unnecessary, once we have the a
retain sequences functionality).)


But to conclude: Unless there appear new and relevant usecases for
the previous sequence, I agree with you to remove it.


meillo



Re: [mmh] retain sequences on refile

2020-03-22 Thread markus schnalke
Hoi,

split this topic from the previous sequence mail.

[2020-03-22 13:31] Philipp Takacs 
> [2020-03-22 07:53] markus schnalke 
> > [2020-03-22 00:21] Philipp Takacs 
> > >
> > > Not sure if this works, but
> > > for this I would suggest to add a -preserve switch to refile instead of
> > > using previous-sequence for this, like nmh has done.
> >
> > The `-preserve' switch changes how the refiling works in nmh ...
> 
> A sorry mixed the switches up, I mean the -retainsequences switch.
> The man page says following:
>   As message sequences are folder-specific, moving the message from the  
> source  folder
>   removes  it from all its sequences in that folder.  -retainsequences 
> adds it to those
>   same sequences in the destination folder, creating any that don't 
> exist.  This adding
>   does not apply for the “cur” sequence.
> 
> I like this feature, but this is an other discussion.

I like it too. Mmh does not act as expected concerning sequences
on refiles.

The length of the switch name is horrible, however. But actually
such a functionality should need no switch in mmh. It is correct
that sequences belong to messages not to folders (despite being
stored within the folder), thus on refiles they should move with
the message. This is how it should be. (Yes, it adds further
write accesses to .mh_sequence files ...)

In nmh every change needs switches so that no user ever needs to
change their setup. Mmh, in contrast can change if there is a
better way to do things. That's the whole reason of mmh's
existance.

I think the sequence behavior on refiles should be either this
way or that way. I see no need for a switch here.


meillo



Re: [mmh] folder_read with scandir()

2020-03-22 Thread markus schnalke
Hoi.

[2020-03-22 13:17] Philipp Takacs 
> [2020-03-22 07:20] markus schnalke 
> > [2020-03-22 01:08] Philipp Takacs 
> > > [2020-03-21 23:01] markus schnalke 
> > > > [2020-03-21 17:48] Philipp Takacs 
> > > > > [2020-03-21 10:28] markus schnalke 
> > > > > >
> > > > > > - 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. 
> > > > > 
> > > > > The files starting with a comma/dot aren't the problem they are 
> > > > > totally
> > > > > ignored. The problem are files witch aren't totally ignored. I have a 
> > > > > ugly
> > > > > workaround in the filter and I want to get rid of it.
> > > >
> > > > How I understand the situation (without looking at the code): A
> > > > folder can contain:
> > > >
> > > > - messages
> > > > - `.' and `..'
> > > > - `.mh_sequences'
> > > > - subfolders
> > > > - other files
> > > >
> > > > Other files are everything else, no matter if they are named
> > > > `.foo' or `,foo' or just `foo'.
> > > >
> > > > When we read a folder, only messages are of relevance. All the
> > > > rest is ignored.
> > > 
> > > expect for the OTHERS bit set for files which aren't messages and don't
> > > start with a '.' or a ','.
> >
> > Why for files starting with a comma? Is there a sense to do that
> > in mmh? Or is it only nmh legacy/compatiblity? And still: What
> > sense does it make to ignore backup files starting with a comma,
> > if the nmh installation might be configured to use hash prefixes
> > instead of comma prefixes?
> 
> I believe this is for legacy/compatiblity. The idea is, you don't have
> other files if you only have backup files in this directory.
> 
> > Either we should add '#' as well or remove ','. Can you follow my
> > reasoning?
> 
> For compatiblity reasons, I would say add the '#' while we have the
> other bit.

Agreed.

Please add a comment to the ',' and '#' case, as their reasons for
existance can not be found within mmh. Like:

For compatibility with nmh, ignore rmm backup files.


meillo



Re: [mmh] Previous-Sequence

2020-03-22 Thread Philipp Takacs
[2020-03-22 07:53] markus schnalke 
> [2020-03-22 00:21] Philipp Takacs 
> > [2020-03-21 09:54] markus schnalke 
> > > [2019-12-15 17:55] Philipp Takacs 
> > > >
> > > > During that I found the Previous-Sequence again. I don't see a reason
> > > > why we need this feature. The usecases, I see, for the feature are 
> > > > better
> > > > handled with mark or the backlog of your shell. But I don't use this
> > > > feature.
> > 
> > During reading some old mails on the nmh mailing list I found a use case.
> > Put mails in a sequence after a refile[0].
>
> > [0] https://lists.nongnu.org/archive/html/nmh-workers/2005-12/msg00086.html
>
> That's a valid usecase. But I don't think that we need to have the
> previous sequence to cover that. The refiled messages will always
> be at the end of the destination folder. Thus, you can solve it
> this way as well:
>
>   b=`mhpath b +dest`
>   refile -src +src ...msg... +dest
>   mark -seq pseq +dest "$b"-l

You missed a b=`basename b`, because mhpath gives you the full path and
mark will take msgs not paths. But the concept is clear.

> As it is a rare usecase and there is a workaround, I'd rather
> document the ``workaround'' (actually not really a workaround but
> rather a different approach to solve the problem) in the man
> page.

I don't think this use case is this rare. I often let my mails in the
inbox just because I'm to lacy to move them away and set it unread. But
this is not a argument for keeping the Previous-Sequence.

> (I think it's good to have hints and examples in the man pages to
> help newer users to figure out how to effectively work with mmh.
> See the manpage of pick(1) for example.)
>
> > Not sure if this works, but
> > for this I would suggest to add a -preserve switch to refile instead of
> > using previous-sequence for this, like nmh has done.
>
> The `-preserve' switch changes how the refiling works in nmh ...

A sorry mixed the switches up, I mean the -retainsequences switch.
The man page says following:
As message sequences are folder-specific, moving the message from the  
source  folder
removes  it from all its sequences in that folder.  -retainsequences 
adds it to those
same sequences in the destination folder, creating any that don't 
exist.  This adding
does not apply for the “cur” sequence.

I like this feature, but this is an other discussion.

Philipp



Re: [mmh] folder_read with scandir()

2020-03-22 Thread Philipp Takacs
[2020-03-22 07:20] markus schnalke 
> [2020-03-22 01:08] Philipp Takacs 
> > [2020-03-21 23:01] markus schnalke 
> > > [2020-03-21 17:48] Philipp Takacs 
> > > > [2020-03-21 10:28] markus schnalke 
> > > > >
> > > > > - 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. 
> > > > 
> > > > The files starting with a comma/dot aren't the problem they are totally
> > > > ignored. The problem are files witch aren't totally ignored. I have a 
> > > > ugly
> > > > workaround in the filter and I want to get rid of it.
> > >
> > > How I understand the situation (without looking at the code): A
> > > folder can contain:
> > >
> > > - messages
> > > - `.' and `..'
> > > - `.mh_sequences'
> > > - subfolders
> > > - other files
> > >
> > > Other files are everything else, no matter if they are named
> > > `.foo' or `,foo' or just `foo'.
> > >
> > > When we read a folder, only messages are of relevance. All the
> > > rest is ignored.
> > 
> > expect for the OTHERS bit set for files which aren't messages and don't
> > start with a '.' or a ','.
>
> Why for files starting with a comma? Is there a sense to do that
> in mmh? Or is it only nmh legacy/compatiblity? And still: What
> sense does it make to ignore backup files starting with a comma,
> if the nmh installation might be configured to use hash prefixes
> instead of comma prefixes?

I believe this is for legacy/compatiblity. The idea is, you don't have
other files if you only have backup files in this directory.

> Either we should add '#' as well or remove ','. Can you follow my
> reasoning?

For compatiblity reasons, I would say add the '#' while we have the
other bit.

Philipp



Re: [mmh] folder_add and msgstats

2020-03-22 Thread Philipp Takacs
[2020-03-21 23:15] markus schnalke 
> [2020-03-21 18:43] Philipp Takacs 
> > [2020-03-21 10:05] markus schnalke 
> > > [2020-03-19 12:33] Philipp Takacs 
> > > > I have also removed the
> > > > clear_msg_flags loop from folder_read and folder_realloc. In most cases
> > > > we use calloc, so this isn't necesarry. At one place in folder_realloc
> > > > realloc is used. There I have add a memset.
> > > > 
> > > > comment?
> > >
> > > Why were (in the section down below) only the flags outside the
> > > current message range cleared? Now you clear all flags.
> > 
> > Not sure whats make you think this. But if I had to guess I would
> > say it's the "- lo".
>
> No, it's the comment a bit more down in the code:
>
> > > -   /*
> > > -   ** Clear all the flags for entries outside
> > > -   ** the current message range for this folder.
> > > -   */

Now I get it.

> Maybe I was irritated that you removed this comment but still
> performed a similar action. The memset() line is pretty complex
> to understand. You could add a similar two-line comment to it,
> explaining what you explained to me here (only a bit more
> destilled):

Working on it.

> > > Or, if your memset() clears only specific
> > > parts, you probably should put a comment there to explain it, as
> > > this might not be what the programmer usually expects from a
> > > memset() after a (re)alloc().
> > 
> > To me this looks completly like this what I would expect.
>
> Of course; you've written the code. ;-)

I mean more a general view. So after a realloc I would expect something
like:

memset(ptr + oldsize + 1, 0, newsize - oldsize)

of course my code dose calculate the oldsize a bit strange, but in it's
more or less the same.

> > But I'll think about a good comment.
>
> Is what you actually clear only the newly allocated memory? If so
> then you could write something like:
>
>   /* zero the newly allocated memory, i.e. no flags set */
>
> Or:
>
>   /* clear the newly allocated msg flag space */

Thanks for the hints. Looks now like this:

+   /*
+   ** Clear the newly allocated msg flag space. The lowoff and
+   ** hghoff are the old messagenumber range. So the calculation
+   ** of the first new element has to subtract lowoff.
+   */
+   memset(mp->msgstats + mp->hghoff - lo + 1, 0, hi - mp->hghoff);

Philipp



Re: [mmh] show sequences racecondition

2020-03-22 Thread Philipp Takacs
[2020-03-21 23:29] markus schnalke 
> [2020-03-21 16:15] Philipp Takacs 
> Thanks for the explanation. Show(1) is the only case where I've
> noticed the problem. Thus it's the most important.

That's the reason I have started there. I would say I look at the
patches again, to see if I have overseen something and push them in the
next days.

> > I found then, that seq_read and seq_list ignores mails witch it thinks
> > don't exist. This I have disabled this, to avoid a secound read of the 
> > of the whole directory.
> > 
> > Now the conflict window is small, but still there. The best way to
> > handle this is locking.
>
> This whole topic is a special one ... One case that one should
> have in mind as well: Stuff shouldn't mess up, if you have show(1)
> running while deleting/refiling/incing messages, not only to the
> end but possibly on previously used message numbers. For instance,
> I use `folder -pack' on the inbox quite often ... there might be a
> `repl' job in background at the same time ...

I have the problems with sortm and folder -pack in mind. I have some
idea how to prevent most of such problems. But the problem is: We can
help the user to prevent such problems, but there is no way we can stop
the user from shooting him in the food. ;-)

> I really don't want to burden you with this stuff. I think that
> improving the most annoying problem is already a great step
> forward. The rest is more of a separate topic to dig into and
> solve consistently and generally. I only wanted to put the
> information onto the list, to have it documented.

It don't burden me. It's nice to read an other view on mmh. Most
of the time it gives me good idea for future work or how I have
to change my code to get better results.

> P.S.
> Not related to this topic, more in general: I really appreciate
> that you supported your changes with test cases. This is good
> work. It will make mmh increasingly stronger. Thank you!

A test helps a lot, first it's the best argument why my patch is needed.
Second I use the tests to see if I really fixed the problems. Third I
see later, if another patch kills some functionality. So if anybody has
a problem with mmh, a test case help me (and others) to fix this issue.

Philipp