Re: cal: add option to highlight the current day
patrick keshishian <[email protected]> Fri, 31 Jul 2026 17:29:15 -0700
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
On Fri, Jul 31, 2026 at 08:48:52PM +0000, Johannes Thyssen Tishman wrote: > 2026-07-05T10:38:19+0000 Johannes Thyssen Tishman <[email protected]>: > > 2026-07-04T09:41:36+0000 Job Snijders <[email protected]>: > > > Hi Johannes, > > > > > > Thanks! I've longed for this functionality for some time now. Some > > > comments below. > > > > Thanks for the feedback Job. I've answered a couple of your comments > > inline and addressed all of them in the patch below. Is this better? > > > > > 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). > > > > The optional argument of the -h flag was mostly so testers could check > > that the highlight was working for other days other than the current one > > without having to change the system date. As mentioned above, my > > intention was to drop this before committing. > > > > I've now updated the patch to highlight the current day by default. > > However, this now conflicts with what Patrick wants. Can we find a > > compromise here? I'm not sure I like an option to *disable* the > > highlight. I'd prefer an option to *enable* it or make it the default > > and have no option at all. > > > > > [...] > > > > > > 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. > > > > Right, this is why I'm asking about the use of cbuf and b, which are not > > used here or in the FreeBSD implementation, instead of just setting area > > to NULL for tgetstr (I've now done this in the revised patch below). > > However, I still don't understand why NULL for the second parameter of > > tgetent. AFAIC, that is not described in termcap(3). > > > > > > + 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); > > > > + } > > ping As a lowly user, who is not a fan of these sort of features, I requested, if this feature is going to be adopted, if it could be enabled by an option, or at least be turned off by one. To which you responded: > > However, this now conflicts with what Patrick wants. Can we find a > > compromise here? I'm not sure I like an option to *disable* the > > highlight. I'd prefer an option to *enable* it or make it the default > > and have no option at all. How about an environment variable to enable the highlight for those who want this option, which can be put in their .profile, retaining the age-long behavior as default for the rest? Something like the following patch on top of your proposal. --patrick --- cal.c.orig Fri Jul 31 16:52:13 2026 +++ cal.c Fri Jul 31 17:03:17 2026 @@ -658,7 +658,13 @@ is_today(int day, int month, int year) int can_highlight(void) { + const char *hilite; char tbuf[1024]; + + hilite = getenv("CAL_HIGHLIGHT"); + if (hilite == NULL || (0 != strcmp(hilite, "1") && + 0 != strcasecmp(hilite, "yes"))) + return 0; if (!isatty(STDOUT_FILENO)) return 0; > 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 31 Jul 2026 20:47:00 -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 <bsd.prog.mk> > 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 31 Jul 2026 20:47:00 -0000 > @@ -42,6 +42,8 @@ > #include <string.h> > #include <time.h> > #include <unistd.h> > +#include <curses.h> > +#include <term.h> > > #define THURSDAY 4 /* for reformation */ > #define SATURDAY 6 /* 1 Jan 1 was a Saturday */ > @@ -127,8 +129,14 @@ const char *day_headings = NULL; > int julian; > int mflag = 0; > int wflag = 0; > +int highlight = 0; > +int highlight_len = 0; > +const char *term_so = NULL; > +const char *term_se = NULL; > +struct tm *lt; > > void ascii_day(char *, int); > +void highlight_day(char *, int); > void center(const char *, int, int); > void day_array(int, int, int *); > int day_in_week(int, int, int); > @@ -141,15 +149,18 @@ void trim_trailing_spaces(char *); > void usage(void); > void yearly(int); > int parsemonth(const char *); > +int is_today(int, int, int); > +int can_highlight(void); > > int > main(int argc, char *argv[]) > { > - struct tm *local_time; > time_t now; > int ch, month, year, yflag; > const char *errstr; > > + highlight = can_highlight(); > + > if (pledge("stdio", NULL) == -1) > err(1, "pledge"); > > @@ -190,6 +201,13 @@ main(int argc, char *argv[]) > day_headings = DAY_HEADINGS_JS; > } > > + (void)time(&now); > + lt = localtime(&now); > + if (lt == NULL) { > + perror("localtime"); > + exit(1); > + } > + > month = 0; > switch(argc) { > case 2: > @@ -202,9 +220,7 @@ 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; > + year = lt->tm_year + 1900; > } else { > year = strtonum(*argv, 1, 9999, &errstr); > if (errstr) > @@ -212,11 +228,9 @@ main(int argc, char *argv[]) > } > break; > case 0: > - (void)time(&now); > - local_time = localtime(&now); > - year = local_time->tm_year + 1900; > + year = lt->tm_year + 1900; > if (!yflag) > - month = local_time->tm_mon + 1; > + month = lt->tm_mon + 1; > break; > default: > usage(); > @@ -298,8 +312,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 +329,22 @@ 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 = highlight_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 +354,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 +377,32 @@ 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 += hl_len; > 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 = highlight_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 +411,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 +439,23 @@ 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 += 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 = highlight_len; > + p += hl_len; > + } else > + ascii_day(p, *dp++); > } > if (wflag && firstday != SPACE) { > wn = week(firstday, > @@ -408,6 +469,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 +583,23 @@ ascii_day(char *p, int day) > } > > void > +highlight_day(char *p, int day) > +{ > + /* 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 = ' '; > +} > + > +void > trim_trailing_spaces(char *s) > { > char *p; > @@ -560,4 +646,29 @@ 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 == lt->tm_year + 1900 && month == lt->tm_mon + 1 && > + day == (julian ? lt->tm_yday + 1 : lt->tm_mday)); > +} > + > +int > +can_highlight(void) > +{ > + char tbuf[1024]; > + > + if (!isatty(STDOUT_FILENO)) > + return 0; > + if (tgetent(tbuf, NULL) == 1) { > + term_so = tgetstr("so", NULL); > + term_se = tgetstr("se", NULL); > + } > + if (term_so != NULL && term_se != NULL) { > + highlight_len = strlen(term_so) + strlen(term_se); > + return 1; > + } > + return 0; > } >