Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -M option to display Monday as the first day of the week in cal(1) #1294

Merged
merged 1 commit into from
Jul 7, 2024
Merged

Add -M option to display Monday as the first day of the week in cal(1) #1294

merged 1 commit into from
Jul 7, 2024

Conversation

vallbsd
Copy link
Contributor

@vallbsd vallbsd commented Jun 15, 2024

As you can find, FreeBSD cal(1) man page says that

It is not possible to display Monday as the first day of the week with cal.

But daily it is more comfortable if weeks start from Monday.
I know that there is ncal (vertical) mode which always starts from Monday,
but any other calendar in the world (software or that on the wall) is horizontal,
so ncal is uncomfortable too.
So I've made a simple patch that adds -M option to cal(1) to start week from Monday.
Tried to make as minimal changes to original code as possible.

Copy link
Contributor

@concussious concussious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only comment on the manual page. Would you be willing to tag SPDX?

.\"-
.\" SPDX-License-Identifier: BSD-2-Clause
.\"

This patch is a nice change for Germans who start weeks on Mondays.

usr.bin/ncal/ncal.1 Outdated Show resolved Hide resolved
usr.bin/ncal/ncal.1 Outdated Show resolved Hide resolved
usr.bin/ncal/ncal.1 Outdated Show resolved Hide resolved
@vallbsd vallbsd requested a review from concussious June 16, 2024 16:25
Copy link
Contributor

@concussious concussious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! After this, manual changes LGTM.

usr.bin/ncal/ncal.1 Show resolved Hide resolved
usr.bin/ncal/ncal.c Outdated Show resolved Hide resolved
@concussious
Copy link
Contributor

Co-authored is incorrect in this case. This work is entirely your own. The appropriate way to credit me is with a Reviewed by: trailer.

Please squash these down to one commit. The first word should describe what's being changed, then a colon, then a short description.

Since this change shouldn't break anything, don't forget to specify MFC after: in order for this work to get into stable. Otherwise, it will stay in the development branch until FreeBSD 15.

Example:

ncal: add -M to start week on Monday

MFC after: 2 weeks
Reviewed by: Alexander Ziaee (manual)

@vallbsd
Copy link
Contributor Author

vallbsd commented Jun 17, 2024

Please squash these down to one commit. The first word should describe what's being changed, then a colon, then a short description.

Example:

ncal: add -M to start week on Monday

MFC after: 2 weeks
Reviewed by: Alexander Ziaee (manual)

It seems I've done this right ))

Copy link
Contributor

@concussious concussious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, you still have to find someone to review the src, but everything I'm qualified to comment on looks great.

usr.bin/ncal/ncal.c Outdated Show resolved Hide resolved
usr.bin/ncal/ncal.c Outdated Show resolved Hide resolved
@bsdimp
Copy link
Member

bsdimp commented Jun 17, 2024

Thanks for coming here and submitting. Two very minor style nits, but the rest looks great.

@bsdimp
Copy link
Member

bsdimp commented Jun 17, 2024

I expect the builds to work... and the commit message looks good too.

@concussious
Copy link
Contributor

I tested it the other day, build worked on amd64/main.

@emaste
Copy link
Member

emaste commented Jun 18, 2024

Is there another cal implementation that uses -M to start on Monday?

@bsdimp
Copy link
Member

bsdimp commented Jun 18, 2024

I looked briefly. I couldn't find one. Gcal was the only other cli program I found. It can also do Monday start, but that is tied to using isolated week mode...

@vallbsd
Copy link
Contributor Author

vallbsd commented Jun 18, 2024

Is there another cal implementation that uses -M to start on Monday?

Debian/Ubuntu uses FreeBSD's patched ncal in their bases with -M for Monday added.

OpenBSD and util-linux use -m for this, but it's already occupied in ncal to print custom month.
Also -M can be useful here in case someone wishes to add -S to start week on Sunday in vertical (ncal) mode :-)

@concussious
Copy link
Contributor

Is there another cal implementation that uses -M to start on Monday?

Debian/Ubuntu uses FreeBSD's patched ncal in their bases with -M for Monday added.

Wow, are you guys seeing this? They took BSDL code, improved it, and didn't wrap it in GPL! Ian Murdock was a great man.

https://metadata.ftp-master.debian.org/changelogs//main/b/bsdmainutils/bsdmainutils_12.1.7+nmu3_copyright

@vallbsd vallbsd requested a review from bsdimp June 19, 2024 15:43
@bsdimp
Copy link
Member

bsdimp commented Jun 19, 2024

Is there another cal implementation that uses -M to start on Monday?

Debian/Ubuntu uses FreeBSD's patched ncal in their bases with -M for Monday added.

Wow, are you guys seeing this? They took BSDL code, improved it, and didn't wrap it in GPL! Ian Murdock was a great man.

https://metadata.ftp-master.debian.org/changelogs//main/b/bsdmainutils/bsdmainutils_12.1.7+nmu3_copyright

Yea. But didn't keep up with upstream... It's good they kept the original license... A lot of early BSDL and BSDL-ish code was swallowed by the GPL either just because they felt like it, or because in a few cases, they located the original author and they said yes. I applaud this group for doing the right thing.

Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good now.

MFC after: 2 weeks
Reviewed by: imp, Alexander Ziaee,
Pull Request: #1294
@freebsd-git freebsd-git merged commit 8c108b3 into freebsd:main Jul 7, 2024
7 of 9 checks passed
freebsd-git pushed a commit that referenced this pull request Sep 22, 2024
MFC after: 2 weeks
Reviewed by: imp, Alexander Ziaee,
Pull Request: #1294

(cherry picked from commit 8c108b3)
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Oct 30, 2024
MFC after: 2 weeks
Reviewed by: imp, Alexander Ziaee,
Pull Request: freebsd/freebsd-src#1294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants