Hi
I have looked at the problem with open pager for show and change the
sequences during the pager is open. This happen often, if you show a
message and recieve a new message.
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.
The problem I see is, that sequences get racy, if you want to handle
your mail in parallel. I.e. have a mail open with show and get a new
mail. Does anyone see a usecase witch can't be handled with mark or the
backlog of your shell? Or an other reason to keep the Previous-Sequence?
So back to show I have two commits attached. One to remove the
Previous-Sequence and one to handle the unseen sequence better. If we
decide to keep the Previous-Sequence, I have to look at the race again.
The unseen sequence has it's own flag beside the sequences in the
msgs struct. The seq_setunseen() function updates the unseen sequence
in this struct. So I have just moved the seq_setunseen() call to the
end and added a seq_read() call before. This way it's still a bit racy
but not this hard. To fix the rest of the race we could add locking.
Comments?
Philipp
From b4b4f102566d1d3d0e4686ebf157d513a6a68f30 Mon Sep 17 00:00:00 2001
From: Philipp Takacs <phil...@bureaucracy.de>
Date: Sun, 8 Dec 2019 17:54:29 +0100
Subject: [PATCH 1/2] remove previus sequence
unfinished, docs need update
---
config/config.c | 1 -
h/mh.h | 1 -
sbr/Makefile.in | 2 +-
sbr/m_draft.c | 1 -
sbr/seq_setprev.c | 44 --------------------------------------------
uip/burst.c | 1 -
uip/comp.c | 1 -
uip/dist.c | 1 -
uip/folder.c | 1 -
uip/forw.c | 1 -
uip/mhlist.c | 1 -
uip/mhparam.c | 2 --
uip/mhpath.c | 2 --
uip/mhshow.c | 1 -
uip/mhstore.c | 1 -
uip/mhtest.c | 1 -
uip/packf.c | 1 -
uip/pick.c | 1 -
uip/refile.c | 2 --
uip/repl.c | 1 -
uip/rmm.c | 1 -
uip/send.c | 1 -
uip/sortm.c | 1 -
23 files changed, 1 insertion(+), 69 deletions(-)
delete mode 100644 sbr/seq_setprev.c
diff --git a/config/config.c b/config/config.c
index 5e250414..0a63912d 100644
--- a/config/config.c
+++ b/config/config.c
@@ -89,7 +89,6 @@ char *seq_unseen = "u";
char *seq_neg = "!";
char *usequence = "Unseen-Sequence";
-char *psequence = "Previous-Sequence";
char *nsequence = "Sequence-Negation";
/* profile entries for storage locations */
diff --git a/h/mh.h b/h/mh.h
index 6337a0ca..649582d3 100644
--- a/h/mh.h
+++ b/h/mh.h
@@ -288,7 +288,6 @@ extern char *msgprot;
extern char *nmhstorage;
extern char *nsequence;
extern char *profile;
-extern char *psequence;
extern char *rcvdistcomps;
extern char *replcomps;
extern char *replgroupcomps;
diff --git a/sbr/Makefile.in b/sbr/Makefile.in
index f2310fb8..142a7f20 100644
--- a/sbr/Makefile.in
+++ b/sbr/Makefile.in
@@ -68,7 +68,7 @@ SRCS = addrsbr.c ambigsw.c brkstring.c \
readconfig.c seq_add.c seq_bits.c \
seq_del.c seq_getnum.c seq_list.c seq_nameok.c \
seq_print.c seq_read.c seq_save.c seq_setcur.c \
- seq_setprev.c seq_setunseen.c signals.c \
+ seq_setunseen.c signals.c \
smatch.c snprintb.c strcasecmp.c \
strindex.c trim.c trimcpy.c uprf.c vfgets.c fmt_def.c \
mf.c utils.c m_mktemp.c seq_msgstats.c \
diff --git a/sbr/m_draft.c b/sbr/m_draft.c
index ee857900..050c7e51 100644
--- a/sbr/m_draft.c
+++ b/sbr/m_draft.c
@@ -53,7 +53,6 @@ m_draft(char *which)
*/
if (!m_convert(mp, which))
exit(EX_SOFTWARE);
- seq_setprev(mp);
snprintf(buffer, sizeof(buffer), "%s/%s", mp->foldpath,
m_name(mp->lowsel));
diff --git a/sbr/seq_setprev.c b/sbr/seq_setprev.c
deleted file mode 100644
index f4304cc5..00000000
--- a/sbr/seq_setprev.c
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
-** seq_setprev.c -- set the Previous-Sequence
-**
-** This code is Copyright (c) 2002, by the authors of nmh. See the
-** COPYRIGHT file in the root directory of the nmh distribution for
-** complete copyright information.
-*/
-
-#include <h/mh.h>
-#include <h/utils.h>
-
-/*
-** Add all the messages currently SELECTED to
-** the Previous-Sequence. This way, when the next
-** command is given, there is a convenient way to
-** selected all the messages used in the previous
-** command.
-*/
-
-void
-seq_setprev(struct msgs *mp)
-{
- char **ap, *cp, *dp;
-
- /*
- ** Get the list of sequences for Previous-Sequence
- ** and split them.
- */
- if ((cp = context_find(psequence))) {
- dp = mh_xstrdup(cp);
- if (!(ap = brkstring(dp, " ", "\n")) || !*ap) {
- mh_free0(&dp);
- return;
- }
- } else {
- return;
- }
-
- /* Now add all SELECTED messages to each sequence */
- for (; *ap; ap++)
- seq_addsel(mp, *ap, -1, 1);
-
- mh_free0(&dp);
-}
diff --git a/uip/burst.c b/uip/burst.c
index 12d1f9d4..09a60d9f 100644
--- a/uip/burst.c
+++ b/uip/burst.c
@@ -117,7 +117,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgp; msgnum++)
if (!m_convert(mp, msgs[msgnum]))
exit(EX_SOFTWARE);
- seq_setprev(mp); /* set the previous-sequence */
smsgs = mh_xcalloc(MAXFOLDER + 2, sizeof(*smsgs));
diff --git a/uip/comp.c b/uip/comp.c
index 75bbf86f..907bbf3d 100644
--- a/uip/comp.c
+++ b/uip/comp.c
@@ -148,7 +148,6 @@ main(int argc, char **argv)
if (!m_convert(mp, msg)) {
exit(EX_SOFTWARE);
}
- seq_setprev(mp); /* set the previous-sequence */
if (mp->numsel > 1) {
adios(EX_USAGE, NULL, "only one message at a time!");
}
diff --git a/uip/dist.c b/uip/dist.c
index 1d5c3aa4..66539e69 100644
--- a/uip/dist.c
+++ b/uip/dist.c
@@ -153,7 +153,6 @@ main(int argc, char **argv)
if (!m_convert(mp, msg)) {
exit(EX_USAGE);
}
- seq_setprev(mp);
if (mp->numsel > 1) {
adios(EX_USAGE, NULL, "only one message at a time!");
diff --git a/uip/folder.c b/uip/folder.c
index b7cdfe54..945e4fc1 100644
--- a/uip/folder.c
+++ b/uip/folder.c
@@ -615,7 +615,6 @@ sfold(struct msgs *mp, char *msg)
admonish(NULL, "only one message at a time!");
return 0;
}
- seq_setprev(mp); /* set the previous-sequence */
seq_setcur(mp, mp->lowsel); /* set current message */
seq_save(mp); /* synchronize message sequences */
context_save(); /* save the context file */
diff --git a/uip/forw.c b/uip/forw.c
index e00ae0e1..d6ded248 100644
--- a/uip/forw.c
+++ b/uip/forw.c
@@ -197,7 +197,6 @@ main(int argc, char **argv)
exit(EX_SOFTWARE);
}
}
- seq_setprev(mp); /* set the previous sequence */
if ((out = creat(drft, m_gmprot())) == NOTOK)
adios(EX_CANTCREAT, drft, "unable to create");
diff --git a/uip/mhlist.c b/uip/mhlist.c
index 7056db5f..169ca501 100644
--- a/uip/mhlist.c
+++ b/uip/mhlist.c
@@ -230,7 +230,6 @@ main(int argc, char **argv)
exit(EX_SOFTWARE);
}
}
- seq_setprev(mp); /* set the previous-sequence */
cts = mh_xcalloc(mp->numsel + 1, sizeof(*cts));
ctp = cts;
diff --git a/uip/mhparam.c b/uip/mhparam.c
index 2c61e608..e02bf495 100644
--- a/uip/mhparam.c
+++ b/uip/mhparam.c
@@ -78,7 +78,6 @@ static struct proc procs [] = {
{ "mimetypequery-component", &mimetypequery },
{ "nmhstorage-component", &nmhstorage },
{ "nsequence-component", &nsequence },
- { "psequence-component", &psequence },
{ "usequence-component", &usequence },
{ "#--Mmh-specific-Mail-Header-Names--", &empty },
@@ -109,7 +108,6 @@ static struct proc procs [] = {
{ "seq-first", &seq_first },
{ "seq-last", &seq_last },
{ "seq-next", &seq_next },
- { "previous-sequence", &seq_prev },
{ "unseen-sequence", &seq_unseen },
{ "sequence-negation", &seq_neg },
diff --git a/uip/mhpath.c b/uip/mhpath.c
index 31e0722e..df4e1401 100644
--- a/uip/mhpath.c
+++ b/uip/mhpath.c
@@ -123,8 +123,6 @@ main(int argc, char **argv)
}
}
- seq_setprev(mp); /* set the previous-sequence */
-
/* print the path of all selected messages */
for (i = mp->lowsel; i <= mp->hghsel; i++)
if (is_selected(mp, i))
diff --git a/uip/mhshow.c b/uip/mhshow.c
index be9cd454..6978454e 100644
--- a/uip/mhshow.c
+++ b/uip/mhshow.c
@@ -301,7 +301,6 @@ main(int argc, char **argv)
if (is_selected(mp, msgnum))
set_unseen(mp, msgnum);
- seq_setprev(mp); /* set the Previous-Sequence */
seq_setunseen(mp, 0); /* unset unseen seqs for shown msgs */
cts = mh_xcalloc(mp->numsel + 1, sizeof(*cts));
diff --git a/uip/mhstore.c b/uip/mhstore.c
index 47b7c025..971ee7ef 100644
--- a/uip/mhstore.c
+++ b/uip/mhstore.c
@@ -292,7 +292,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgs.size; msgnum++)
if (!m_convert(mp, msgs.msgs[msgnum]))
exit(EX_USAGE);
- seq_setprev(mp); /* set the previous-sequence */
cts = mh_xcalloc(mp->numsel + 1, sizeof(*cts));
ctp = cts;
diff --git a/uip/mhtest.c b/uip/mhtest.c
index d4bf6a6a..71787afc 100644
--- a/uip/mhtest.c
+++ b/uip/mhtest.c
@@ -240,7 +240,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgs.size; msgnum++)
if (!m_convert(mp, msgs.msgs[msgnum]))
exit(EX_USAGE);
- seq_setprev(mp); /* set the previous-sequence */
cts = mh_xcalloc(mp->numsel + 1, sizeof(*cts));
ctp = cts;
diff --git a/uip/packf.c b/uip/packf.c
index fe1102c6..b2726f46 100644
--- a/uip/packf.c
+++ b/uip/packf.c
@@ -95,7 +95,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgs.size; msgnum++)
if (!m_convert(mp, msgs.msgs[msgnum]))
exit(EX_USAGE);
- seq_setprev(mp); /* set the previous-sequence */
/* copy all the SELECTED messages to stdout */
for (msgnum = mp->lowsel; msgnum <= mp->hghsel; msgnum++) {
diff --git a/uip/pick.c b/uip/pick.c
index 8f28f354..0b9205bb 100644
--- a/uip/pick.c
+++ b/uip/pick.c
@@ -340,7 +340,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgs.size; msgnum++)
if (!m_convert(mp, msgs.msgs[msgnum]))
exit(EX_USAGE);
- seq_setprev(mp); /* set the previous-sequence */
/*
** If we aren't saving the results to a sequence,
diff --git a/uip/refile.c b/uip/refile.c
index 682f66a5..663936ba 100644
--- a/uip/refile.c
+++ b/uip/refile.c
@@ -188,7 +188,6 @@ main(int argc, char **argv)
exit(EX_SOFTWARE);
}
}
- seq_setprev(mp);
/* create folder structures for each destination folder */
opnfolds(folders, foldp);
@@ -277,7 +276,6 @@ clsfolds(struct st_fold *folders, int nfolders)
for (fp = folders, ep = folders + nfolders; fp < ep; fp++) {
mp = fp->f_mp;
- seq_setprev(mp);
seq_save(mp);
}
}
diff --git a/uip/repl.c b/uip/repl.c
index b2adebfe..438d8544 100644
--- a/uip/repl.c
+++ b/uip/repl.c
@@ -311,7 +311,6 @@ main(int argc, char **argv)
/* parse the message range/sequence/name and set SELECTED */
if (!m_convert(mp, msg))
exit(EX_SOFTWARE);
- seq_setprev(mp); /* set the previous-sequence */
if (mp->numsel > 1)
adios(EX_USAGE, NULL, "only one message at a time!");
diff --git a/uip/rmm.c b/uip/rmm.c
index 5059383d..72583270 100644
--- a/uip/rmm.c
+++ b/uip/rmm.c
@@ -117,7 +117,6 @@ main(int argc, char **argv)
if (unlink_msgs) {
/* "remove" the SELECTED messages */
folder_delmsgs(mp, 1);
- seq_setprev(mp);
seq_save(mp);
folder_free(mp);
return 0;
diff --git a/uip/send.c b/uip/send.c
index d7691db9..148c05bc 100644
--- a/uip/send.c
+++ b/uip/send.c
@@ -165,7 +165,6 @@ main(int argc, char **argv)
exit(EX_USAGE);
}
}
- seq_setprev(mp);
for (nmsgs = 0, msgnum = mp->lowsel;
msgnum <= mp->hghsel; msgnum++) {
diff --git a/uip/sortm.c b/uip/sortm.c
index c56a4019..740e6922 100644
--- a/uip/sortm.c
+++ b/uip/sortm.c
@@ -191,7 +191,6 @@ main(int argc, char **argv)
for (msgnum = 0; msgnum < msgs.size; msgnum++)
if (!m_convert(mp, msgs.msgs[msgnum]))
exit(EX_USAGE);
- seq_setprev(mp); /* set the previous sequence */
if ((nmsgs = read_hdrs(mp, datesw)) <= 0)
adios(EX_DATAERR, NULL, "no messages to sort");
--
2.20.1
From 3919a640096bfcbe2c465f59eb4010dcf1de6056 Mon Sep 17 00:00:00 2001
From: Philipp Takacs <phil...@bureaucracy.de>
Date: Sun, 15 Dec 2019 14:44:57 +0100
Subject: [PATCH 2/2] update unseen sequence after display
before this patch, if messages are displaied with a pager and during
that time a new message comes to the folder. The new message is added
to the unseen sequence. But after closing the pager the unseen sequence
gets overwritten and the new message is removed from the unseen
sequence.
Now the sequence file gets reread before remove the shown messages
from the unseen sequenece.
---
test/common.sh | 11 +++++++
test/tests/show/test-unseen-update | 46 ++++++++++++++++++++++++++++++
uip/mhshow.c | 4 +--
3 files changed, 59 insertions(+), 2 deletions(-)
create mode 100644 test/tests/show/test-unseen-update
diff --git a/test/common.sh b/test/common.sh
index b869d56d..9e6cb226 100644
--- a/test/common.sh
+++ b/test/common.sh
@@ -8,6 +8,17 @@ trap '
' 0 1 2 15
failed=0
+#fake sleeps 60 secounds and then reads all input
+mmh_test_fakepager()
+{
+ sleep 60
+
+ while read a
+ do
+ sleep 0
+ done
+ exit 0
+}
test_skip()
{
diff --git a/test/tests/show/test-unseen-update b/test/tests/show/test-unseen-update
new file mode 100644
index 00000000..a4875c43
--- /dev/null
+++ b/test/tests/show/test-unseen-update
@@ -0,0 +1,46 @@
+#!/bin/sh
+######################################################
+#
+# Test change of sequence during show
+#
+######################################################
+
+. "$MH_TEST_COMMON"
+
+mark -sequence u -add -nozero 1-9
+show 1-8 | mmh_test_fakepager &
+pagerpid=$!
+
+#because the fakepager sleeps 60 secounds the unseen sequence should be unchainged
+#this can fail if the buffer of the pipe is big enouth to hold all messages
+runandcheck "pick u" <<!
+1
+2
+3
+4
+5
+6
+7
+8
+9
+!
+mark -sequence u -add -nozero 10
+runandcheck "pick u" <<!
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+!
+pkill -P $pagerpid
+wait $pagerpid
+
+runandcheck "pick u" <<!
+9
+10
+!
diff --git a/uip/mhshow.c b/uip/mhshow.c
index 6978454e..2a62777a 100644
--- a/uip/mhshow.c
+++ b/uip/mhshow.c
@@ -301,8 +301,6 @@ main(int argc, char **argv)
if (is_selected(mp, msgnum))
set_unseen(mp, msgnum);
- seq_setunseen(mp, 0); /* unset unseen seqs for shown msgs */
-
cts = mh_xcalloc(mp->numsel + 1, sizeof(*cts));
ctp = cts;
@@ -363,6 +361,8 @@ main(int argc, char **argv)
/* If reading from a folder, do some updating */
if (mp) {
+ seq_read(mp);
+ seq_setunseen(mp, 0); /* unset unseen seqs for shown msgs */
context_replace(curfolder, folder); /* update current folder */
seq_setcur(mp, mp->hghsel); /* update current message */
seq_save(mp); /* synchronize sequences */
--
2.20.1
