Refactor function calls: recurrence parameters as a single argument

The recurrence parameters are type, frequency, until date and exception
list (in RFC 5545 parlance FREQ, INTERVAL, UNTIL and EXDATE's). When
these are passed in a function call, the argument list becomes long and
not very readable.  When support for extended recurrence rules is
implemented, the number of recurrence parameters increases, and function
signatures must be amended.

Solution: The "struct rpt" is extended with the exception list; any
future recurrence parameters are added here. A pointer to this structure
replaces the recurrence parameters in function calls.

Note: Each recurrent event and appoinment instance has (a pointer to) a
"struct rpt" and in addition an exception list. The latter is retained
to avoid the derived changes, and the exception list in the structure is
initialized to an empty list when the recurrent instance is created.

Signed-off-by: Lars Henriksen <LarsHenriksen@get2net.dk>
Signed-off-by: Lukas Fleischer <lfleischer@calcurse.org>

5 files changed, 151 insertions, 189 deletions

diff --git a/src/calcurse.h b/src/calcurse.h
index 70a3923..204ee2c 100644
--- a/src/calcurse.h
+++ b/src/calcurse.h
@@ -379,40 +379,44 @@ struct excp {
 };
 
 enum recur_type {
-	RECUR_NO,
 	RECUR_DAILY,
 	RECUR_WEEKLY,
 	RECUR_MONTHLY,
 	RECUR_YEARLY,
-	RECUR_TYPES
+	NBRECUR
 };
 
-/* To describe an item's repetition. */
+/*
+ * Recurrence rule according to RFC5545; used
+ * - in each recurrent appointment/event instance
+ * - in passing parameters as a single function argument
+ */
 struct rpt {
-	enum recur_type type;	/* repetition type */
-	int freq;		/* repetition frequency */
-	time_t until;		/* ending date for repeated event */
+	enum recur_type type;	/* FREQ */
+	int freq;		/* INTERVAL */
+	time_t until;		/* UNTIL */
+	llist_t exc;		/* EXDATE's */
 };
 
 /* Recurrent appointment definition. */
 struct recur_apoint {
-	struct rpt *rpt;	/* information about repetition */
-	llist_t exc;		/* days when the item should not be repeated */
-	time_t start;		/* beggining of the appointment */
-	long dur;		/* duration of the appointment */
-	char state;		/* 8 bits to store item state */
-	char *mesg;		/* appointment description */
-	char *note;		/* note attached to appointment */
+	struct rpt *rpt;	/* recurrence rule */
+	llist_t exc;		/* recurrence exceptions (NOT rpt->exc) */
+	time_t start;		/* start time */
+	long dur;		/* duration */
+	char state;		/* item state */
+	char *mesg;		/* description */
+	char *note;		/* attached note */
 };
 
-/* Reccurent event definition. */
+/* Recurrent event definition. */
 struct recur_event {
-	struct rpt *rpt;	/* information about repetition */
-	llist_t exc;		/* days when the item should not be repeated */
+	struct rpt *rpt;	/* recurrence rule */
+	llist_t exc;		/* recurrence exceptions (NOT rpt->exc) */
 	int id;			/* event type */
-	time_t day;		/* day at which event occurs */
-	char *mesg;		/* event description */
-	char *note;		/* note attached to event */
+	time_t day;		/* day of the event */
+	char *mesg;		/* description */
+	char *note;		/* attached note */
 };
 
 /* Generic pointer data type for appointments and events. */
@@ -1042,17 +1046,16 @@ void recur_event_llist_init(void);
 void recur_apoint_llist_free(void);
 void recur_event_llist_free(void);
 struct recur_apoint *recur_apoint_new(char *, char *, time_t, long, char,
-				      int, int, time_t, llist_t *);
-struct recur_event *recur_event_new(char *, char *, time_t, int, int, int,
-				    time_t, llist_t *);
+				      struct rpt *);
+struct recur_event *recur_event_new(char *, char *, time_t, int,
+				     struct rpt *);
 char recur_def2char(enum recur_type);
 int recur_char2def(char);
-struct recur_apoint *recur_apoint_scan(FILE *, struct tm, struct tm,
-				       char, int, struct tm, char *,
-				       llist_t *, char, struct item_filter *);
-struct recur_event *recur_event_scan(FILE *, struct tm, int, char,
-				     int, struct tm, char *, llist_t *,
-				     struct item_filter *);
+struct recur_apoint *recur_apoint_scan(FILE *, struct tm, struct tm, char,
+				       char *, struct item_filter *,
+				       struct rpt *);
+struct recur_event *recur_event_scan(FILE *, struct tm, int, char *,
+				     struct item_filter *, struct rpt *);
 char *recur_apoint_tostr(struct recur_apoint *);
 char *recur_apoint_hash(struct recur_apoint *);
 void recur_apoint_write(struct recur_apoint *, FILE *);
diff --git a/src/ical.c b/src/ical.c
index 521ce0c..fa9a663 100644
--- a/src/ical.c
+++ b/src/ical.c
@@ -78,8 +78,8 @@ static void ical_export_apoints(FILE *, int);
 static void ical_export_todo(FILE *, int);
 static void ical_export_footer(FILE *);
 
-static const char *ical_recur_type[RECUR_TYPES] =
-    { "", "DAILY", "WEEKLY", "MONTHLY", "YEARLY" };
+static const char *ical_recur_type[NBRECUR] =
+    { "DAILY", "WEEKLY", "MONTHLY", "YEARLY" };
 
 /* Escape characters in field before printing */
 static void ical_format_line(FILE * stream, char * field, char * msg)
@@ -375,17 +375,21 @@ static void ical_store_todo(int priority, int completed, char *mesg,
 
 static void
 ical_store_event(char *mesg, char *note, long day, long end,
-		 ical_rpt_t * rpt, llist_t * exc, const char *fmt_ev,
+		 ical_rpt_t *irpt, llist_t *exc, const char *fmt_ev,
 		 const char *fmt_rev)
 {
 	const int EVENTID = 1;
 	struct event *ev;
 	struct recur_event *rev;
 
-	if (rpt) {
-		rev = recur_event_new(mesg, note, day, EVENTID, rpt->type,
-				      rpt->freq, rpt->until, exc);
-		mem_free(rpt);
+	if (irpt) {
+		struct rpt rpt;
+		rpt.type = irpt->type;
+		rpt.freq = irpt->freq;
+		rpt.until = irpt->until;
+		rpt.exc = *exc;
+		rev = recur_event_new(mesg, note, day, EVENTID, &rpt);
+		mem_free(irpt);
 		if (fmt_rev)
 			print_recur_event(fmt_rev, day, rev);
 		goto cleanup;
@@ -406,14 +410,12 @@ ical_store_event(char *mesg, char *note, long day, long end,
 	 * event starts, so we need to do some conversion here.
 	 */
 	end = day + ((end - day - 1) / DAYINSEC) * DAYINSEC;
-	rpt = mem_malloc(sizeof(ical_rpt_t));
-	rpt->type = RECUR_DAILY;
-	rpt->freq = 1;
-	rpt->count = 0;
-	rpt->until = end;
-	rev = recur_event_new(mesg, note, day, EVENTID, rpt->type,
-			      rpt->freq, rpt->until, exc);
-	mem_free(rpt);
+	struct rpt rpt;
+	rpt.type = RECUR_DAILY;
+	rpt.freq = 1;
+	rpt.until = end;
+	rpt.exc = *exc;
+	rev = recur_event_new(mesg, note, day, EVENTID, &rpt);
 	if (fmt_rev)
 		print_recur_event(fmt_rev, day, rev);
 
@@ -424,7 +426,7 @@ cleanup:
 
 static void
 ical_store_apoint(char *mesg, char *note, long start, long dur,
-		  ical_rpt_t * rpt, llist_t * exc, int has_alarm,
+		  ical_rpt_t * irpt, llist_t * exc, int has_alarm,
 		  const char *fmt_apt, const char *fmt_rapt)
 {
 	char state = 0L;
@@ -433,10 +435,14 @@ ical_store_apoint(char *mesg, char *note, long start, long dur,
 
 	if (has_alarm)
 		state |= APOINT_NOTIFY;
-	if (rpt) {
-		rapt = recur_apoint_new(mesg, note, start, dur, state,
-					rpt->type, rpt->freq, rpt->until, exc);
-		mem_free(rpt);
+	if (irpt) {
+		struct rpt rpt;
+		rpt.type = irpt->type;
+		rpt.freq = irpt->freq;
+		rpt.until = irpt->until;
+		rpt.exc = *exc;
+		rapt = recur_apoint_new(mesg, note, start, dur, state, &rpt);
+		mem_free(irpt);
 		if (fmt_rapt)
 			print_recur_apoint(fmt_rapt, start, rapt->start, rapt);
 	} else {
diff --git a/src/io.c b/src/io.c
index 1782105..725b31b 100644
--- a/src/io.c
+++ b/src/io.c
@@ -553,10 +553,9 @@ void io_load_app(struct item_filter *filter)
 	FILE *data_file;
 	int c, is_appointment, is_event, is_recursive;
 	struct tm start, end, until, lt;
-	llist_t exc;
+	struct rpt rpt;
 	time_t t;
 	int id = 0;
-	int freq;
 	char type, state = 0L;
 	char note[MAX_NOTESIZ + 1], *notep;
 	unsigned line = 0;
@@ -572,7 +571,6 @@ void io_load_app(struct item_filter *filter)
 	rewind(data_file);
 
 	for (;;) {
-		LLIST_INIT(&exc);
 		is_appointment = is_event = is_recursive = 0;
 		line++;
 		c = getc(data_file);
@@ -630,96 +628,83 @@ void io_load_app(struct item_filter *filter)
 
 		if (c == '{') {
 			is_recursive = 1;
-			if (fscanf(data_file, " %d%c ", &freq, &type) != 2)
+			if (fscanf(data_file, " %d%c ", &rpt.freq, &type) != 2)
 				io_load_error(path_apts, line,
 					      _("syntax error in item repetition"));
-
+			else
+				rpt.type = recur_char2def(type);
 			c = getc(data_file);
-			if (c == '}') {	/* endless recurrent item */
-				until.tm_year = 0;
-				while ((c = getc(data_file)) == ' ') ;
-				ungetc(c, data_file);
-			} else if (c == '-' && getc(data_file) == '>') {
+			/* Optional until date */
+			if (c == '-' && getc(data_file) == '>') {
 				if (fscanf
 				    (data_file, " %d / %d / %d ",
 				     &until.tm_mon, &until.tm_mday,
 				     &until.tm_year) != 3)
 					io_load_error(path_apts, line,
 						      _("syntax error in item repetition"));
-				c = getc(data_file);
-				if (c == '!') {
-					ungetc(c, data_file);
-					recur_exc_scan(&exc, data_file);
-					while ((c =
-						getc(data_file)) == ' ') ;
-					ungetc(c, data_file);
-				} else if (c == '}') {
-					while ((c =
-						getc(data_file)) == ' ') ;
-					ungetc(c, data_file);
-				} else {
+				if (!check_date(until.tm_year, until.tm_mon,
+						until.tm_mday))
 					io_load_error(path_apts, line,
-						      _("syntax error in item repetition"));
-				}
-			} else if (c == '!') {	/* endless item with exceptions */
-				ungetc(c, data_file);
-				recur_exc_scan(&exc, data_file);
-				while ((c = getc(data_file)) == ' ') ;
+						      _("until date error"));
+				until.tm_hour = 0;
+				until.tm_min = 0;
+				until.tm_sec = 0;
+				until.tm_isdst = -1;
+				until.tm_year -= 1900;
+				until.tm_mon--;
+				rpt.until = mktime(&until);
+				c = getc(data_file);
+			} else
+				rpt.until = 0;
+			/* Optional exception dates */
+			if (c == '!') {
 				ungetc(c, data_file);
-				until.tm_year = 0;
-			} else {
+				recur_exc_scan(&rpt.exc, data_file);
+				c = getc(data_file);
+			} else
+				LLIST_INIT(&rpt.exc);
+			/* End of recurrence rule */
+			if (c != '}')
 				io_load_error(path_apts, line,
-					      _("wrong format in the appointment or event"));
-				/* NOTREACHED */
-			}
-		} else {
-			ungetc(c, data_file);
+					      _("missing end of recurrence"));
+			while ((c = getc(data_file)) == ' ') ;
 		}
 
 		/* Check if a note is attached to the item. */
-		c = getc(data_file);
 		if (c == '>') {
 			note_read(note, data_file);
+			c = getc(data_file);
 			notep = note;
-		} else {
+		} else
 			notep = NULL;
-			ungetc(c, data_file);
-		}
 
 		/*
 		 * Last: read the item description and load it into its
 		 * corresponding linked list, depending on the item type.
 		 */
 		if (is_appointment) {
-			c = getc(data_file);
-			if (c == '!') {
+			if (c == '!')
 				state |= APOINT_NOTIFY;
-				while ((c = getc(data_file)) == ' ') ;
-				ungetc(c, data_file);
-			} else if (c == '|') {
+			else if (c == '|')
 				state = 0L;
-				while ((c = getc(data_file)) == ' ') ;
-				ungetc(c, data_file);
-			} else {
+			else
 				io_load_error(path_apts, line,
-					      _("syntax error in item repetition"));
-			}
+					      _("syntax error in item state"));
+
 			if (is_recursive) {
-				recur_apoint_scan(data_file, start, end,
-						  type, freq, until, notep,
-						  &exc, state, filter);
+				recur_apoint_scan(data_file, start, end, state,
+						  notep, filter, &rpt);
 			} else {
 				apoint_scan(data_file, start, end, state,
 					    notep, filter);
 			}
 		} else if (is_event) {
+			ungetc(c, data_file);
 			if (is_recursive) {
-				recur_event_scan(data_file, start, id,
-						 type, freq, until, notep,
-						 &exc, filter);
+				recur_event_scan(data_file, start, id, notep,
+						 filter, &rpt);
 			} else {
-				event_scan(data_file, start, id, notep,
-					   filter);
+				event_scan(data_file, start, id, notep, filter);
 			}
 		} else {
 			io_load_error(path_apts, line,
diff --git a/src/recur.c b/src/recur.c
index cad8345..ff3be45 100644
--- a/src/recur.c
+++ b/src/recur.c
@@ -261,28 +261,25 @@ static int recur_event_cmp(struct recur_event *a, struct recur_event *b)
 
 /* Insert a new recursive appointment in the general linked list */
 struct recur_apoint *recur_apoint_new(char *mesg, char *note, time_t start,
-				      long dur, char state, int type,
-				      int freq, time_t until,
-				      llist_t * except)
+				      long dur, char state, struct rpt *rpt)
 {
 	struct recur_apoint *rapt =
 	    mem_malloc(sizeof(struct recur_apoint));
 
-	rapt->rpt = mem_malloc(sizeof(struct rpt));
 	rapt->mesg = mem_strdup(mesg);
 	rapt->note = (note != NULL) ? mem_strdup(note) : 0;
 	rapt->start = start;
-	rapt->state = state;
 	rapt->dur = dur;
-	rapt->rpt->type = type;
-	rapt->rpt->freq = freq;
-	rapt->rpt->until = until;
-	if (except) {
-		exc_dup(&rapt->exc, except);
-		free_exc_list(except);
-	} else {
-		LLIST_INIT(&rapt->exc);
-	}
+	rapt->state = state;
+	rapt->rpt = mem_malloc(sizeof(struct rpt));
+	*rapt->rpt = *rpt;
+	/*
+	 * Note. The exception dates are in the list rapt->exc.
+	 * The (empty) list rapt->rpt->exc is not used.
+	 */
+	exc_dup(&rapt->exc, &rpt->exc);
+	free_exc_list(&rpt->exc);
+	LLIST_INIT(&rapt->rpt->exc);
 
 	LLIST_TS_LOCK(&recur_alist_p);
 	LLIST_TS_ADD_SORTED(&recur_alist_p, rapt, recur_apoint_cmp);
@@ -293,25 +290,20 @@ struct recur_apoint *recur_apoint_new(char *mesg, char *note, time_t start,
 
 /* Insert a new recursive event in the general linked list */
 struct recur_event *recur_event_new(char *mesg, char *note, time_t day,
-				    int id, int type, int freq, time_t until,
-				    llist_t * except)
+				    int id, struct rpt *rpt)
 {
 	struct recur_event *rev = mem_malloc(sizeof(struct recur_event));
 
-	rev->rpt = mem_malloc(sizeof(struct rpt));
 	rev->mesg = mem_strdup(mesg);
 	rev->note = (note != NULL) ? mem_strdup(note) : 0;
 	rev->day = day;
 	rev->id = id;
-	rev->rpt->type = type;
-	rev->rpt->freq = freq;
-	rev->rpt->until = until;
-	if (except) {
-		exc_dup(&rev->exc, except);
-		free_exc_list(except);
-	} else {
-		LLIST_INIT(&rev->exc);
-	}
+	rev->rpt = mem_malloc(sizeof(struct rpt));
+	*rev->rpt = *rpt;
+	/* Similarly as for recurrent appointment. */
+	exc_dup(&rev->exc, &rpt->exc);
+	free_exc_list(&rpt->exc);
+	LLIST_INIT(&rev->rpt->exc);
 
 	LLIST_ADD_SORTED(&recur_elist, rev, recur_event_cmp);
 
@@ -395,24 +387,20 @@ static void recur_exc_append(struct string *s, llist_t *lexc)
 }
 
 /* Load the recursive appointment description */
-struct recur_apoint *recur_apoint_scan(FILE * f, struct tm start,
-				       struct tm end, char type, int freq,
-				       struct tm until, char *note,
-				       llist_t * exc, char state,
-				       struct item_filter *filter)
+struct recur_apoint *recur_apoint_scan(FILE *f, struct tm start, struct tm end,
+				       char state, char *note,
+				       struct item_filter *filter,
+				       struct rpt *rpt)
 {
 	char buf[BUFSIZ], *nl;
-	time_t tstart, tend, tuntil;
+	time_t tstart, tend;
 	struct recur_apoint *rapt = NULL;
 	int cond;
 
 	EXIT_IF(!check_date(start.tm_year, start.tm_mon, start.tm_mday) ||
 		!check_date(end.tm_year, end.tm_mon, end.tm_mday) ||
 		!check_time(start.tm_hour, start.tm_min) ||
-		!check_time(end.tm_hour, end.tm_min) ||
-		(until.tm_year != 0
-		 && !check_date(until.tm_year, until.tm_mon,
-				until.tm_mday)),
+		!check_time(end.tm_hour, end.tm_min),
 		_("date error in appointment"));
 
 	/* Read the appointment description */
@@ -432,19 +420,8 @@ struct recur_apoint *recur_apoint_scan(FILE * f, struct tm start,
 	tstart = mktime(&start);
 	tend = mktime(&end);
 
-	if (until.tm_year != 0) {
-		until.tm_hour = 0;
-		until.tm_min = 0;
-		until.tm_sec = 0;
-		until.tm_isdst = -1;
-		until.tm_year -= 1900;
-		until.tm_mon--;
-		tuntil = mktime(&until);
-	} else {
-		tuntil = 0;
-	}
-	EXIT_IF(tstart == -1 || tend == -1 || tstart > tend
-		|| tuntil == -1, _("date error in appointment"));
+	EXIT_IF(tstart == -1 || tend == -1 || tstart > tend,
+		 _("date error in appointment"));
 
 	/* Filter item. */
 	if (filter) {
@@ -458,9 +435,8 @@ struct recur_apoint *recur_apoint_scan(FILE * f, struct tm start,
 		);
 		if (filter->hash) {
 			rapt = recur_apoint_new(buf, note, tstart,
-						tend - tstart, state,
-						recur_char2def(type),
-						freq, tuntil, exc);
+						 tend - tstart, state,
+						 rpt);
 			char *hash = recur_apoint_hash(rapt);
 			cond = cond || !hash_matches(filter->hash, hash);
 			mem_free(hash);
@@ -473,29 +449,25 @@ struct recur_apoint *recur_apoint_scan(FILE * f, struct tm start,
 		}
 	}
 	if (!rapt)
-		rapt = recur_apoint_new(buf, note, tstart, tend - tstart,
-					state, recur_char2def(type), freq,
-					tuntil, exc);
+		rapt = recur_apoint_new(buf, note, tstart, tend - tstart, state,
+					 rpt);
 
 	return rapt;
 }
 
 /* Load the recursive events from file */
 struct recur_event *recur_event_scan(FILE * f, struct tm start, int id,
-				     char type, int freq, struct tm until,
-				     char *note, llist_t * exc,
-				     struct item_filter *filter)
+				     char *note, struct item_filter *filter,
+				     struct rpt *rpt)
 {
 	char buf[BUFSIZ], *nl;
-	time_t tstart, tend, tuntil;
+	time_t tstart, tend;
 	struct recur_event *rev = NULL;
 	int cond;
 
 	EXIT_IF(!check_date(start.tm_year, start.tm_mon, start.tm_mday) ||
-		!check_time(start.tm_hour, start.tm_min) ||
-		(until.tm_year != 0
-		 && !check_date(until.tm_year, until.tm_mon,
-				until.tm_mday)), _("date error in event"));
+		!check_time(start.tm_hour, start.tm_min),
+		("date error in event"));
 
 	/* Read the event description */
 	if (!fgets(buf, sizeof buf, f))
@@ -505,22 +477,16 @@ struct recur_event *recur_event_scan(FILE * f, struct tm start, int id,
 	if (nl) {
 		*nl = '\0';
 	}
-	start.tm_hour = until.tm_hour = 0;
-	start.tm_min = until.tm_min = 0;
-	start.tm_sec = until.tm_sec = 0;
-	start.tm_isdst = until.tm_isdst = -1;
+	start.tm_hour = 0;
+	start.tm_min = 0;
+	start.tm_sec = 0;
+	start.tm_isdst = -1;
 	start.tm_year -= 1900;
 	start.tm_mon--;
-	if (until.tm_year != 0) {
-		until.tm_year -= 1900;
-		until.tm_mon--;
-		tuntil = mktime(&until);
-	} else {
-		tuntil = 0;
-	}
+
 	tstart = mktime(&start);
-	EXIT_IF(tstart == -1 || tuntil == -1, _("date error in event"));
-	tend = tstart + DAYINSEC - 1;
+	EXIT_IF(tstart == -1, _("date error in event"));
+	tend = ENDOFDAY(tstart);
 
 	/* Filter item. */
 	if (filter) {
@@ -534,8 +500,7 @@ struct recur_event *recur_event_scan(FILE * f, struct tm start, int id,
 		);
 		if (filter->hash) {
 			rev = recur_event_new(buf, note, tstart, id,
-					      recur_char2def(type),
-					      freq, tuntil, exc);
+					       rpt);
 			char *hash = recur_event_hash(rev);
 			cond = cond || !hash_matches(filter->hash, hash);
 			mem_free(hash);
@@ -548,9 +513,7 @@ struct recur_event *recur_event_scan(FILE * f, struct tm start, int id,
 		}
 	}
 	if (!rev)
-		rev = recur_event_new(buf, note, tstart, id,
-				      recur_char2def(type),
-				      freq, tuntil, exc);
+		rev = recur_event_new(buf, note, tstart, id, rpt);
 
 	return rev;
 }
@@ -977,6 +940,7 @@ void recur_exc_scan(llist_t * lexc, FILE * data_file)
 		exc->st = mktime(&day);
 		LLIST_ADD(lexc, exc);
 	}
+	ungetc(c, data_file);
 }
 
 /*
diff --git a/src/ui-day.c b/src/ui-day.c
index 7d571fc..cb038d1 100644
--- a/src/ui-day.c
+++ b/src/ui-day.c
@@ -975,16 +975,20 @@ void ui_day_item_repeat(void)
 
 	/* Set the selected APP item. */
 	struct day_item d = empty_day;
+	struct rpt rpt;
+	rpt.type = type;
+	rpt.freq = freq;
+	rpt.until = until;
+	LLIST_INIT(&rpt.exc);
 	if (p->type == EVNT) {
 		struct event *ev = p->item.ev;
 		d.item.rev = recur_event_new(ev->mesg, ev->note, ev->day,
-					     ev->id, type, freq, until, NULL);
+					      ev->id, &rpt);
 	} else if (p->type == APPT) {
 		struct apoint *apt = p->item.apt;
 		d.item.rapt = ra = recur_apoint_new(apt->mesg, apt->note,
 						    apt->start, apt->dur,
-						    apt->state, type, freq,
-						    until, NULL);
+						    apt->state, &rpt);
 		if (notify_bar())
 			notify_check_repeated(ra);
 	} else {