Bug 268936 - cal should not highlight today's date unless stdout is a TTY
Summary: cal should not highlight today's date unless stdout is a TTY
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL: https://reviews.freebsd.org/D38045
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-13 17:00 UTC by Ray Bellis
Modified: 2023-01-27 20:08 UTC (History)
1 user (show)

See Also:
asomers: mfc-stable13+
asomers: mfc-stable12-

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ray Bellis 2023-01-13 17:00:28 UTC 
The cal utility (part of the ncal binary) has the same issue that ncal itself had, reported in 158580.

% cal | cat -v
    January 2023      
Su Mo Tu We Th Fr Sa  
 1  2  3  4  5  6  7  
 8  9 10 11 12 _^H1_^H3 14  
15 16 17 18 19 20 21  
22 23 24 25 26 27 28  
29 30 31
Comment 1 Alan Somers freebsd_committer freebsd_triage 2023-01-13 20:23:30 UTC 
Ray, could you please test the change in the linked URL?
Comment 2 commit-hook freebsd_committer freebsd_triage 2023-01-13 21:30:40 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=92e978439f0c3139775ad96d412959f5a74b17b6

commit 92e978439f0c3139775ad96d412959f5a74b17b6
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-01-13 20:19:03 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-01-13 21:30:00 +0000

    cal: don't print terminal control characters unless stdout is a TTY

    A similar change was made in svn r223931, but it was incomplete, working
    only when the utility was invoked as "ncal".  Fix the same issue when
    invoking as "cal".

    PR:             268936
    Reported by:    Ray Bellis <ray@bellis.me.uk>
    MFC after:      2 weeks
    Sponsored by:   Axcient
    Reviewed by:    imp
    Differential Revision: https://reviews.freebsd.org/D38045

 usr.bin/ncal/ncal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 3 Ray Bellis 2023-01-13 22:36:02 UTC 
(In reply to Alan Somers from comment #1)
I need to update my source tree first, but the patch looks 100% plausible.
Comment 4 Ray Bellis 2023-01-14 11:51:11 UTC 
The patch works, but might I suggest that changing line 220 to:

   flag_nohighlight = !isatty(STDOUT_FILENO);

would be a simpler fix for both cal and ncal, by setting the default
value of this flag right at the start of the code.

With either fix in place, I think the additional check for isatty()
in line 1121 would be superfluous, since highlight() can now never be
called unless stdout really is a TTY.

With my fix above, you could also reverse the older ncal patch on
lines 829/830.

cheers,

Ray
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-01-27 19:32:03 UTC 
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0811d18fea9177b8c378d9d28ea34d6d3c81a5f6

commit 0811d18fea9177b8c378d9d28ea34d6d3c81a5f6
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-01-13 20:19:03 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-01-27 19:30:53 +0000

    cal: don't print terminal control characters unless stdout is a TTY

    A similar change was made in svn r223931, but it was incomplete, working
    only when the utility was invoked as "ncal".  Fix the same issue when
    invoking as "cal".

    PR:             268936
    Reported by:    Ray Bellis <ray@bellis.me.uk>
    Sponsored by:   Axcient
    Reviewed by:    imp
    Differential Revision: https://reviews.freebsd.org/D38045

    (cherry picked from commit 92e978439f0c3139775ad96d412959f5a74b17b6)

 usr.bin/ncal/ncal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 6 Alan Somers freebsd_committer freebsd_triage 2023-01-27 20:08:11 UTC 
Fixed.  No MFC to stable/12 planned.  Reopen the bug if that's needed.