Index | Thread | Search

From:
Johannes Thyssen Tishman <jtt@openbsd.org>
Subject:
Re: cal: add option to highlight the current day
To:
Job Snijders <job@bsd.nl>, tech@openbsd.org
Date:
Fri, 31 Jul 2026 20:48:52 +0000

Download raw body.

Thread
  • Johannes Thyssen Tishman:

    cal: add option to highlight the current day

  • 2026-07-05T10:38:19+0000 Johannes Thyssen Tishman <jtt@openbsd.org>:
    > 2026-07-04T09:41:36+0000 Job Snijders <job@bsd.nl>:
    > > 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
    
    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;
     }
    
    
  • Johannes Thyssen Tishman:

    cal: add option to highlight the current day