fix(agenda): align multi-week spans to the start weekday - #1178
Open
liamrlawrence wants to merge 1 commit into
Open
fix(agenda): align multi-week spans to the start weekday#1178liamrlawrence wants to merge 1 commit into
liamrlawrence wants to merge 1 commit into
Conversation
org_agenda_start_on_weekday was only honored for the `week` span and the string '7', so numeric spans never aligned. Setting org_agenda_span = 14 started the range on today rather than the configured weekday. Resolve the span to a day count and align whenever it covers whole weeks (7, 14, 21, ...). Spans with no fixed length and partial weeks keep starting from today: advance_span steps by exactly one span, so only whole-week strides preserve the alignment when paging. Also corrects the start_on_weekday annotations to `number | false`, which the constructor has always supported via utils.if_nil.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
org_agenda_start_on_weekdaywas only honored for theweekspan and thestring
'7', so numeric spans never aligned. Settingorg_agenda_span = 14started the range on today rather than the configured weekday, leaving no way
to get a two-week agenda that begins on a week boundary.
The documentation disagreed with itself and with the code:
docs/configuration.orgweekand number spanlua/orgmode/config/_meta.lua:207weekspan_set_date_rangespan == 'week' or span == '7'Changes
Resolve the span to a day count and align whenever it covers whole weeks
(7, 14, 21, ...). Spans with no fixed length (
day,month,year) andpartial weeks such as 10 keep starting from today.
Partial weeks are excluded deliberately:
advance_spansteps by exactly onespan, so only whole-week jumps preserve the alignment when paging with
f/b. Aligning a 10-day span would give one correct view and then drift,which is worse than never aligning.
On the removal of the
'7'special caseThe removed
span == '7'branch was not working behavior thus was dropped. Astring span aligns correctly, but then crashes immediately afterward in the
same function:
With
span = '7',type(span) == 'number'is false, somodifierbecomes{ ['7'] = 1 }, andDate:adddoesdate[opt] = date[opt] + valagainst anosdatefield that doesn't exist, which throws the error:attempt to perform arithmetic on a nil value.It is also unreachable through config, since
Config:get_agenda_spanvalidates string spans against
{'day', 'month', 'week', 'year'}and fallsback to
week. Only an explicitorg_agenda_span = '7'in a custom command'sopts gets past that, straight into the crash.
tonumberreplaces the branchwith the numeric spans that actually work.
Numeric-string spans remain broken elsewhere (
advance_spanbuilds{ [self.span] = direction }the same way,_get_titlewould print'7'-agenda). Fixing that properly means normalizingspanto a number atconstruction and is left out of scope.
Relation to Emacs
Emacs restricts this to 7 and 14 days. From the
org-agenda-start-on-weekdaydocstring in
lisp/org/org-agenda.el:Note the check is on
ndays, so the integer14does align in Emacs(
org-agenda-ndays-to-spannormalizes it tofortnightfirst).This means that accepting any multiple of 7 is a deliberate superset.
I'm happy to narrow it to
days == 7 or days == 14for exact parity ifyou'd prefer, but I think aligning with multiples of 7 makes more sense than
some arbitrary limit. I can easily imagine myself using a 21 or 28 day agenda.
Type hints
OrgAgendaTypeOpts.start_on_weekdayandOrgAgendaType.start_on_weekdaywereannotated
number, butfalseis supported: the constructor usesutils.if_nilrather thanorspecifically so an explicitfalseisn'treplaced by the config default, and
if self.start_on_weekday thenconsumes it._meta.luaalready typed the user-facing option asnumber | false, andconfiguration.orgdocumented the behavior ("If set tofalse, starts from today")while its
Type:line saidnumber. Corrected everywhere to match, which is whyagenda.luahas changes outside_set_date_range.Tests
Added three tests to verify behavior.
Checklist
I confirm that I have:
Conventional Commits
specification (e.g.,
feat: add new feature,fix: correct bug,docs: update documentation).make test.