Re: [9front] [patch] upas: plumb flag updates on messages.

[ori@eigenstate.org]

>> ok, small remarks.
>> 
>> +			if(doplumb){
>> +				if((m->cstate & Cnew) && ensurecache(mb, m) == 0)
>>  					msgdecref(mb, m);
>> 
>> i think you want to do the msgdecref() of the message *after* mailplumb(),
>> not before. otherwise its like free before use. i know with the current
>> code it will still work but it is semantially wrong. if we later change
>> how cache eviction works that will blow up.
> 
> Ok. I was looking at the code, and it *seemed* like the intent was
> to have even evicted messages be somewhat valid. I think there are a
> number of places where we manipulate seen but not cached messages.
> 
>> +		print("child\n");
>> +		if(mboxfile != nil)
>> +			if(err = newmbox(mboxfile, "mbox", 0, nil))
>> +				sysfatal("opening %s: %s", mboxfile, err);
> 
>> theres a debug print left there and this changes some semantics.
>> i know this is because initial sync from imap takes forever. but
>> people sometimes want to specify mailbox path at the initial
>> command line and expect error status if the initial sync failed
>> (like if mounting a mbox file in a script to parse a message).
> 
> Right, forgot about this change. Reverted -- It doesn't even
> make things respon faster, since the mount doesn't return until
> after we do the initial sync.
> 
>> i think a good compromise would be to add a commandline flag to
>> specify if you want initial sync in the background or not
>> (can then be added to /rc/bin/startupasfs).
>>
>> if we do background syncing, then even if the initial sync
>> fails, it should try to continue. imagine your imap server
>> is currently down or network is unavailable, you want it to
>> continue trying in that case. and sysfataling here makes no
>> sense as nobody will be listening.
> 
> Yes. background sync will need a bunch more work, and a lot
> more thought to get right.
> 
> Updated patch below, slipped in 2 small changes:
> 
>  - Acme Mail => Next button to take you to the next msg
>  - Imap => small consistency tweak: flags are uint.
> 
> Can back them out if you care about separating them.
> 

One more revision, now without typoed redundant checks.

diff -r eda8210ca3fc sys/src/cmd/faces/plumb.c
--- a/sys/src/cmd/faces/plumb.c	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/faces/plumb.c	Mon Dec 09 10:27:56 2019 -0800
@@ -273,13 +273,18 @@
 			if(m == nil)
 				killall("error on seemail plumb port");
 			t = value(m->attr, "mailtype", "");
-			if(strcmp(t, "delete") == 0)
+			if(strcmp(t, "modify") == 0)
+				goto Ignore;
+			else if(strcmp(t, "delete") == 0)
 				delete(m->data, value(m->attr, "digest", nil));
-			else if(strcmp(t, "new") != 0)
+			else if(strcmp(t, "new") == 0)
+				for(i=0; i<nmaildirs; i++){
+					if(strncmp(m->data, maildirs[i], strlen(maildirs[i])) == 0)
+						goto Found;
+				}
+			else
 				fprint(2, "faces: unknown plumb message type %s\n", t);
-			else for(i=0; i<nmaildirs; i++)
-				if(strncmp(m->data, maildirs[i], strlen(maildirs[i])) == 0)
-					goto Found;
+		Ignore:
 			plumbfree(m);
 			continue;
 
diff -r eda8210ca3fc sys/src/cmd/upas/Mail/dat.h
--- a/sys/src/cmd/upas/Mail/dat.h	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/upas/Mail/dat.h	Mon Dec 09 10:27:56 2019 -0800
@@ -124,6 +124,7 @@
 
 extern	int		mesgadd(Message*, char*, Dir*, char*);
 extern	void		mesgmenu(Window*, Message*);
+extern	void		mesgmenuselect(Window*, Message*);
 extern	void		mesgmenunew(Window*, Message*);
 extern	void		mesgmenureflag(Window*, Message*);
 extern	int		mesgopen(Message*, char*, char*, Message*, int, char*);
diff -r eda8210ca3fc sys/src/cmd/upas/Mail/mail.c
--- a/sys/src/cmd/upas/Mail/mail.c	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/upas/Mail/mail.c	Mon Dec 09 10:27:56 2019 -0800
@@ -189,7 +189,7 @@
 
 	wbox = newwindow();
 	winname(wbox, mbox.name);
-	wintagwrite(wbox, "Put Mail Delmesg Save ", 3+1+4+1+7+1+4+1);
+	wintagwrite(wbox, "Put Mail Delmesg Save Next ", 3+1+4+1+7+1+4+1+4+1);
 	threadcreate(mainctl, wbox, STACK);
 
 	fmtstrinit(&fmt);
@@ -317,6 +317,22 @@
 	}
 }
 
+void
+modmesg(char *name, char *digest)
+{
+	Message *m;
+	char *flags;
+
+	if((m = mesglookupfile(&mbox, name, digest)) == nil)
+		return;
+	if((flags = readfile(name, "/flags", nil)) == nil)
+		return;
+	free(m->flags);
+	m->flags = flags;
+	mesgmenureflag(mbox.w, m);
+}
+	
+
 extern int mesgsave(Message*, char*);
 void
 savemesg(char *box, char *name, char *digest)
@@ -361,6 +377,8 @@
 			newmesg(m->data, digest);
 		else if(strcmp(type, "delete") == 0)
 			delmesg(m->data, digest, 0);
+		else if(strcmp(type, "modify") == 0)
+			modmesg(m->data, digest);
 		else
 			fprint(2, "Mail: unknown plumb attribute %s\n", type);
 		plumbfree(m);
@@ -513,6 +531,11 @@
 		free(targs);
 		return 1;
 	}
+	if(strcmp(args[0], "Next") == 0){
+		ctlprint(w->ctl, "addr=dot\n");
+		winselect(w, "/^[0-9]*\\/ \\[\\*.*(\\n(\t.*)*)/", 1);
+		ctlprint(w->ctl, "show\n");
+	}
 	return 0;
 }
 
diff -r eda8210ca3fc sys/src/cmd/upas/fs/dat.h
--- a/sys/src/cmd/upas/fs/dat.h	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/upas/fs/dat.h	Mon Dec 09 10:27:56 2019 -0800
@@ -7,6 +7,7 @@
 	Cheader 	= 1<<2,
 	Cbody		= 1<<3,
 	Cnew		= 1<<4,
+	Cmod		= 1<<5,
 
 	/* encodings */
 	Enone=	0,
diff -r eda8210ca3fc sys/src/cmd/upas/fs/imap.c
--- a/sys/src/cmd/upas/fs/imap.c	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/upas/fs/imap.c	Mon Dec 09 10:27:56 2019 -0800
@@ -35,7 +35,7 @@
 	uvlong	uid;
 	ulong	sizes;
 	ulong	dates;
-	ulong	flags;
+	uint	flags;
 } Fetchi;
 
 typedef struct Imap Imap;
@@ -1006,6 +1006,8 @@
 			m->deleted = Disappear;
 			ll = &m->next;
 		}else{
+			if((m->flags & ~Frecent) != (f[i].flags & ~Frecent))
+				m->cstate |= Cmod;
 			m->flags = f[i].flags;
 			ll = &m->next;
 			i++;
diff -r eda8210ca3fc sys/src/cmd/upas/fs/mbox.c
--- a/sys/src/cmd/upas/fs/mbox.c	Mon Dec 09 02:03:10 2019 +0100
+++ b/sys/src/cmd/upas/fs/mbox.c	Mon Dec 09 10:27:56 2019 -0800
@@ -58,9 +58,6 @@
 static void delmessage(Mailbox*, Message*);
 static void mailplumb(Mailbox*, Message*);
 
-/*
- * do we want to plumb flag changes?
- */
 char*
 syncmbox(Mailbox *mb, int doplumb)
 {
@@ -90,16 +87,13 @@
 				cachehash(mb, m);
 				m->cstate |= Cnew;
 				n++;
-			} else if(!doplumb)
-				m->cstate &= ~Cnew;
-			if(m->cstate & Cnew){
-				if(ensurecache(mb, m) == 0){
-					if(doplumb)
-						mailplumb(mb, m);
-					msgdecref(mb, m);
-				}
-				m->cstate &= ~Cnew;
 			}
+			if((m->cstate & Cnew|Cmod) && ensurecache(mb, m) == 0){
+				if(doplumb)
+					mailplumb(mb, m);
+				msgdecref(mb, m);
+			}
+			m->cstate &= ~(Cnew|Cmod);
 		}
 		if(m->cstate & Cidxstale)
 			y++;
@@ -1555,7 +1549,12 @@
 	a[ai-1].next = &a[ai];
 
 	a[++ai].name = "mailtype";
-	a[ai].value = !m->inmbox ? "delete": "new";
+	if(m->cstate & Cmod)
+		a[ai].value = "modify";
+	else if (!m->inmbox)
+		a[ai].value = "delete";
+	else
+		a[ai].value = "new";
 	a[ai-1].next = &a[ai];
 
 	snprint(date, sizeof date, "%Δ", m->fileid);