Yuchen Pei writes: > Thanks for the comments. > Eli Zaretskii writes: > >>> From: Yuchen Pei >>> Date: Wed, 25 Aug 2021 13:46:48 +1000 >>> See below my first patch to Emacs. The copyright assignment >>> has already been done BTW. >> >> Congrats, and welcome aboard. >> >>> Let me know what you think. >> >> Some minor comments below. >> >>> Would you like me to add some tests? >> >> Adding more tests is always welcome, thanks. > > All diary sexp tests are in icalendar tests, but diary-offset > does not > easily translate to icalendar events (exporting to icalendar for > this > sexp is rather complicated if not impossible as it applies on > top of > another arbitrary sexp), so I am just adding a simple no-op-like > test. > >> >>> Subject: [PATCH] Adding diary-offset, a diary-sexp offsetting >>> another >>> diary-sexp. >> >> "git am" uses the Subject for the heading line, and this >> Subject is >> too long for that. Please consider making it shorter. > > Done. > >> >>> A bit like diary-remind, as a diary-sexp rather than reminder, >>> and >>> also support both positive and negative offsets. >>> This is useful when for example your organization has a >>> committee >>> meeting two days after every monthly meeting which takes place >>> on >>> the >>> third Thursday, or if you would like to attend a virtual >>> meeting >>> scheduled in a different timezone causing a difference in the >>> date. >> >> The commit log message should include a ChangeLog-style >> description >> of >> the files and functions where you made the changes. See >> CONTRIBUTE >> for more details about the format we prefer. >> >> Also, please in the next version include the bug number as part >> of >> the >> log message. > > Done. > >> >>> --- a/doc/emacs/calendar.texi >>> +++ b/doc/emacs/calendar.texi >>> @@ -1363,6 +1363,20 @@ Special Diary Entries >>> Thursday of January, February, and March. If the month is >>> @code{t}, the >>> entry applies to all months of the year. >>> +@findex diary-offset >>> +@example >>> +%%(diary-offset '(diary-float t 3 4) 2) Monthly post-event >>> committee meeting >> >> That line is too long, and will overflow the page width in the >> printed >> version of the manual. Please break it in two. > > Done. I made it shorter :) > >> >>> +@noindent >>> +This entry applies to the Saturday after the third Thursday >>> of >>> each >>> +month. The 2 specifies number of days after when the sexp >>> +@samp{'(diary-float t 3 4)} would evaluate to @code{t}. This >>> is >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> This should be in @code, not @samp. I'd also wrap it in >> @w{..}, so >> that it doesn't get broken between two lines. > > Done. > >> >>> +(defun diary-offset (sexp days) >>> + "Offsetted diary entry. >> >> The first line of a doc string should preferably mention the >> arguments, but without becoming too long, so it could still fit >> on a >> single line. > > Done. > >> >>> + (with-no-warnings (defvar date) (defvar entry)) >> >> Why did you need this? > > The sexp diary-offset itself requires the date supplied by > diary-sexp-entry. The sexp passed to diary-offset will ask for > both date and entry. Removed (defvar entry) as this is not > needed by > diary-offset. After the removel I tested it in org mode and org > agenda and it works. > >> >>> + (integerp days) >> >> Isn't it better to use an assertion? > > Done. Added a user-error statement like in diary-cyclic. > >> >> Thanks. -- Best, Yuchen PGP Key: 47F9 D050 1E11 8879 9040 4941 2126 7E93 EF86 DFD0