From: patrick keshishian Subject: Re: cal: add option to highlight the current day To: Job Snijders Cc: Johannes Thyssen Tishman , tech@openbsd.org, landry@openbsd.org Date: Sat, 4 Jul 2026 16:36:45 -0700 On Sat, Jul 4, 2026 at 2:44 AM Job Snijders wrote: > Hi Johannes, > > Thanks! I've longed for this functionality for some time now. Some > comments below. > > On Fri, Jul 03, 2026 at 04:37:29PM +0000, Johannes Thyssen Tishman wrote: > > Please find below a patch to add an option (-h) to cal to allow > > highlighting the current day. > > > > For testing purposes, the -h flag takes an optional argument specifying > > the date to highlight, e.g., cal -h2026-10-10 oct. I intend to remove > > this before committing if the patch is accepted. > > Since the program tests whether standout highlighting can be used, to > me it would make most sense to highlight the current day by default. > Highlighting of arbitrary days seems less useful. I suggest remove '-h' > (bonus: it reduces the size of patch). > Please no. The highlighting should not be default, but if you insist making this feature the default, please allow and option to disable this highlighting. Sorry, not a fan of these sorts of extras. --patrick > > > The highlight_day function was mostly taken from FreeBSD[1]. Their > > implementation falls back to underlining the date (IIUC) if the terminal > > has no standout mode. I decided to leave this out for now and simply > > ignore the -h flag in this case. > > Good idea. > > > To test this and check for regressions, I used the attached script. The > > script tests the in-tree patched cal against the unpatched installed > > one. It probably doesn't cover all scenarios, but hopefully enough. In > > fact, it is probably a little excessive, as it tests every day of 28 > > years, hehe. There's probably a smarter way to do this. > > > > Lastly, I'm not super confident in my C skills yet so please don't hold > > back on your feedback. I've added some comments with questions in parts > > I'm not so sure about. I'd be grateful for any input on these. > > I recommend removing all the debug code to make the proposed changeset > more concise. Please see below for some more comments. > > > Index: Makefile > > =================================================================== > > RCS file: /cvs/src/usr.bin/cal/Makefile,v > > diff -u -p -r1.3 Makefile > > --- Makefile 21 Sep 1997 11:48:29 -0000 1.3 > > +++ Makefile 3 Jul 2026 15:22:08 -0000 > > @@ -1,5 +1,6 @@ > > # $OpenBSD: Makefile,v 1.3 1997/09/21 11:48:29 deraadt Exp $ > > > > -PROG= cal > > +PROG= cal > > +LDADD+= -lcurses > > > > .include > > Index: cal.1 > > =================================================================== > > RCS file: /cvs/src/usr.bin/cal/cal.1,v > > diff -u -p -r1.34 cal.1 > > --- cal.1 2 Jul 2026 20:40:53 -0000 1.34 > > +++ cal.1 3 Jul 2026 15:22:08 -0000 > > @@ -41,7 +41,7 @@ > > .Nd displays a calendar > > .Sh SYNOPSIS > > .Nm cal > > -.Op Fl jmwy > > +.Op Fl hjmwy > > .Op Ar month > > .Op Ar year > > .Sh DESCRIPTION > > @@ -51,6 +51,8 @@ Calendars may be displayed by month or b > > .Pp > > The options are as follows: > > .Bl -tag -width Ds > > +.It Fl h > > +Highlight the current day. > > .It Fl j > > Display Julian dates (days one-based, numbered from January 1). > > The options > > If -h is removed man page update not needed > > > Index: cal.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/cal/cal.c,v > > diff -u -p -r1.36 cal.c > > --- cal.c 2 Jul 2026 20:40:53 -0000 1.36 > > +++ cal.c 3 Jul 2026 15:22:08 -0000 > > @@ -42,6 +42,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > > > #define THURSDAY 4 /* for reformation > */ > > #define SATURDAY 6 /* 1 Jan 1 was a > Saturday */ > > @@ -127,8 +129,11 @@ const char *day_headings = NULL; > > int julian; > > int mflag = 0; > > int wflag = 0; > > +int highlight = 0; > > +struct tm *local_time; > > > > void ascii_day(char *, int); > > +void highlight_day(char *, int, int *); > > void center(const char *, int, int); > > void day_array(int, int, int *); > > int day_in_week(int, int, int); > > @@ -141,21 +146,26 @@ void trim_trailing_spaces(char *); > > void usage(void); > > void yearly(int); > > int parsemonth(const char *); > > +int is_today(int, int, int); > > > > int > > main(int argc, char *argv[]) > > { > > - struct tm *local_time; > > time_t now; > > int ch, month, year, yflag; > > const char *errstr; > > + const char *hl_date = NULL; /* debug only */ > > > > - if (pledge("stdio", NULL) == -1) > > + if (pledge("stdio rpath tty", NULL) == -1) > > err(1, "pledge"); > > > > yflag = year = 0; > > - while ((ch = getopt(argc, argv, "jmwy")) != -1) > > + while ((ch = getopt(argc, argv, "h::jmwy")) != -1) > > switch(ch) { > > + case 'h': > > + hl_date = optarg; /* debug only */ > > + highlight = isatty(STDOUT_FILENO); > > + break; > > > It might be better to move the detection of highlighting ability into > a separate function. In that new function, not only use isatty() to > help determine the nature of the terminal, but also check whether > tgetstr("so", &ptr) and tgetstr("se", &ptr) return 1. > > > > case 'j': > > julian = 1; > > break; > > @@ -190,6 +200,13 @@ main(int argc, char *argv[]) > > day_headings = DAY_HEADINGS_JS; > > } > > > > + (void)time(&now); > > + local_time = localtime(&now); > > + if (local_time == NULL) { > > + perror("localtime"); > > + exit(1); > > + } > > + > > month = 0; > > switch(argc) { > > case 2: > > @@ -202,8 +219,6 @@ main(int argc, char *argv[]) > > if (yflag) > > errx(1, "specifying a month conflicts with > -y"); > > month = parsemonth(*argv); > > - (void)time(&now); > > - local_time = localtime(&now); > > year = local_time->tm_year + 1900; > > } else { > > year = strtonum(*argv, 1, 9999, &errstr); > > @@ -212,8 +227,6 @@ main(int argc, char *argv[]) > > } > > break; > > case 0: > > - (void)time(&now); > > - local_time = localtime(&now); > > year = local_time->tm_year + 1900; > > if (!yflag) > > month = local_time->tm_mon + 1; > > @@ -222,6 +235,28 @@ main(int argc, char *argv[]) > > usage(); > > } > > > > + /* debug only */ > > + if (hl_date != NULL) { > > + if (sscanf(hl_date, "%4d-%2d-%2d", &local_time->tm_year, > > + &local_time->tm_mon, &local_time->tm_mday) != 3 || > > + strlen(hl_date) != 10) > > + errx(1, "illegal date format: use yyyy-mm-dd"); > > + > > + if (local_time->tm_year < 1 || local_time->tm_mon < 1 || > > + local_time->tm_mday < 1 || local_time->tm_year > 9999 > || > > + local_time->tm_mon > 12 || local_time->tm_mday > 31) > > + errx(1, "illegal date format: use yyyy-mm-dd"); > > + > > + local_time->tm_year -= 1900; > > + local_time->tm_mon -= 1; > > + local_time->tm_hour = 0; > > + local_time->tm_min = 0; > > + local_time->tm_sec = 0; > > + local_time->tm_isdst = -1; > > + if (mktime(local_time) == -1 && local_time->tm_wday == -1) > > + errx(1, "specified date is outside allowed range"); > > + } > > + > > if (month) > > monthly(month, year); > > else if (julian) > > @@ -298,8 +333,13 @@ isoweek(int day, int month, int year) > > void > > monthly(int month, int year) > > { > > - int col, row, len, days[MAXDAYS], firstday; > > - char *p, lineout[30]; > > + int col, row, len, hl_len, d, days[MAXDAYS], firstday; > > + > > + /* > > + * At least 9 bytes for standout mode escape sequence. What is a > safe > > + * size for this? Could this differ from terminal to terminal? > > + */ > > + char *p, lineout[48]; > > > > day_array(month, year, days); > > (void)snprintf(lineout, sizeof(lineout), "%s %d", > > @@ -310,15 +350,21 @@ monthly(int month, int year) > > lineout, day_headings); > > for (row = 0; row < 6; row++) { > > firstday = SPACE; > > + hl_len = 0; > > for (col = 0, p = lineout; col < 7; col++, > > p += julian ? J_DAY_LEN : DAY_LEN) { > > - if (firstday == SPACE && days[row * 7 + col] != > SPACE) > > - firstday = days[row * 7 + col]; > > - ascii_day(p, days[row * 7 + col]); > > + d = days[row * 7 + col]; > > + if (firstday == SPACE && d != SPACE) > > + firstday = d; > > + if (is_today(d, month, year) && highlight) { > > + highlight_day(p, d, &hl_len); > > + p += hl_len; > > + } else > > + ascii_day(p, d); > > } > > *p = '\0'; > > trim_trailing_spaces(lineout); > > - (void)printf("%-20s", lineout); > > + (void)printf("%-*s", 20 + hl_len, lineout); > > if (wflag && firstday != SPACE) > > printf(" [%2d]", week(firstday, month, year)); > > printf("\n"); > > @@ -328,8 +374,13 @@ monthly(int month, int year) > > void > > j_yearly(int year) > > { > > - int col, *dp, i, month, row, which_cal; > > + int col, *dp, i, month, row, which_cal, hl_len; > > int days[12][MAXDAYS]; > > + > > + /* > > + * At least 9 bytes for standout mode escape sequence. What is a > safe > > + * size for this? Could this differ from terminal to terminal? > > + */ > > char *p, lineout[80]; > > > > (void)snprintf(lineout, sizeof(lineout), "%d", year); > > @@ -346,15 +397,30 @@ j_yearly(int year) > > J_HEAD_SEP, "", day_headings); > > > > for (row = 0; row < 6; row++) { > > + hl_len = 0; > > for (which_cal = 0; which_cal < 2; which_cal++) { > > - p = lineout + which_cal * (J_WEEK_LEN + 2); > > + p = lineout + which_cal * (J_WEEK_LEN + 2) > + hl_len; > > These lines are overly long > > > dp = &days[month + which_cal][row * 7]; > > - for (col = 0; col < 7; col++, p += > J_DAY_LEN) > > - ascii_day(p, *dp++); > > + for (col = 0; col < 7; col++, p += > J_DAY_LEN) { > > + if (is_today(*dp, month + > which_cal + 1, > > + year) && highlight) { > > + highlight_day(p, *dp++, > &hl_len); > > + p += hl_len; > > + } else > > + ascii_day(p, *dp++); > > + } > > } > > *p = '\0'; > > trim_trailing_spaces(lineout); > > (void)printf("%s\n", lineout); > > + > > + /* > > + * Highlighting a day causes escape sequences to > > + * misalign the rows. Rather than tracking offsets > to > > + * overwrite rows day-by-day, clear the row. > > + */ > > + if (highlight) > > + (void)memset(lineout, ' ', sizeof(lineout) > - 1); > > } > > } > > (void)printf("\n"); > > @@ -363,9 +429,14 @@ j_yearly(int year) > > void > > yearly(int year) > > { > > - int col, *dp, i, month, row, which_cal, week_len, wn, firstday; > > + int col, *dp, i, month, row, which_cal, week_len, hl_len, wn, > firstday; > > int days[12][MAXDAYS]; > > - char *p, lineout[81]; > > + > > + /* > > + * At least 9 bytes for standout mode escape sequence. What is a > safe > > + * size for this? Could this differ from terminal to terminal? > > + */ > > + char *p, lineout[96]; > > > > week_len = WEEK_LEN; > > if (wflag) > > @@ -386,15 +457,21 @@ yearly(int year) > > HEAD_SEP + (wflag ? WEEKNUMBER_LEN : 0), "", > day_headings); > > > > for (row = 0; row < 6; row++) { > > + hl_len = 0; > > for (which_cal = 0; which_cal < 3; which_cal++) { > > - p = lineout + which_cal * (week_len + 2); > > + p = lineout + which_cal * (week_len + 2) + > hl_len; > > > > dp = &days[month + which_cal][row * 7]; > > firstday = SPACE; > > for (col = 0; col < 7; col++, p += > DAY_LEN) { > > if (firstday == SPACE && *dp != > SPACE) > > firstday = *dp; > > - ascii_day(p, *dp++); > > + if (is_today(*dp, month + > which_cal + 1, > > + year) && highlight) { > > + highlight_day(p, *dp++, > &hl_len); > > + p += hl_len; > > + } else > > + ascii_day(p, *dp++); > > } > > if (wflag && firstday != SPACE) { > > wn = week(firstday, > > @@ -408,6 +485,14 @@ yearly(int year) > > *p = '\0'; > > trim_trailing_spaces(lineout); > > (void)printf("%s\n", lineout); > > + > > + /* > > + * Highlighting a day causes escape sequences to > > + * misalign the rows. Rather than tracking offsets > to > > + * overwrite rows day-by-day, clear the row. > > + */ > > + if (highlight) > > + (void)memset(lineout, ' ', sizeof(lineout) > - 1); > > } > > } > > (void)printf("\n"); > > @@ -514,6 +599,50 @@ ascii_day(char *p, int day) > > } > > > > void > > +highlight_day(char *p, int day, int *hl_len) > > +{ > > + const char *term_so, *term_se; > > + char cbuf[512]; > > + char tbuf[1024], *b; > > + > > + term_se = term_so = NULL; > > + > > + /* > > + * why NULL as name here? > > + * termcap(3) doesn't seem document this > > + */ > > From termcap(3): > > The tgetstr routine returns the string entry for id, or zero if it is > not > available. Use tputs to output the returned string. The area > parameter > is used as follows: > > * It is assumed to be the address of a pointer to a buffer managed > by the calling application. > > * However, ncurses checks to ensure that area is not NULL, and also > that the resulting buffer pointer is not NULL. If either check > fails, the area parameter is ignored. > > * If the checks succeed, ncurses also copies the return value to > the > buffer pointed to by area, and the area value will be updated > to > point past the null ending this value. > > * The return value itself is an address in the terminal description > which is loaded into memory. > > > > + if (tgetent(tbuf, NULL) == 1) { > > + b = cbuf; > > + > > + /* > > + * why not NULL here for area instead of &b? > > + * cbuf is never used > > + */ > > + term_so = tgetstr("so", &b); > > + term_se = tgetstr("se", &b); > > + } > > I think the above tgetstr() calls only need to be done once (at program > start). See above suggestion to move setup of the terminal into a > separate function. > > > + if (term_so != NULL && term_se != NULL) { > > + /* highlight on */ > > + memcpy(p, term_so, strlen(term_so)); > > + p += strlen(term_so); > > + > > + /* the actual text */ > > + ascii_day(p, day); > > + p += (julian ? J_DAY_LEN : DAY_LEN) - 1; > > + > > + /* highlight off */ > > + memcpy(p, term_se, strlen(term_se)); > > + p += strlen(term_se); > > + *p = ' '; > > + *hl_len = strlen(term_so) + strlen(term_se); > > + } else { > > + ascii_day(p, day); > > + *hl_len = 0; > > + } > > +} > > + > > +void > > trim_trailing_spaces(char *s) > > { > > char *p; > > @@ -539,8 +668,7 @@ center(const char *str, int len, int sep > > void > > usage(void) > > { > > - > > - (void)fprintf(stderr, "usage: cal [-jmwy] [month] [year]\n"); > > + (void)fprintf(stderr, "usage: cal [-hjmwy] [month] [year]\n"); > > exit(1); > > no change needed here if -h is removed > > > } > > > > @@ -560,4 +688,12 @@ parsemonth(const char *s) > > if (v <= 0 || v > 12) > > errx(1, "invalid month: use 1-12 or a name"); > > return (v); > > +} > > + > > +int > > +is_today(int day, int month, int year) > > +{ > > + return (year == local_time->tm_year + 1900 && > > + month == local_time->tm_mon + 1 && > > Please use tabs instead of spaces > > > + day == (julian ? local_time->tm_yday + 1 : > local_time->tm_mday)); > > This line is overly long, maybe rename 'local_time' to just 'tm' or 'lt'? > > Kind regards, > > Job > >